Skip to content

Commit 89910dd

Browse files
rvermeulenrvermeulen
committed
Adjust alert message for A12-4-1 and A12-8-6
For A12-8-6, the alert no longer contains the reason for the base class being a base class. This is because a base class with both a derived class and being abstract is reported as a separate alert. For A12-4-1, the alert contains the fully qualified name of the base class and is formatted according to the style guide.
1 parent 6266962 commit 89910dd

File tree

5 files changed

+69
-53
lines changed

5 files changed

+69
-53
lines changed

‎cpp/autosar/src/rules/A12-4-1/DestructorOfABaseClassNotPublicVirtual.ql

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -29,9 +29,9 @@ predicate isProtectedNonVirtual(Destructor d) { d.isProtected() and not d.isVirt
2929
from Destructor d
3030
where
3131
not isExcluded(d, VirtualFunctionsPackage::destructorOfABaseClassNotPublicVirtualQuery()) and
32-
isPossibleBaseClass(d.getDeclaringType(), _) and
32+
d.getDeclaringType() instanceof BaseClass and
3333
(not isPublicOverride(d) and not isProtectedNonVirtual(d) and not isPublicVirtual(d))
3434
// Report the declaration entry in the class body, as that is where the access specifier should be set
3535
select getDeclarationEntryInClassDeclaration(d),
36-
"Destructor of base class " + d.getDeclaringType() +
37-
" is not declared as public virtual, public override, or protected non-virtual."
36+
"Destructor of base class '" + d.getDeclaringType().getQualifiedName() +
37+
"' is not declared as public virtual, public override, or protected non-virtual."

‎cpp/autosar/src/rules/A12-8-6/CopyAndMoveNotDeclaredProtected.ql

Lines changed: 22 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -20,31 +20,45 @@ predicate isInvalidConstructor(Constructor f, string constructorType) {
2020
not f.isDeleted() and
2121
not f.isProtected() and
2222
(
23-
f instanceof MoveConstructor and constructorType = "Move constructor"
23+
f instanceof MoveConstructor and
24+
if f.isCompilerGenerated()
25+
then constructorType = "Implicit move constructor"
26+
else constructorType = "Move constructor"
2427
or
25-
f instanceof CopyConstructor and constructorType = "Copy constructor"
28+
f instanceof CopyConstructor and
29+
if f.isCompilerGenerated()
30+
then constructorType = "Implicit copy constructor"
31+
else constructorType = "Copy constructor"
2632
)
2733
}
2834

2935
predicate isInvalidAssignment(Operator f, string operatorType) {
3036
not f.isDeleted() and
3137
(
32-
f instanceof CopyAssignmentOperator and operatorType = "Copy assignment operator"
38+
f instanceof MoveAssignmentOperator and
39+
if f.isCompilerGenerated()
40+
then operatorType = "Implicit move assignment operator"
41+
else operatorType = "Move constructor"
3342
or
34-
f instanceof MoveAssignmentOperator and operatorType = "Move assignment operator"
43+
f instanceof CopyAssignmentOperator and
44+
if f.isCompilerGenerated()
45+
then operatorType = "Implicit copy assignment operator"
46+
else operatorType = "Copy assignment operator"
3547
) and
3648
not f.hasSpecifier("protected")
3749
}
3850

39-
from MemberFunction mf, string type, string baseReason
51+
from BaseClass baseClass, MemberFunction mf, string type
4052
where
4153
not isExcluded(mf, OperatorsPackage::copyAndMoveNotDeclaredProtectedQuery()) and
4254
(
4355
isInvalidConstructor(mf, type)
4456
or
4557
isInvalidAssignment(mf, type)
4658
) and
47-
isPossibleBaseClass(mf.getDeclaringType(), baseReason)
59+
baseClass = mf.getDeclaringType()
60+
// To avoid duplicate alerts due to inaccurate location information in the database we don't use the location of the base class.
61+
// This for example happens if multiple copies of the same header file are present in the database.
4862
select getDeclarationEntryInClassDeclaration(mf),
49-
type + " for base class " + mf.getDeclaringType().getQualifiedName() + " (" + baseReason +
50-
") is not declared protected or deleted."
63+
type + " for base class '" + baseClass.getQualifiedName() +
64+
"' is not declared protected or deleted."

‎cpp/autosar/test/rules/A12-4-1/DestructorOfABaseClassNotPublicVirtual.expected

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,2 +1,2 @@
1-
| test.cpp:4:3:4:4 | definition of ~A | Destructor of base class A is not declared as public virtual, public override, or protected non-virtual. |
2-
| test.cpp:30:3:30:4 | definition of ~E | Destructor of base class E is not declared as public virtual, public override, or protected non-virtual. |
1+
| test.cpp:4:3:4:4 | definition of ~A | Destructor of base class 'A' is not declared as public virtual, public override, or protected non-virtual. |
2+
| test.cpp:30:3:30:4 | definition of ~E | Destructor of base class 'E' is not declared as public virtual, public override, or protected non-virtual. |

‎cpp/autosar/test/rules/A12-8-6/CopyAndMoveNotDeclaredProtected.expected

Lines changed: 22 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -1,22 +1,22 @@
1-
| test.cpp:4:3:4:12 | declaration of BaseClass1 | Copy constructor for base class BaseClass1 (a derived class exists) is not declared protected or deleted. |
2-
| test.cpp:5:3:5:12 | declaration of BaseClass1 | Move constructor for base class BaseClass1 (a derived class exists) is not declared protected or deleted. |
3-
| test.cpp:6:15:6:23 | declaration of operator= | Copy assignment operator for base class BaseClass1 (a derived class exists) is not declared protected or deleted. |
4-
| test.cpp:7:15:7:23 | declaration of operator= | Move assignment operator for base class BaseClass1 (a derived class exists) is not declared protected or deleted. |
5-
| test.cpp:15:7:15:7 | declaration of operator= | Copy assignment operator for base class BaseClass2 (a derived class exists) is not declared protected or deleted. |
6-
| test.cpp:15:7:15:7 | declaration of operator= | Move assignment operator for base class BaseClass2 (a derived class exists) is not declared protected or deleted. |
7-
| test.cpp:55:3:55:12 | declaration of BaseClass5 | Copy constructor for base class BaseClass5 (a derived class exists) is not declared protected or deleted. |
8-
| test.cpp:56:3:56:12 | declaration of BaseClass5 | Move constructor for base class BaseClass5 (a derived class exists) is not declared protected or deleted. |
9-
| test.cpp:57:15:57:23 | declaration of operator= | Copy assignment operator for base class BaseClass5 (a derived class exists) is not declared protected or deleted. |
10-
| test.cpp:58:15:58:23 | declaration of operator= | Move assignment operator for base class BaseClass5 (a derived class exists) is not declared protected or deleted. |
11-
| test.cpp:75:3:75:12 | declaration of BaseClass6 | Copy constructor for base class BaseClass6 (the class is abstract) is not declared protected or deleted. |
12-
| test.cpp:76:3:76:12 | declaration of BaseClass6 | Move constructor for base class BaseClass6 (the class is abstract) is not declared protected or deleted. |
13-
| test.cpp:77:15:77:23 | declaration of operator= | Copy assignment operator for base class BaseClass6 (the class is abstract) is not declared protected or deleted. |
14-
| test.cpp:78:15:78:23 | declaration of operator= | Move assignment operator for base class BaseClass6 (the class is abstract) is not declared protected or deleted. |
15-
| test.cpp:85:3:85:12 | declaration of BaseClass7 | Copy constructor for base class BaseClass7<T1> (a derived class exists) is not declared protected or deleted. |
16-
| test.cpp:86:3:86:12 | declaration of BaseClass7 | Move constructor for base class BaseClass7<T1> (a derived class exists) is not declared protected or deleted. |
17-
| test.cpp:87:15:87:23 | declaration of operator= | Copy assignment operator for base class BaseClass7<T1> (a derived class exists) is not declared protected or deleted. |
18-
| test.cpp:88:15:88:23 | declaration of operator= | Move assignment operator for base class BaseClass7<T1> (a derived class exists) is not declared protected or deleted. |
19-
| test.cpp:108:3:108:12 | declaration of BaseClass8 | Copy constructor for base class BaseClass8 (a derived class exists) is not declared protected or deleted. |
20-
| test.cpp:109:3:109:12 | declaration of BaseClass8 | Move constructor for base class BaseClass8 (a derived class exists) is not declared protected or deleted. |
21-
| test.cpp:110:15:110:23 | declaration of operator= | Copy assignment operator for base class BaseClass8 (a derived class exists) is not declared protected or deleted. |
22-
| test.cpp:111:15:111:23 | declaration of operator= | Move assignment operator for base class BaseClass8 (a derived class exists) is not declared protected or deleted. |
1+
| test.cpp:4:3:4:12 | declaration of BaseClass1 | Copy constructor for base class 'BaseClass1' is not declared protected or deleted. |
2+
| test.cpp:5:3:5:12 | declaration of BaseClass1 | Move constructor for base class 'BaseClass1' is not declared protected or deleted. |
3+
| test.cpp:6:15:6:23 | declaration of operator= | Copy assignment operator for base class 'BaseClass1' is not declared protected or deleted. |
4+
| test.cpp:7:15:7:23 | declaration of operator= | Move constructor for base class 'BaseClass1' is not declared protected or deleted. |
5+
| test.cpp:15:7:15:7 | declaration of operator= | Implicit copy assignment operator for base class 'BaseClass2' is not declared protected or deleted. |
6+
| test.cpp:15:7:15:7 | declaration of operator= | Implicit move assignment operator for base class 'BaseClass2' is not declared protected or deleted. |
7+
| test.cpp:55:3:55:12 | declaration of BaseClass5 | Copy constructor for base class 'BaseClass5' is not declared protected or deleted. |
8+
| test.cpp:56:3:56:12 | declaration of BaseClass5 | Move constructor for base class 'BaseClass5' is not declared protected or deleted. |
9+
| test.cpp:57:15:57:23 | declaration of operator= | Copy assignment operator for base class 'BaseClass5' is not declared protected or deleted. |
10+
| test.cpp:58:15:58:23 | declaration of operator= | Move constructor for base class 'BaseClass5' is not declared protected or deleted. |
11+
| test.cpp:75:3:75:12 | declaration of BaseClass6 | Copy constructor for base class 'BaseClass6' is not declared protected or deleted. |
12+
| test.cpp:76:3:76:12 | declaration of BaseClass6 | Move constructor for base class 'BaseClass6' is not declared protected or deleted. |
13+
| test.cpp:77:15:77:23 | declaration of operator= | Copy assignment operator for base class 'BaseClass6' is not declared protected or deleted. |
14+
| test.cpp:78:15:78:23 | declaration of operator= | Move constructor for base class 'BaseClass6' is not declared protected or deleted. |
15+
| test.cpp:85:3:85:12 | declaration of BaseClass7 | Copy constructor for base class 'BaseClass7<T1>' is not declared protected or deleted. |
16+
| test.cpp:86:3:86:12 | declaration of BaseClass7 | Move constructor for base class 'BaseClass7<T1>' is not declared protected or deleted. |
17+
| test.cpp:87:15:87:23 | declaration of operator= | Copy assignment operator for base class 'BaseClass7<T1>' is not declared protected or deleted. |
18+
| test.cpp:88:15:88:23 | declaration of operator= | Move constructor for base class 'BaseClass7<T1>' is not declared protected or deleted. |
19+
| test.cpp:108:3:108:12 | declaration of BaseClass8 | Copy constructor for base class 'BaseClass8' is not declared protected or deleted. |
20+
| test.cpp:109:3:109:12 | declaration of BaseClass8 | Move constructor for base class 'BaseClass8' is not declared protected or deleted. |
21+
| test.cpp:110:15:110:23 | declaration of operator= | Copy assignment operator for base class 'BaseClass8' is not declared protected or deleted. |
22+
| test.cpp:111:15:111:23 | declaration of operator= | Move constructor for base class 'BaseClass8' is not declared protected or deleted. |

‎cpp/common/src/codingstandards/cpp/Class.qll

Lines changed: 20 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -5,27 +5,29 @@
55
import cpp
66
import codingstandards.cpp.Expr
77

8-
/**
9-
* Holds if we believe that `c` is used or intended to be used as a base class.
10-
*/
11-
predicate isPossibleBaseClass(Class c, string reason) {
12-
// There exists a derivation in this database
13-
(
14-
// We make a distinction between class template instantiations, regular classes and template classes.
15-
// For template classes we do have derived classes, because derived classes would derive from a
16-
// class template instantiation.
17-
// Therefore, we check for derived classes for regular classes
18-
not c instanceof ClassTemplateInstantiation and not c instanceof TemplateClass and exists(c.getADerivedClass())
8+
9+
private Class getADerivedClass(Class c) {
10+
not c instanceof ClassTemplateInstantiation and not c instanceof TemplateClass and result = c.getADerivedClass()
1911
or
20-
// and use template instantiations to check for derived classes for template classes
2112
exists(ClassTemplateInstantiation instantiation |
22-
exists(instantiation.getADerivedClass()) and c = instantiation.getTemplate()
13+
instantiation.getADerivedClass() = result and c = instantiation.getTemplate()
2314
)
24-
) and
25-
reason = "a derived class exists"
26-
or
27-
// The class must be extended at some point
28-
c.isAbstract() and reason = "the class is abstract"
15+
}
16+
17+
/**
18+
* A class that is used or intended to be used as a base class.
19+
*/
20+
class BaseClass extends Class {
21+
BaseClass() {
22+
exists(getADerivedClass(this))
23+
or
24+
this.isAbstract()
25+
}
26+
27+
// We don't override `getADerivedClass` because that introduces a non-monotonic recursion.
28+
Class getASubClass() {
29+
result = getADerivedClass(this)
30+
}
2931
}
3032

3133
/**

0 commit comments

Comments
 (0)