From 6266962d3e43cecc7bd6892b14cfab18c1107c6c Mon Sep 17 00:00:00 2001 From: Remco Vermeulen Date: Fri, 1 Mar 2024 16:16:53 -0800 Subject: [PATCH 1/7] Add support for templates to base class detection Detect template base classes through class template instantiations. --- .../CopyAndMoveNotDeclaredProtected.expected | 8 +++ cpp/autosar/test/rules/A12-8-6/test.cpp | 50 ++++++++++++++++++- cpp/common/src/codingstandards/cpp/Class.qll | 14 +++++- 3 files changed, 70 insertions(+), 2 deletions(-) diff --git a/cpp/autosar/test/rules/A12-8-6/CopyAndMoveNotDeclaredProtected.expected b/cpp/autosar/test/rules/A12-8-6/CopyAndMoveNotDeclaredProtected.expected index 39b98e0500..a5890f10c0 100644 --- a/cpp/autosar/test/rules/A12-8-6/CopyAndMoveNotDeclaredProtected.expected +++ b/cpp/autosar/test/rules/A12-8-6/CopyAndMoveNotDeclaredProtected.expected @@ -12,3 +12,11 @@ | 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. | | 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. | | 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. | +| test.cpp:85:3:85:12 | declaration of BaseClass7 | Copy constructor for base class BaseClass7 (a derived class exists) is not declared protected or deleted. | +| test.cpp:86:3:86:12 | declaration of BaseClass7 | Move constructor for base class BaseClass7 (a derived class exists) is not declared protected or deleted. | +| test.cpp:87:15:87:23 | declaration of operator= | Copy assignment operator for base class BaseClass7 (a derived class exists) is not declared protected or deleted. | +| test.cpp:88:15:88:23 | declaration of operator= | Move assignment operator for base class BaseClass7 (a derived class exists) is not declared protected or deleted. | +| 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. | +| 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. | +| 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. | +| 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. | diff --git a/cpp/autosar/test/rules/A12-8-6/test.cpp b/cpp/autosar/test/rules/A12-8-6/test.cpp index 2dc5425706..4ce65c2c51 100644 --- a/cpp/autosar/test/rules/A12-8-6/test.cpp +++ b/cpp/autosar/test/rules/A12-8-6/test.cpp @@ -77,4 +77,52 @@ class BaseClass6 { BaseClass6 &operator=(BaseClass6 const &) = default; // NON_COMPLIANT BaseClass6 &operator=(BaseClass6 &&) = default; // NON_COMPLIANT virtual void test() = 0; // pure virtual function, making this abstract -}; \ No newline at end of file +}; + +template class BaseClass7 { +public: + BaseClass7() {} + BaseClass7(BaseClass7 const &) = default; // NON_COMPLIANT + BaseClass7(BaseClass7 &&) = default; // NON_COMPLIANT + BaseClass7 &operator=(BaseClass7 const &) = default; // NON_COMPLIANT + BaseClass7 &operator=(BaseClass7 &&) = default; // NON_COMPLIANT + int operator=(int i); // COMPLIANT - not an assignment operator +}; // COMPLIANT + +template +class DerivedClass7 // COMPLIANT - not a base class itself + : public BaseClass7 { +public: + DerivedClass7() {} +}; + +class DerivedClass8 // COMPLIANT - not a base class itself + : public BaseClass7 { +public: + DerivedClass8() {} +}; + +class BaseClass8 { +public: + BaseClass8() {} + BaseClass8(BaseClass8 const &) = default; // NON_COMPLIANT + BaseClass8(BaseClass8 &&) = default; // NON_COMPLIANT + BaseClass8 &operator=(BaseClass8 const &) = default; // NON_COMPLIANT + BaseClass8 &operator=(BaseClass8 &&) = default; // NON_COMPLIANT +}; + +template +class DerivedClass9 // COMPLIANT - not a base class itself + : public BaseClass8 { +public: + DerivedClass9() {} + +private: + T t; +}; + +void test() { + BaseClass7 b; + DerivedClass7 d; + DerivedClass9 e; +} \ No newline at end of file diff --git a/cpp/common/src/codingstandards/cpp/Class.qll b/cpp/common/src/codingstandards/cpp/Class.qll index 2fff01c91f..f29e0990c0 100644 --- a/cpp/common/src/codingstandards/cpp/Class.qll +++ b/cpp/common/src/codingstandards/cpp/Class.qll @@ -10,7 +10,19 @@ import codingstandards.cpp.Expr */ predicate isPossibleBaseClass(Class c, string reason) { // There exists a derivation in this database - exists(c.getADerivedClass()) and reason = "a derived class exists" + ( + // We make a distinction between class template instantiations, regular classes and template classes. + // For template classes we do have derived classes, because derived classes would derive from a + // class template instantiation. + // Therefore, we check for derived classes for regular classes + not c instanceof ClassTemplateInstantiation and not c instanceof TemplateClass and exists(c.getADerivedClass()) + or + // and use template instantiations to check for derived classes for template classes + exists(ClassTemplateInstantiation instantiation | + exists(instantiation.getADerivedClass()) and c = instantiation.getTemplate() + ) + ) and + reason = "a derived class exists" or // The class must be extended at some point c.isAbstract() and reason = "the class is abstract" From 89910dd7b014633bfdaa59d1723194b296f8ab7b Mon Sep 17 00:00:00 2001 From: Remco Vermeulen Date: Fri, 1 Mar 2024 17:58:07 -0800 Subject: [PATCH 2/7] 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. --- .../DestructorOfABaseClassNotPublicVirtual.ql | 6 +-- .../CopyAndMoveNotDeclaredProtected.ql | 30 +++++++++---- ...uctorOfABaseClassNotPublicVirtual.expected | 4 +- .../CopyAndMoveNotDeclaredProtected.expected | 44 +++++++++---------- cpp/common/src/codingstandards/cpp/Class.qll | 38 ++++++++-------- 5 files changed, 69 insertions(+), 53 deletions(-) diff --git a/cpp/autosar/src/rules/A12-4-1/DestructorOfABaseClassNotPublicVirtual.ql b/cpp/autosar/src/rules/A12-4-1/DestructorOfABaseClassNotPublicVirtual.ql index 4743e0b529..c534f9e591 100644 --- a/cpp/autosar/src/rules/A12-4-1/DestructorOfABaseClassNotPublicVirtual.ql +++ b/cpp/autosar/src/rules/A12-4-1/DestructorOfABaseClassNotPublicVirtual.ql @@ -29,9 +29,9 @@ predicate isProtectedNonVirtual(Destructor d) { d.isProtected() and not d.isVirt from Destructor d where not isExcluded(d, VirtualFunctionsPackage::destructorOfABaseClassNotPublicVirtualQuery()) and - isPossibleBaseClass(d.getDeclaringType(), _) and + d.getDeclaringType() instanceof BaseClass and (not isPublicOverride(d) and not isProtectedNonVirtual(d) and not isPublicVirtual(d)) // Report the declaration entry in the class body, as that is where the access specifier should be set select getDeclarationEntryInClassDeclaration(d), - "Destructor of base class " + d.getDeclaringType() + - " is not declared as public virtual, public override, or protected non-virtual." + "Destructor of base class '" + d.getDeclaringType().getQualifiedName() + + "' is not declared as public virtual, public override, or protected non-virtual." diff --git a/cpp/autosar/src/rules/A12-8-6/CopyAndMoveNotDeclaredProtected.ql b/cpp/autosar/src/rules/A12-8-6/CopyAndMoveNotDeclaredProtected.ql index 0098c7e43f..2be35fbe59 100644 --- a/cpp/autosar/src/rules/A12-8-6/CopyAndMoveNotDeclaredProtected.ql +++ b/cpp/autosar/src/rules/A12-8-6/CopyAndMoveNotDeclaredProtected.ql @@ -20,23 +20,35 @@ predicate isInvalidConstructor(Constructor f, string constructorType) { not f.isDeleted() and not f.isProtected() and ( - f instanceof MoveConstructor and constructorType = "Move constructor" + f instanceof MoveConstructor and + if f.isCompilerGenerated() + then constructorType = "Implicit move constructor" + else constructorType = "Move constructor" or - f instanceof CopyConstructor and constructorType = "Copy constructor" + f instanceof CopyConstructor and + if f.isCompilerGenerated() + then constructorType = "Implicit copy constructor" + else constructorType = "Copy constructor" ) } predicate isInvalidAssignment(Operator f, string operatorType) { not f.isDeleted() and ( - f instanceof CopyAssignmentOperator and operatorType = "Copy assignment operator" + f instanceof MoveAssignmentOperator and + if f.isCompilerGenerated() + then operatorType = "Implicit move assignment operator" + else operatorType = "Move constructor" or - f instanceof MoveAssignmentOperator and operatorType = "Move assignment operator" + f instanceof CopyAssignmentOperator and + if f.isCompilerGenerated() + then operatorType = "Implicit copy assignment operator" + else operatorType = "Copy assignment operator" ) and not f.hasSpecifier("protected") } -from MemberFunction mf, string type, string baseReason +from BaseClass baseClass, MemberFunction mf, string type where not isExcluded(mf, OperatorsPackage::copyAndMoveNotDeclaredProtectedQuery()) and ( @@ -44,7 +56,9 @@ where or isInvalidAssignment(mf, type) ) and - isPossibleBaseClass(mf.getDeclaringType(), baseReason) + baseClass = mf.getDeclaringType() +// To avoid duplicate alerts due to inaccurate location information in the database we don't use the location of the base class. +// This for example happens if multiple copies of the same header file are present in the database. select getDeclarationEntryInClassDeclaration(mf), - type + " for base class " + mf.getDeclaringType().getQualifiedName() + " (" + baseReason + - ") is not declared protected or deleted." + type + " for base class '" + baseClass.getQualifiedName() + + "' is not declared protected or deleted." diff --git a/cpp/autosar/test/rules/A12-4-1/DestructorOfABaseClassNotPublicVirtual.expected b/cpp/autosar/test/rules/A12-4-1/DestructorOfABaseClassNotPublicVirtual.expected index f8900da92f..dbe7f89585 100644 --- a/cpp/autosar/test/rules/A12-4-1/DestructorOfABaseClassNotPublicVirtual.expected +++ b/cpp/autosar/test/rules/A12-4-1/DestructorOfABaseClassNotPublicVirtual.expected @@ -1,2 +1,2 @@ -| 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. | -| 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. | +| 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. | +| 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. | diff --git a/cpp/autosar/test/rules/A12-8-6/CopyAndMoveNotDeclaredProtected.expected b/cpp/autosar/test/rules/A12-8-6/CopyAndMoveNotDeclaredProtected.expected index a5890f10c0..abbe8728a6 100644 --- a/cpp/autosar/test/rules/A12-8-6/CopyAndMoveNotDeclaredProtected.expected +++ b/cpp/autosar/test/rules/A12-8-6/CopyAndMoveNotDeclaredProtected.expected @@ -1,22 +1,22 @@ -| 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. | -| 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. | -| 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. | -| 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. | -| 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. | -| 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. | -| 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. | -| 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. | -| 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. | -| 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. | -| 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. | -| 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. | -| 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. | -| 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. | -| test.cpp:85:3:85:12 | declaration of BaseClass7 | Copy constructor for base class BaseClass7 (a derived class exists) is not declared protected or deleted. | -| test.cpp:86:3:86:12 | declaration of BaseClass7 | Move constructor for base class BaseClass7 (a derived class exists) is not declared protected or deleted. | -| test.cpp:87:15:87:23 | declaration of operator= | Copy assignment operator for base class BaseClass7 (a derived class exists) is not declared protected or deleted. | -| test.cpp:88:15:88:23 | declaration of operator= | Move assignment operator for base class BaseClass7 (a derived class exists) is not declared protected or deleted. | -| 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. | -| 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. | -| 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. | -| 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. | +| test.cpp:4:3:4:12 | declaration of BaseClass1 | Copy constructor for base class 'BaseClass1' is not declared protected or deleted. | +| test.cpp:5:3:5:12 | declaration of BaseClass1 | Move constructor for base class 'BaseClass1' is not declared protected or deleted. | +| test.cpp:6:15:6:23 | declaration of operator= | Copy assignment operator for base class 'BaseClass1' is not declared protected or deleted. | +| test.cpp:7:15:7:23 | declaration of operator= | Move constructor for base class 'BaseClass1' is not declared protected or deleted. | +| test.cpp:15:7:15:7 | declaration of operator= | Implicit copy assignment operator for base class 'BaseClass2' is not declared protected or deleted. | +| test.cpp:15:7:15:7 | declaration of operator= | Implicit move assignment operator for base class 'BaseClass2' is not declared protected or deleted. | +| test.cpp:55:3:55:12 | declaration of BaseClass5 | Copy constructor for base class 'BaseClass5' is not declared protected or deleted. | +| test.cpp:56:3:56:12 | declaration of BaseClass5 | Move constructor for base class 'BaseClass5' is not declared protected or deleted. | +| test.cpp:57:15:57:23 | declaration of operator= | Copy assignment operator for base class 'BaseClass5' is not declared protected or deleted. | +| test.cpp:58:15:58:23 | declaration of operator= | Move constructor for base class 'BaseClass5' is not declared protected or deleted. | +| test.cpp:75:3:75:12 | declaration of BaseClass6 | Copy constructor for base class 'BaseClass6' is not declared protected or deleted. | +| test.cpp:76:3:76:12 | declaration of BaseClass6 | Move constructor for base class 'BaseClass6' is not declared protected or deleted. | +| test.cpp:77:15:77:23 | declaration of operator= | Copy assignment operator for base class 'BaseClass6' is not declared protected or deleted. | +| test.cpp:78:15:78:23 | declaration of operator= | Move constructor for base class 'BaseClass6' is not declared protected or deleted. | +| test.cpp:85:3:85:12 | declaration of BaseClass7 | Copy constructor for base class 'BaseClass7' is not declared protected or deleted. | +| test.cpp:86:3:86:12 | declaration of BaseClass7 | Move constructor for base class 'BaseClass7' is not declared protected or deleted. | +| test.cpp:87:15:87:23 | declaration of operator= | Copy assignment operator for base class 'BaseClass7' is not declared protected or deleted. | +| test.cpp:88:15:88:23 | declaration of operator= | Move constructor for base class 'BaseClass7' is not declared protected or deleted. | +| test.cpp:108:3:108:12 | declaration of BaseClass8 | Copy constructor for base class 'BaseClass8' is not declared protected or deleted. | +| test.cpp:109:3:109:12 | declaration of BaseClass8 | Move constructor for base class 'BaseClass8' is not declared protected or deleted. | +| test.cpp:110:15:110:23 | declaration of operator= | Copy assignment operator for base class 'BaseClass8' is not declared protected or deleted. | +| test.cpp:111:15:111:23 | declaration of operator= | Move constructor for base class 'BaseClass8' is not declared protected or deleted. | diff --git a/cpp/common/src/codingstandards/cpp/Class.qll b/cpp/common/src/codingstandards/cpp/Class.qll index f29e0990c0..be775e99f7 100644 --- a/cpp/common/src/codingstandards/cpp/Class.qll +++ b/cpp/common/src/codingstandards/cpp/Class.qll @@ -5,27 +5,29 @@ import cpp import codingstandards.cpp.Expr -/** - * Holds if we believe that `c` is used or intended to be used as a base class. - */ -predicate isPossibleBaseClass(Class c, string reason) { - // There exists a derivation in this database - ( - // We make a distinction between class template instantiations, regular classes and template classes. - // For template classes we do have derived classes, because derived classes would derive from a - // class template instantiation. - // Therefore, we check for derived classes for regular classes - not c instanceof ClassTemplateInstantiation and not c instanceof TemplateClass and exists(c.getADerivedClass()) + +private Class getADerivedClass(Class c) { + not c instanceof ClassTemplateInstantiation and not c instanceof TemplateClass and result = c.getADerivedClass() or - // and use template instantiations to check for derived classes for template classes exists(ClassTemplateInstantiation instantiation | - exists(instantiation.getADerivedClass()) and c = instantiation.getTemplate() + instantiation.getADerivedClass() = result and c = instantiation.getTemplate() ) - ) and - reason = "a derived class exists" - or - // The class must be extended at some point - c.isAbstract() and reason = "the class is abstract" +} + +/** + * A class that is used or intended to be used as a base class. + */ +class BaseClass extends Class { + BaseClass() { + exists(getADerivedClass(this)) + or + this.isAbstract() + } + + // We don't override `getADerivedClass` because that introduces a non-monotonic recursion. + Class getASubClass() { + result = getADerivedClass(this) + } } /** From 5c411fe2b94ec701ce6854390ddf22e5b4cb6989 Mon Sep 17 00:00:00 2001 From: Remco Vermeulen Date: Fri, 1 Mar 2024 18:07:13 -0800 Subject: [PATCH 3/7] Add changenote --- change_notes/2024-03-01-fix-fp-a12-4-1-and-a12-8-6.md | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 change_notes/2024-03-01-fix-fp-a12-4-1-and-a12-8-6.md diff --git a/change_notes/2024-03-01-fix-fp-a12-4-1-and-a12-8-6.md b/change_notes/2024-03-01-fix-fp-a12-4-1-and-a12-8-6.md new file mode 100644 index 0000000000..7ba99b44f1 --- /dev/null +++ b/change_notes/2024-03-01-fix-fp-a12-4-1-and-a12-8-6.md @@ -0,0 +1,6 @@ +- `A12-4-1` - `DestructorOfABaseClassNotPublicVirtual.ql`: + - Fix FP reported in #392. Improve base class detection for template classes. + - Update the alert message to prevent duplicate alerts for base classes that are both derived and abstract. +- `A12-8-6` - `CopyAndMoveNotDeclaredProtected.ql`: + - Fix FP reported in #392. Improve base class detection for template classes. + - Update the alert message to prevent duplicate alerts for base classes that are both derived and abstract. From de82e2287ebe4d00e1da471b4059bd2d4761c941 Mon Sep 17 00:00:00 2001 From: Remco Vermeulen Date: Mon, 4 Mar 2024 14:53:01 -0800 Subject: [PATCH 4/7] Format module --- cpp/common/src/codingstandards/cpp/Class.qll | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/cpp/common/src/codingstandards/cpp/Class.qll b/cpp/common/src/codingstandards/cpp/Class.qll index be775e99f7..b967ae6365 100644 --- a/cpp/common/src/codingstandards/cpp/Class.qll +++ b/cpp/common/src/codingstandards/cpp/Class.qll @@ -5,13 +5,14 @@ import cpp import codingstandards.cpp.Expr - private Class getADerivedClass(Class c) { - not c instanceof ClassTemplateInstantiation and not c instanceof TemplateClass and result = c.getADerivedClass() - or - exists(ClassTemplateInstantiation instantiation | - instantiation.getADerivedClass() = result and c = instantiation.getTemplate() - ) + not c instanceof ClassTemplateInstantiation and + not c instanceof TemplateClass and + result = c.getADerivedClass() + or + exists(ClassTemplateInstantiation instantiation | + instantiation.getADerivedClass() = result and c = instantiation.getTemplate() + ) } /** @@ -24,10 +25,8 @@ class BaseClass extends Class { this.isAbstract() } - // We don't override `getADerivedClass` because that introduces a non-monotonic recursion. - Class getASubClass() { - result = getADerivedClass(this) - } + // We don't override `getADerivedClass` because that introduces a non-monotonic recursion. + Class getASubClass() { result = getADerivedClass(this) } } /** From 945d67a3957819100971a6309a7489fe3238a3c2 Mon Sep 17 00:00:00 2001 From: Remco Vermeulen Date: Thu, 14 Mar 2024 15:42:00 -0700 Subject: [PATCH 5/7] Add support for template base class with compiler generated copy and move constructors/operators --- .../CopyAndMoveNotDeclaredProtected.expected | 4 ++++ cpp/autosar/test/rules/A12-8-6/test.cpp | 15 +++++++++++++++ cpp/common/src/codingstandards/cpp/Class.qll | 2 -- 3 files changed, 19 insertions(+), 2 deletions(-) diff --git a/cpp/autosar/test/rules/A12-8-6/CopyAndMoveNotDeclaredProtected.expected b/cpp/autosar/test/rules/A12-8-6/CopyAndMoveNotDeclaredProtected.expected index abbe8728a6..42e5cc0946 100644 --- a/cpp/autosar/test/rules/A12-8-6/CopyAndMoveNotDeclaredProtected.expected +++ b/cpp/autosar/test/rules/A12-8-6/CopyAndMoveNotDeclaredProtected.expected @@ -20,3 +20,7 @@ | test.cpp:109:3:109:12 | declaration of BaseClass8 | Move constructor for base class 'BaseClass8' is not declared protected or deleted. | | test.cpp:110:15:110:23 | declaration of operator= | Copy assignment operator for base class 'BaseClass8' is not declared protected or deleted. | | test.cpp:111:15:111:23 | declaration of operator= | Move constructor for base class 'BaseClass8' is not declared protected or deleted. | +| test.cpp:124:26:124:26 | declaration of BaseClass9 | Implicit copy constructor for base class 'BaseClass9' is not declared protected or deleted. | +| test.cpp:124:26:124:26 | declaration of BaseClass9 | Implicit move constructor for base class 'BaseClass9' is not declared protected or deleted. | +| test.cpp:124:26:124:26 | declaration of operator= | Implicit copy assignment operator for base class 'BaseClass9' is not declared protected or deleted. | +| test.cpp:124:26:124:26 | declaration of operator= | Implicit move assignment operator for base class 'BaseClass9' is not declared protected or deleted. | diff --git a/cpp/autosar/test/rules/A12-8-6/test.cpp b/cpp/autosar/test/rules/A12-8-6/test.cpp index 4ce65c2c51..6a31ca60ae 100644 --- a/cpp/autosar/test/rules/A12-8-6/test.cpp +++ b/cpp/autosar/test/rules/A12-8-6/test.cpp @@ -121,8 +121,23 @@ class DerivedClass9 // COMPLIANT - not a base class itself T t; }; +template class BaseClass9 { // NON_COMPLIANT + +public: + BaseClass9() {} +}; + +template +class DerivedClass10 // COMPLIANT - not a base class itself + : public BaseClass9 { +public: + DerivedClass10() {} +}; + void test() { BaseClass7 b; DerivedClass7 d; DerivedClass9 e; + BaseClass9 f; + DerivedClass10 g; } \ No newline at end of file diff --git a/cpp/common/src/codingstandards/cpp/Class.qll b/cpp/common/src/codingstandards/cpp/Class.qll index b967ae6365..19bec9fa5f 100644 --- a/cpp/common/src/codingstandards/cpp/Class.qll +++ b/cpp/common/src/codingstandards/cpp/Class.qll @@ -6,8 +6,6 @@ import cpp import codingstandards.cpp.Expr private Class getADerivedClass(Class c) { - not c instanceof ClassTemplateInstantiation and - not c instanceof TemplateClass and result = c.getADerivedClass() or exists(ClassTemplateInstantiation instantiation | From 1d1e41c31e40440c220b59f330100acf98b8851b Mon Sep 17 00:00:00 2001 From: Remco Vermeulen Date: Thu, 14 Mar 2024 16:02:56 -0700 Subject: [PATCH 6/7] Improve precision alert message We know explicitly mention the missing actions in the alert message. --- .../CopyAndMoveNotDeclaredProtected.ql | 28 +++++----- .../CopyAndMoveNotDeclaredProtected.expected | 52 +++++++++---------- 2 files changed, 40 insertions(+), 40 deletions(-) diff --git a/cpp/autosar/src/rules/A12-8-6/CopyAndMoveNotDeclaredProtected.ql b/cpp/autosar/src/rules/A12-8-6/CopyAndMoveNotDeclaredProtected.ql index 2be35fbe59..5200485511 100644 --- a/cpp/autosar/src/rules/A12-8-6/CopyAndMoveNotDeclaredProtected.ql +++ b/cpp/autosar/src/rules/A12-8-6/CopyAndMoveNotDeclaredProtected.ql @@ -16,49 +16,49 @@ import cpp import codingstandards.cpp.autosar import codingstandards.cpp.Class -predicate isInvalidConstructor(Constructor f, string constructorType) { +predicate isInvalidConstructor(Constructor f, string constructorType, string missingAction) { not f.isDeleted() and not f.isProtected() and ( f instanceof MoveConstructor and if f.isCompilerGenerated() - then constructorType = "Implicit move constructor" - else constructorType = "Move constructor" + then constructorType = "Implicit move constructor" and missingAction = "deleted" + else (constructorType = "Move constructor" and missingAction = "protected") or f instanceof CopyConstructor and if f.isCompilerGenerated() - then constructorType = "Implicit copy constructor" - else constructorType = "Copy constructor" + then constructorType = "Implicit copy constructor" and missingAction = "deleted" + else (constructorType = "Copy constructor" and missingAction = "protected") ) } -predicate isInvalidAssignment(Operator f, string operatorType) { +predicate isInvalidAssignment(Operator f, string operatorType, string missingAction) { not f.isDeleted() and ( f instanceof MoveAssignmentOperator and if f.isCompilerGenerated() - then operatorType = "Implicit move assignment operator" - else operatorType = "Move constructor" + then operatorType = "Implicit move assignment operator" and missingAction = "deleted" + else (operatorType = "Move assignment operator" and missingAction = "protected") or f instanceof CopyAssignmentOperator and if f.isCompilerGenerated() - then operatorType = "Implicit copy assignment operator" - else operatorType = "Copy assignment operator" + then operatorType = "Implicit copy assignment operator" and missingAction = "deleted" + else (operatorType = "Copy assignment operator" and missingAction = "protected") ) and not f.hasSpecifier("protected") } -from BaseClass baseClass, MemberFunction mf, string type +from BaseClass baseClass, MemberFunction mf, string type, string missingAction where not isExcluded(mf, OperatorsPackage::copyAndMoveNotDeclaredProtectedQuery()) and ( - isInvalidConstructor(mf, type) + isInvalidConstructor(mf, type, missingAction) or - isInvalidAssignment(mf, type) + isInvalidAssignment(mf, type, missingAction) ) and baseClass = mf.getDeclaringType() // To avoid duplicate alerts due to inaccurate location information in the database we don't use the location of the base class. // This for example happens if multiple copies of the same header file are present in the database. select getDeclarationEntryInClassDeclaration(mf), type + " for base class '" + baseClass.getQualifiedName() + - "' is not declared protected or deleted." + "' is not declared "+ missingAction +"." diff --git a/cpp/autosar/test/rules/A12-8-6/CopyAndMoveNotDeclaredProtected.expected b/cpp/autosar/test/rules/A12-8-6/CopyAndMoveNotDeclaredProtected.expected index 42e5cc0946..9f85da12d6 100644 --- a/cpp/autosar/test/rules/A12-8-6/CopyAndMoveNotDeclaredProtected.expected +++ b/cpp/autosar/test/rules/A12-8-6/CopyAndMoveNotDeclaredProtected.expected @@ -1,26 +1,26 @@ -| test.cpp:4:3:4:12 | declaration of BaseClass1 | Copy constructor for base class 'BaseClass1' is not declared protected or deleted. | -| test.cpp:5:3:5:12 | declaration of BaseClass1 | Move constructor for base class 'BaseClass1' is not declared protected or deleted. | -| test.cpp:6:15:6:23 | declaration of operator= | Copy assignment operator for base class 'BaseClass1' is not declared protected or deleted. | -| test.cpp:7:15:7:23 | declaration of operator= | Move constructor for base class 'BaseClass1' is not declared protected or deleted. | -| test.cpp:15:7:15:7 | declaration of operator= | Implicit copy assignment operator for base class 'BaseClass2' is not declared protected or deleted. | -| test.cpp:15:7:15:7 | declaration of operator= | Implicit move assignment operator for base class 'BaseClass2' is not declared protected or deleted. | -| test.cpp:55:3:55:12 | declaration of BaseClass5 | Copy constructor for base class 'BaseClass5' is not declared protected or deleted. | -| test.cpp:56:3:56:12 | declaration of BaseClass5 | Move constructor for base class 'BaseClass5' is not declared protected or deleted. | -| test.cpp:57:15:57:23 | declaration of operator= | Copy assignment operator for base class 'BaseClass5' is not declared protected or deleted. | -| test.cpp:58:15:58:23 | declaration of operator= | Move constructor for base class 'BaseClass5' is not declared protected or deleted. | -| test.cpp:75:3:75:12 | declaration of BaseClass6 | Copy constructor for base class 'BaseClass6' is not declared protected or deleted. | -| test.cpp:76:3:76:12 | declaration of BaseClass6 | Move constructor for base class 'BaseClass6' is not declared protected or deleted. | -| test.cpp:77:15:77:23 | declaration of operator= | Copy assignment operator for base class 'BaseClass6' is not declared protected or deleted. | -| test.cpp:78:15:78:23 | declaration of operator= | Move constructor for base class 'BaseClass6' is not declared protected or deleted. | -| test.cpp:85:3:85:12 | declaration of BaseClass7 | Copy constructor for base class 'BaseClass7' is not declared protected or deleted. | -| test.cpp:86:3:86:12 | declaration of BaseClass7 | Move constructor for base class 'BaseClass7' is not declared protected or deleted. | -| test.cpp:87:15:87:23 | declaration of operator= | Copy assignment operator for base class 'BaseClass7' is not declared protected or deleted. | -| test.cpp:88:15:88:23 | declaration of operator= | Move constructor for base class 'BaseClass7' is not declared protected or deleted. | -| test.cpp:108:3:108:12 | declaration of BaseClass8 | Copy constructor for base class 'BaseClass8' is not declared protected or deleted. | -| test.cpp:109:3:109:12 | declaration of BaseClass8 | Move constructor for base class 'BaseClass8' is not declared protected or deleted. | -| test.cpp:110:15:110:23 | declaration of operator= | Copy assignment operator for base class 'BaseClass8' is not declared protected or deleted. | -| test.cpp:111:15:111:23 | declaration of operator= | Move constructor for base class 'BaseClass8' is not declared protected or deleted. | -| test.cpp:124:26:124:26 | declaration of BaseClass9 | Implicit copy constructor for base class 'BaseClass9' is not declared protected or deleted. | -| test.cpp:124:26:124:26 | declaration of BaseClass9 | Implicit move constructor for base class 'BaseClass9' is not declared protected or deleted. | -| test.cpp:124:26:124:26 | declaration of operator= | Implicit copy assignment operator for base class 'BaseClass9' is not declared protected or deleted. | -| test.cpp:124:26:124:26 | declaration of operator= | Implicit move assignment operator for base class 'BaseClass9' is not declared protected or deleted. | +| test.cpp:4:3:4:12 | declaration of BaseClass1 | Copy constructor for base class 'BaseClass1' is not declared protected. | +| test.cpp:5:3:5:12 | declaration of BaseClass1 | Move constructor for base class 'BaseClass1' is not declared protected. | +| test.cpp:6:15:6:23 | declaration of operator= | Copy assignment operator for base class 'BaseClass1' is not declared protected. | +| test.cpp:7:15:7:23 | declaration of operator= | Move assignment operator for base class 'BaseClass1' is not declared protected. | +| test.cpp:15:7:15:7 | declaration of operator= | Implicit copy assignment operator for base class 'BaseClass2' is not declared deleted. | +| test.cpp:15:7:15:7 | declaration of operator= | Implicit move assignment operator for base class 'BaseClass2' is not declared deleted. | +| test.cpp:55:3:55:12 | declaration of BaseClass5 | Copy constructor for base class 'BaseClass5' is not declared protected. | +| test.cpp:56:3:56:12 | declaration of BaseClass5 | Move constructor for base class 'BaseClass5' is not declared protected. | +| test.cpp:57:15:57:23 | declaration of operator= | Copy assignment operator for base class 'BaseClass5' is not declared protected. | +| test.cpp:58:15:58:23 | declaration of operator= | Move assignment operator for base class 'BaseClass5' is not declared protected. | +| test.cpp:75:3:75:12 | declaration of BaseClass6 | Copy constructor for base class 'BaseClass6' is not declared protected. | +| test.cpp:76:3:76:12 | declaration of BaseClass6 | Move constructor for base class 'BaseClass6' is not declared protected. | +| test.cpp:77:15:77:23 | declaration of operator= | Copy assignment operator for base class 'BaseClass6' is not declared protected. | +| test.cpp:78:15:78:23 | declaration of operator= | Move assignment operator for base class 'BaseClass6' is not declared protected. | +| test.cpp:85:3:85:12 | declaration of BaseClass7 | Copy constructor for base class 'BaseClass7' is not declared protected. | +| test.cpp:86:3:86:12 | declaration of BaseClass7 | Move constructor for base class 'BaseClass7' is not declared protected. | +| test.cpp:87:15:87:23 | declaration of operator= | Copy assignment operator for base class 'BaseClass7' is not declared protected. | +| test.cpp:88:15:88:23 | declaration of operator= | Move assignment operator for base class 'BaseClass7' is not declared protected. | +| test.cpp:108:3:108:12 | declaration of BaseClass8 | Copy constructor for base class 'BaseClass8' is not declared protected. | +| test.cpp:109:3:109:12 | declaration of BaseClass8 | Move constructor for base class 'BaseClass8' is not declared protected. | +| test.cpp:110:15:110:23 | declaration of operator= | Copy assignment operator for base class 'BaseClass8' is not declared protected. | +| test.cpp:111:15:111:23 | declaration of operator= | Move assignment operator for base class 'BaseClass8' is not declared protected. | +| test.cpp:124:26:124:26 | declaration of BaseClass9 | Implicit copy constructor for base class 'BaseClass9' is not declared deleted. | +| test.cpp:124:26:124:26 | declaration of BaseClass9 | Implicit move constructor for base class 'BaseClass9' is not declared deleted. | +| test.cpp:124:26:124:26 | declaration of operator= | Implicit copy assignment operator for base class 'BaseClass9' is not declared deleted. | +| test.cpp:124:26:124:26 | declaration of operator= | Implicit move assignment operator for base class 'BaseClass9' is not declared deleted. | From b214af4b6d8caaaf05ee8c4e000cecf6987e3908 Mon Sep 17 00:00:00 2001 From: Remco Vermeulen Date: Fri, 15 Mar 2024 08:51:40 -0700 Subject: [PATCH 7/7] Format query --- .../CopyAndMoveNotDeclaredProtected.ql | 20 +++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/cpp/autosar/src/rules/A12-8-6/CopyAndMoveNotDeclaredProtected.ql b/cpp/autosar/src/rules/A12-8-6/CopyAndMoveNotDeclaredProtected.ql index 5200485511..7507eb1d7c 100644 --- a/cpp/autosar/src/rules/A12-8-6/CopyAndMoveNotDeclaredProtected.ql +++ b/cpp/autosar/src/rules/A12-8-6/CopyAndMoveNotDeclaredProtected.ql @@ -23,12 +23,16 @@ predicate isInvalidConstructor(Constructor f, string constructorType, string mis f instanceof MoveConstructor and if f.isCompilerGenerated() then constructorType = "Implicit move constructor" and missingAction = "deleted" - else (constructorType = "Move constructor" and missingAction = "protected") + else ( + constructorType = "Move constructor" and missingAction = "protected" + ) or f instanceof CopyConstructor and if f.isCompilerGenerated() then constructorType = "Implicit copy constructor" and missingAction = "deleted" - else (constructorType = "Copy constructor" and missingAction = "protected") + else ( + constructorType = "Copy constructor" and missingAction = "protected" + ) ) } @@ -38,12 +42,16 @@ predicate isInvalidAssignment(Operator f, string operatorType, string missingAct f instanceof MoveAssignmentOperator and if f.isCompilerGenerated() then operatorType = "Implicit move assignment operator" and missingAction = "deleted" - else (operatorType = "Move assignment operator" and missingAction = "protected") + else ( + operatorType = "Move assignment operator" and missingAction = "protected" + ) or f instanceof CopyAssignmentOperator and if f.isCompilerGenerated() then operatorType = "Implicit copy assignment operator" and missingAction = "deleted" - else (operatorType = "Copy assignment operator" and missingAction = "protected") + else ( + operatorType = "Copy assignment operator" and missingAction = "protected" + ) ) and not f.hasSpecifier("protected") } @@ -60,5 +68,5 @@ where // To avoid duplicate alerts due to inaccurate location information in the database we don't use the location of the base class. // This for example happens if multiple copies of the same header file are present in the database. select getDeclarationEntryInClassDeclaration(mf), - type + " for base class '" + baseClass.getQualifiedName() + - "' is not declared "+ missingAction +"." + type + " for base class '" + baseClass.getQualifiedName() + "' is not declared " + missingAction + + "."