Skip to content

Commit ca3c09c

Browse files
Merge pull request #948 from github/michaelrfairhurst/package-classes2-split
Implement most of Classes2, split out two harder rules
2 parents 4fea942 + 75510f4 commit ca3c09c

32 files changed

+1423
-2
lines changed
Lines changed: 129 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,129 @@
1+
//** THIS FILE IS AUTOGENERATED, DO NOT MODIFY DIRECTLY. **/
2+
import cpp
3+
import RuleMetadata
4+
import codingstandards.cpp.exclusions.RuleMetadata
5+
6+
newtype Classes2Query =
7+
TVirtualInheritanceNotAllowedQuery() or
8+
TMemberSpecifiersNotUsedAppropriatelyQuery() or
9+
TPrivateAndPublicDataMembersMixedQuery() or
10+
TInvalidSignatureForSpecialMemberFunctionQuery() or
11+
TNonExplicitConversionMemberQuery() or
12+
TLogicalAndAndLogicalOrOperatorsOverloadedQuery() or
13+
TInvalidOperatorOverloadedAsMemberFunctionQuery()
14+
15+
predicate isClasses2QueryMetadata(Query query, string queryId, string ruleId, string category) {
16+
query =
17+
// `Query` instance for the `virtualInheritanceNotAllowed` query
18+
Classes2Package::virtualInheritanceNotAllowedQuery() and
19+
queryId =
20+
// `@id` for the `virtualInheritanceNotAllowed` query
21+
"cpp/misra/virtual-inheritance-not-allowed" and
22+
ruleId = "RULE-13-1-1" and
23+
category = "advisory"
24+
or
25+
query =
26+
// `Query` instance for the `memberSpecifiersNotUsedAppropriately` query
27+
Classes2Package::memberSpecifiersNotUsedAppropriatelyQuery() and
28+
queryId =
29+
// `@id` for the `memberSpecifiersNotUsedAppropriately` query
30+
"cpp/misra/member-specifiers-not-used-appropriately" and
31+
ruleId = "RULE-13-3-1" and
32+
category = "required"
33+
or
34+
query =
35+
// `Query` instance for the `privateAndPublicDataMembersMixed` query
36+
Classes2Package::privateAndPublicDataMembersMixedQuery() and
37+
queryId =
38+
// `@id` for the `privateAndPublicDataMembersMixed` query
39+
"cpp/misra/private-and-public-data-members-mixed" and
40+
ruleId = "RULE-14-1-1" and
41+
category = "advisory"
42+
or
43+
query =
44+
// `Query` instance for the `invalidSignatureForSpecialMemberFunction` query
45+
Classes2Package::invalidSignatureForSpecialMemberFunctionQuery() and
46+
queryId =
47+
// `@id` for the `invalidSignatureForSpecialMemberFunction` query
48+
"cpp/misra/invalid-signature-for-special-member-function" and
49+
ruleId = "RULE-15-0-2" and
50+
category = "advisory"
51+
or
52+
query =
53+
// `Query` instance for the `nonExplicitConversionMember` query
54+
Classes2Package::nonExplicitConversionMemberQuery() and
55+
queryId =
56+
// `@id` for the `nonExplicitConversionMember` query
57+
"cpp/misra/non-explicit-conversion-member" and
58+
ruleId = "RULE-15-1-3" and
59+
category = "required"
60+
or
61+
query =
62+
// `Query` instance for the `logicalAndAndLogicalOrOperatorsOverloaded` query
63+
Classes2Package::logicalAndAndLogicalOrOperatorsOverloadedQuery() and
64+
queryId =
65+
// `@id` for the `logicalAndAndLogicalOrOperatorsOverloaded` query
66+
"cpp/misra/logical-and-and-logical-or-operators-overloaded" and
67+
ruleId = "RULE-16-5-1" and
68+
category = "required"
69+
or
70+
query =
71+
// `Query` instance for the `invalidOperatorOverloadedAsMemberFunction` query
72+
Classes2Package::invalidOperatorOverloadedAsMemberFunctionQuery() and
73+
queryId =
74+
// `@id` for the `invalidOperatorOverloadedAsMemberFunction` query
75+
"cpp/misra/invalid-operator-overloaded-as-member-function" and
76+
ruleId = "RULE-16-6-1" and
77+
category = "advisory"
78+
}
79+
80+
module Classes2Package {
81+
Query virtualInheritanceNotAllowedQuery() {
82+
//autogenerate `Query` type
83+
result =
84+
// `Query` type for `virtualInheritanceNotAllowed` query
85+
TQueryCPP(TClasses2PackageQuery(TVirtualInheritanceNotAllowedQuery()))
86+
}
87+
88+
Query memberSpecifiersNotUsedAppropriatelyQuery() {
89+
//autogenerate `Query` type
90+
result =
91+
// `Query` type for `memberSpecifiersNotUsedAppropriately` query
92+
TQueryCPP(TClasses2PackageQuery(TMemberSpecifiersNotUsedAppropriatelyQuery()))
93+
}
94+
95+
Query privateAndPublicDataMembersMixedQuery() {
96+
//autogenerate `Query` type
97+
result =
98+
// `Query` type for `privateAndPublicDataMembersMixed` query
99+
TQueryCPP(TClasses2PackageQuery(TPrivateAndPublicDataMembersMixedQuery()))
100+
}
101+
102+
Query invalidSignatureForSpecialMemberFunctionQuery() {
103+
//autogenerate `Query` type
104+
result =
105+
// `Query` type for `invalidSignatureForSpecialMemberFunction` query
106+
TQueryCPP(TClasses2PackageQuery(TInvalidSignatureForSpecialMemberFunctionQuery()))
107+
}
108+
109+
Query nonExplicitConversionMemberQuery() {
110+
//autogenerate `Query` type
111+
result =
112+
// `Query` type for `nonExplicitConversionMember` query
113+
TQueryCPP(TClasses2PackageQuery(TNonExplicitConversionMemberQuery()))
114+
}
115+
116+
Query logicalAndAndLogicalOrOperatorsOverloadedQuery() {
117+
//autogenerate `Query` type
118+
result =
119+
// `Query` type for `logicalAndAndLogicalOrOperatorsOverloaded` query
120+
TQueryCPP(TClasses2PackageQuery(TLogicalAndAndLogicalOrOperatorsOverloadedQuery()))
121+
}
122+
123+
Query invalidOperatorOverloadedAsMemberFunctionQuery() {
124+
//autogenerate `Query` type
125+
result =
126+
// `Query` type for `invalidOperatorOverloadedAsMemberFunction` query
127+
TQueryCPP(TClasses2PackageQuery(TInvalidOperatorOverloadedAsMemberFunctionQuery()))
128+
}
129+
}

cpp/common/src/codingstandards/cpp/exclusions/cpp/RuleMetadata.qll

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import BannedLibraries
99
import BannedSyntax
1010
import BannedTypes
1111
import Classes
12+
import Classes2
1213
import Comments
1314
import Concurrency
1415
import Conditionals
@@ -78,6 +79,7 @@ newtype TCPPQuery =
7879
TBannedSyntaxPackageQuery(BannedSyntaxQuery q) or
7980
TBannedTypesPackageQuery(BannedTypesQuery q) or
8081
TClassesPackageQuery(ClassesQuery q) or
82+
TClasses2PackageQuery(Classes2Query q) or
8183
TCommentsPackageQuery(CommentsQuery q) or
8284
TConcurrencyPackageQuery(ConcurrencyQuery q) or
8385
TConditionalsPackageQuery(ConditionalsQuery q) or
@@ -147,6 +149,7 @@ predicate isQueryMetadata(Query query, string queryId, string ruleId, string cat
147149
isBannedSyntaxQueryMetadata(query, queryId, ruleId, category) or
148150
isBannedTypesQueryMetadata(query, queryId, ruleId, category) or
149151
isClassesQueryMetadata(query, queryId, ruleId, category) or
152+
isClasses2QueryMetadata(query, queryId, ruleId, category) or
150153
isCommentsQueryMetadata(query, queryId, ruleId, category) or
151154
isConcurrencyQueryMetadata(query, queryId, ruleId, category) or
152155
isConditionalsQueryMetadata(query, queryId, ruleId, category) or
Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
/**
2+
* @id cpp/misra/virtual-inheritance-not-allowed
3+
* @name RULE-13-1-1: Classes should not be inherited virtually
4+
* @description Virtual inheritance should not be used as it may lead to unexpected behavior.
5+
* @kind problem
6+
* @precision very-high
7+
* @problem.severity warning
8+
* @tags external/misra/id/rule-13-1-1
9+
* scope/single-translation-unit
10+
* correctness
11+
* readability
12+
* external/misra/enforcement/decidable
13+
* external/misra/obligation/advisory
14+
*/
15+
16+
import cpp
17+
import codingstandards.cpp.misra
18+
19+
from VirtualClassDerivation vcd
20+
where not isExcluded(vcd, Classes2Package::virtualInheritanceNotAllowedQuery())
21+
select vcd,
22+
"Class '" + vcd.getDerivedClass().getName() + "' inherits virtually from '" +
23+
vcd.getBaseClass().getName() + "'."
Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
/**
2+
* @id cpp/misra/member-specifiers-not-used-appropriately
3+
* @name RULE-13-3-1: User-declared member functions shall use the virtual, override and final specifiers appropriately
4+
* @description Appropriate use of specifiers on member functions more clearly indicate the
5+
* intention of the function.
6+
* @kind problem
7+
* @precision very-high
8+
* @problem.severity warning
9+
* @tags external/misra/id/rule-13-3-1
10+
* scope/single-translation-unit
11+
* readability
12+
* maintainability
13+
* external/misra/enforcement/decidable
14+
* external/misra/obligation/required
15+
*/
16+
17+
import cpp
18+
import codingstandards.cpp.misra
19+
20+
from MemberFunction f, string message
21+
where
22+
not isExcluded(f, Classes2Package::memberSpecifiersNotUsedAppropriatelyQuery()) and
23+
(
24+
// Case 1: Specifiers incompatible with explicitly virtual
25+
f.isDeclaredVirtual() and
26+
exists(Specifier s |
27+
s = f.getASpecifier() and
28+
s.getName() = ["final", "override"] and
29+
message =
30+
"Member function '" + f.getName() + "' uses redundant 'virtual' and '" + s.getName() +
31+
"' specifiers together."
32+
)
33+
or
34+
// Case 2: Redundant 'virtual' specifier
35+
f.overrides(_) and
36+
f.isDeclaredVirtual() and
37+
message =
38+
"Member function '" + f.getName() +
39+
"' overrides a base function but uses 'virtual' specifier."
40+
or
41+
// Case 3: both 'override' and 'final' specifiers on an overridden virtual function
42+
f.isVirtual() and
43+
not f.isDeclaredVirtual() and
44+
f.isOverride() and
45+
f.isFinal() and
46+
message =
47+
"Member function '" + f.getName() +
48+
"' uses redundant 'override' and 'final' specifiers together."
49+
or
50+
// Case 5: overrides a virtual function but has no override or final specifier
51+
f.overrides(_) and
52+
not f.isDeclaredVirtual() and
53+
not f.isOverride() and
54+
not f.isFinal() and
55+
message =
56+
"Member function '" + f.getName() +
57+
"' overrides a base function but lacks 'override' or 'final' specifier."
58+
)
59+
select f, message
Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
/**
2+
* @id cpp/misra/private-and-public-data-members-mixed
3+
* @name RULE-14-1-1: Non-static data members should be either all private or all public
4+
* @description Mixing public and private data members in a class obfuscates the range of valid
5+
* states allowable for that class.
6+
* @kind problem
7+
* @precision very-high
8+
* @problem.severity warning
9+
* @tags external/misra/id/rule-14-1-1
10+
* scope/single-translation-unit
11+
* maintainability
12+
* external/misra/enforcement/decidable
13+
* external/misra/obligation/advisory
14+
*/
15+
16+
import cpp
17+
import codingstandards.cpp.misra
18+
19+
from Class c, Field f
20+
where
21+
not isExcluded(f, Classes2Package::privateAndPublicDataMembersMixedQuery()) and
22+
f = c.getAMember() and
23+
not f.isStatic() and
24+
(
25+
// Protected data members is not allowed.
26+
f.isProtected()
27+
or
28+
// Public and private data members cannot be mixed.
29+
exists(Field other |
30+
// Allow manual exclusion of 'other' field.
31+
not isExcluded(other, Classes2Package::privateAndPublicDataMembersMixedQuery()) and
32+
other = c.getAMember() and
33+
not other.isStatic() and
34+
other != f and
35+
(
36+
f.isPublic() and other.isPrivate()
37+
or
38+
f.isPrivate() and other.isPublic()
39+
or
40+
f.isProtected() and
41+
(other.isPublic() or other.isPrivate())
42+
or
43+
(f.isPublic() or f.isPrivate()) and
44+
other.isProtected()
45+
)
46+
)
47+
)
48+
select f,
49+
"Non-static data member '" + f.getName() +
50+
"' has mixed access level with other data members in class '" + c.getName() + "'."
Lines changed: 106 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,106 @@
1+
/**
2+
* @id cpp/misra/invalid-signature-for-special-member-function
3+
* @name RULE-15-0-2: User-provided copy and move member functions of a class should have appropriate signatures
4+
* @description Proper implementations of copy and move constructors and assignment operators ensure
5+
* that resources are managed correctly.
6+
* @kind problem
7+
* @precision very-high
8+
* @problem.severity warning
9+
* @tags external/misra/id/rule-15-0-2
10+
* scope/single-translation-unit
11+
* correctness
12+
* maintainability
13+
* external/misra/enforcement/decidable
14+
* external/misra/obligation/advisory
15+
*/
16+
17+
import cpp
18+
import codingstandards.cpp.misra
19+
import codeql.util.Boolean
20+
21+
pragma[inline]
22+
predicate checkSignature(
23+
MemberFunction f, Boolean noexcept, Boolean lvalueQualified, Boolean rvalueRef, string err
24+
) {
25+
f.getNumberOfParameters() > 1 and
26+
err = "has too many parameters"
27+
or
28+
noexcept = true and
29+
not f.isNoExcept() and
30+
err = "should be noexcept"
31+
or
32+
// Note: There is no check when `lvalueQualified` is false, only when true. `lvalueQualified` will
33+
// only be false for constructors, and constructors cannot be ref-qualified.
34+
lvalueQualified = true and
35+
not f.isLValueRefQualified() and
36+
err = "should be lvalue-qualified"
37+
or
38+
// Note: This case is also only here for completeness. `rvalueRef` is only true for move
39+
// constructors and move assignment operators. However, these special member functions by
40+
// definition must take an rvalue reference, so this case cannot hold.
41+
rvalueRef = true and
42+
not isRvalueRefX(f.getParameter(0).getType(), f.getDeclaringType()) and
43+
err = "should take rvalue reference to class or struct type"
44+
or
45+
rvalueRef = false and
46+
not isConstRefX(f.getParameter(0).getType(), f.getDeclaringType()) and
47+
err = "should take const reference to class or struct type"
48+
or
49+
f.isVirtual() and
50+
err = "should not be virtual"
51+
or
52+
not f instanceof Constructor and
53+
not f.getType().(LValueReferenceType).getBaseType() = f.getDeclaringType() and
54+
not f.getType() instanceof VoidType and
55+
err = "should return void or lvalue reference to class or struct type"
56+
or
57+
// Note: this is here for completeness. It is a static error to declare copy or move constructors
58+
// as `explicit`, so this cannot hold.
59+
not f instanceof Constructor and
60+
f.isExplicit() and
61+
err = "should not be explicit"
62+
}
63+
64+
predicate isRvalueRefX(Type t, Class x) {
65+
// X &&
66+
t.(RValueReferenceType).getBaseType() = x
67+
}
68+
69+
predicate isConstRefX(Type t, Class x) {
70+
exists(SpecifiedType st |
71+
// X const &
72+
st = t.(LValueReferenceType).getBaseType() and
73+
st.getSpecifierString() = "const" and
74+
st.getBaseType() = x
75+
or
76+
// const X &
77+
st = t and
78+
st.getSpecifierString() = "const" and
79+
st.getBaseType().(LValueReferenceType).getBaseType() = x
80+
)
81+
}
82+
83+
from MemberFunction f, string err, string prefix
84+
where
85+
not isExcluded(f, Classes2Package::invalidSignatureForSpecialMemberFunctionQuery()) and
86+
not f.isDeleted() and
87+
not f.isDefaulted() and
88+
not f.isCompilerGenerated() and
89+
(
90+
f instanceof CopyConstructor and
91+
prefix = "Copy constructor" and
92+
checkSignature(f, false, false, false, err)
93+
or
94+
f instanceof MoveConstructor and
95+
prefix = "Move constructor" and
96+
checkSignature(f, true, false, true, err)
97+
or
98+
f instanceof CopyAssignmentOperator and
99+
prefix = "Copy assignment operator" and
100+
checkSignature(f, false, true, false, err)
101+
or
102+
f instanceof MoveAssignmentOperator and
103+
prefix = "Move assignment operator" and
104+
checkSignature(f, true, true, true, err)
105+
)
106+
select f, prefix + " on type $@ " + err + ".", f.getDeclaringType(), f.getDeclaringType().getName()

0 commit comments

Comments
 (0)