Merge branch 'main' into lcartey/exceptions-exclude-noexcept

rvermeulen · web-flow · commit 793f9a7dab8b · 2024-02-07T16:18:55.000-08:00
diff --git a/change_notes/2023-11-22-remove-parameters-a7-1-1.md b/change_notes/2023-11-22-remove-parameters-a7-1-1.md
@@ -0,0 +1 @@
+ * `A7-1-1` - no longer report parameters as contravening this rule. This is inline with the rule intent as described in the referenced C++ Core Guidelines rule [CON.1](https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#con1-by-default-make-objects-immutable), which states "To avoid confusion and lots of false positives, don’t enforce this rule for function parameters."
diff --git a/change_notes/2024-01-25-exclusion-m9-3-3.md b/change_notes/2024-01-25-exclusion-m9-3-3.md
@@ -0,0 +1,2 @@
+`M9-3-3` - `MemberFunctionConstIfPossible.ql`, `MemberFunctionStaticIfPossible.ql`:
+    - Fixes #413. Exclude deleted member functions.
diff --git a/change_notes/2024-01-25-fix-reported-fp-for-a8-4-7.md b/change_notes/2024-01-25-fix-reported-fp-for-a8-4-7.md
@@ -0,0 +1,3 @@
+`A8-4-7` - `InParametersForCheapToCopyTypesNotPassedByValue.ql`, `InParametersForNotCheapToCopyTypesNotPassedByReference.ql`:
+    - Fixes #397. Exclude user defined operators and move constructors.`
+    - Exclude parameters for instantiated templates because the declaration ___location of the function does not contain enough information about the type used in the instantiation to make an actionable alert. 
diff --git a/change_notes/2024-01-26-exclusion-a5-0-2.md b/change_notes/2024-01-26-exclusion-a5-0-2.md
@@ -0,0 +1,2 @@
+`A5-0-2` - `NonBooleanIfStmt.qll`, `NonBooleanIterationStmt.qll`:
+    - Exclude compiler generated conditions.
diff --git a/change_notes/2024-01-31-fix-fp-m9-3-3.md b/change_notes/2024-01-31-fix-fp-m9-3-3.md
@@ -0,0 +1,2 @@
+`M9-3-3`: `MemberFunctionConstIfPossible.ql`:
+    - Fix FP reported in 467. Excluding candidates in uninstantiated templates.
diff --git a/cpp/autosar/src/rules/A7-1-1/DeclarationUnmodifiedParamMissingConstSpecifier.ql b/cpp/autosar/src/rules/A7-1-1/DeclarationUnmodifiedParamMissingConstSpecifier.ql
diff --git a/cpp/autosar/src/rules/A8-4-7/InParametersForCheapToCopyTypesNotPassedByValue.ql b/cpp/autosar/src/rules/A8-4-7/InParametersForCheapToCopyTypesNotPassedByValue.ql
@@ -17,6 +17,7 @@ import cpp
 import codingstandards.cpp.autosar
 import TriviallySmallType
 import codingstandards.cpp.CommonTypes as CommonTypes
+import codingstandards.cpp.Class
 
 /*
  * For the purposes of this rule, "cheap to copy" is defined as a trivially copyable type that is no
@@ -34,8 +35,10 @@ where
   ) and
   t.isConst() and
   not exists(CatchBlock cb | cb.getParameter() = v) and
-  not exists(CopyConstructor cc | cc.getAParameter() = v) and
-  not v.isFromUninstantiatedTemplate(_)
+  not exists(SpecialMemberFunction cc | cc.getAParameter() = v) and
+  not exists(Operator op | op.getAParameter() = v) and
+  not v.isFromUninstantiatedTemplate(_) and
+  not v.isFromTemplateInstantiation(_)
 select v,
-  "Parameter " + v.getName() + " is the trivially copyable type " + t.getName() +
-    " but it is passed by reference instead of by value."
+  "Parameter '" + v.getName() + "' is the trivially copyable type '" + t.getName() +
+    "' but it is passed by reference instead of by value."
diff --git a/cpp/autosar/src/rules/A8-4-7/InParametersForNotCheapToCopyTypesNotPassedByReference.ql b/cpp/autosar/src/rules/A8-4-7/InParametersForNotCheapToCopyTypesNotPassedByReference.ql
@@ -31,7 +31,8 @@ where
   not v.getType() instanceof TriviallySmallType and
   not v.getType().getUnderlyingType() instanceof ReferenceType and
   not exists(CatchBlock cb | cb.getParameter() = v) and
-  not v.isFromUninstantiatedTemplate(_)
+  not v.isFromUninstantiatedTemplate(_) and
+  not v.isFromTemplateInstantiation(_)
 select v,
   "Parameter " + v.getName() +
     " is the trivially non-copyable type $@ but it is passed by value instead of by reference.",
diff --git a/cpp/autosar/src/rules/M9-3-3/MemberFunctionConstIfPossible.ql b/cpp/autosar/src/rules/M9-3-3/MemberFunctionConstIfPossible.ql
@@ -54,7 +54,10 @@ class ConstMemberFunctionCandidate extends NonConstMemberFunction {
     not this instanceof Destructor and
     not this instanceof Operator and
     //less interested in MemberFunctions with no definition
-    this.hasDefinition()
+    this.hasDefinition() and
+    // For uninstantiated templates we have only partial information that prevents us from determining
+    // if the candidate calls non-const functions. Therefore we exclude these.
+    not this.isFromUninstantiatedTemplate(_)
   }
 
   /**
@@ -121,5 +124,6 @@ where
   not f.callsNonConstOwnMember() and
   not f.callsNonConstFromMemberVariable() and
   not f.isOverride() and
-  not f.isFinal()
+  not f.isFinal() and
+  not f.isDeleted()
 select f, "Member function can be declared as const."
diff --git a/cpp/autosar/src/rules/M9-3-3/MemberFunctionStaticIfPossible.ql b/cpp/autosar/src/rules/M9-3-3/MemberFunctionStaticIfPossible.ql
@@ -39,5 +39,6 @@ from NonStaticMemberFunction nonstatic
 where
   not isExcluded(nonstatic, ConstPackage::memberFunctionStaticIfPossibleQuery()) and
   not exists(ThisExpr t | t.getEnclosingFunction() = nonstatic) and
-  not nonstatic.isVirtual()
+  not nonstatic.isVirtual() and
+  not nonstatic.isDeleted()
 select nonstatic, "Member function can be declared as static."
diff --git a/cpp/autosar/test/rules/A7-1-1/DeclarationUnmodifiedParamMissingConstSpecifier.expected b/cpp/autosar/test/rules/A7-1-1/DeclarationUnmodifiedParamMissingConstSpecifier.expected
diff --git a/cpp/autosar/test/rules/A7-1-1/DeclarationUnmodifiedParamMissingConstSpecifier.qlref b/cpp/autosar/test/rules/A7-1-1/DeclarationUnmodifiedParamMissingConstSpecifier.qlref
diff --git a/cpp/autosar/test/rules/A7-1-1/test.cpp b/cpp/autosar/test/rules/A7-1-1/test.cpp
@@ -4,7 +4,7 @@
 void f1(int *p) { // COMPLIANT
   *p += 2;
 }
-void f2(int *p) { // NON_COMPLIANT
+void f2(int *p) { // COMPLIANT - we ignore parameters for this rule
   int l4 = 1;     // NON_COMPLIANT
   int *p1 = p;    // NON_COMPLIANT
 }
diff --git a/cpp/autosar/test/rules/A8-4-7/InParametersForCheapToCopyTypesNotPassedByValue.expected b/cpp/autosar/test/rules/A8-4-7/InParametersForCheapToCopyTypesNotPassedByValue.expected
@@ -1 +1 @@
-| test.cpp:20:19:20:21 | f5a | Parameter f5a is the trivially copyable type const S1 but it is passed by reference instead of by value. |
+| test.cpp:20:19:20:21 | f5a | Parameter 'f5a' is the trivially copyable type 'const S1' but it is passed by reference instead of by value. |
diff --git a/cpp/autosar/test/rules/A8-4-7/test.cpp b/cpp/autosar/test/rules/A8-4-7/test.cpp
@@ -37,4 +37,12 @@ inline S1 Value(size_t n, const char *data) {} // COMPLIANT
 struct A {
   int n;
   A(const A &a) : n(a.n) {} // COMPLIANT user-defined copy ctor
+  A(const A &&other_a);     // COMPLIANT user-defined move ctor
 };
+
+class C1 {};
+
+class C2 : public C1 {
+public:
+  C2 &operator=(const C2 &); // COMPLIANT
+};
diff --git a/cpp/autosar/test/rules/M9-3-3/test.cpp b/cpp/autosar/test/rules/M9-3-3/test.cpp
@@ -161,3 +161,35 @@ class Z22 : Z1 {
   void f2() final {}                   // COMPLIANT
   void f3() { this->a = 100; }         // COMPLIANT
 };
+
+template <class T> class Array {
+public:
+  T &back();
+
+private:
+  T data[128];
+  unsigned int size;
+};
+
+template <class T, template <class...> class U> class Stack {
+public:
+  T &Top() {
+    return this->data.back();
+  } // COMPLIANT[FALSE_NEGATIVE|TRUE_NEGATIVE] - exception not specified in the
+    // standard, we opt to not raise an issue because the template can be both
+    // compliant and non-compliant depending on instantiations.
+private:
+  U<T> data;
+};
+
+using IntVectorStack = Stack<int, Array>;
+
+void test_template() {
+  IntVectorStack s;
+
+  int i = s.Top();
+}
+
+class Z3 {
+  void f(int) = delete; // COMPLIANT
+};
diff --git a/cpp/common/src/codingstandards/cpp/exclusions/cpp/Const.qll b/cpp/common/src/codingstandards/cpp/exclusions/cpp/Const.qll
@@ -5,7 +5,6 @@ import codingstandards.cpp.exclusions.RuleMetadata
 
 newtype ConstQuery =
   TRemoveConstOrVolatileQualificationAutosarQuery() or
-  TDeclarationUnmodifiedParamMissingConstSpecifierQuery() or
   TDeclarationUnmodifiedObjectMissingConstSpecifierQuery() or
   TVariableMissingConstexprQuery() or
   TFunctionMissingConstexprQuery() or
@@ -28,15 +27,6 @@ predicate isConstQueryMetadata(Query query, string queryId, string ruleId, strin
   ruleId = "A5-2-3" and
   category = "required"
   or
-  query =
-    // `Query` instance for the `declarationUnmodifiedParamMissingConstSpecifier` query
-    ConstPackage::declarationUnmodifiedParamMissingConstSpecifierQuery() and
-  queryId =
-    // `@id` for the `declarationUnmodifiedParamMissingConstSpecifier` query
-    "cpp/autosar/declaration-unmodified-param-missing-const-specifier" and
-  ruleId = "A7-1-1" and
-  category = "required"
-  or
   query =
     // `Query` instance for the `declarationUnmodifiedObjectMissingConstSpecifier` query
     ConstPackage::declarationUnmodifiedObjectMissingConstSpecifierQuery() and
@@ -145,13 +135,6 @@ module ConstPackage {
       TQueryCPP(TConstPackageQuery(TRemoveConstOrVolatileQualificationAutosarQuery()))
   }
 
-  Query declarationUnmodifiedParamMissingConstSpecifierQuery() {
-    //autogenerate `Query` type
-    result =
-      // `Query` type for `declarationUnmodifiedParamMissingConstSpecifier` query
-      TQueryCPP(TConstPackageQuery(TDeclarationUnmodifiedParamMissingConstSpecifierQuery()))
-  }
-
   Query declarationUnmodifiedObjectMissingConstSpecifierQuery() {
     //autogenerate `Query` type
     result =
diff --git a/cpp/common/src/codingstandards/cpp/rules/nonbooleanifstmt/NonBooleanIfStmt.qll b/cpp/common/src/codingstandards/cpp/rules/nonbooleanifstmt/NonBooleanIfStmt.qll
@@ -14,6 +14,8 @@ query predicate problems(Expr condition, string message) {
   not isExcluded(condition, getQuery()) and
   exists(IfStmt ifStmt, Type explicitConversionType |
     condition = ifStmt.getCondition() and
+    //exclude any generated conditions
+    not condition.isCompilerGenerated() and
     not ifStmt.isFromUninstantiatedTemplate(_) and
     explicitConversionType = condition.getExplicitlyConverted().getUnderlyingType() and
     not explicitConversionType instanceof BoolType and
diff --git a/cpp/common/src/codingstandards/cpp/rules/nonbooleaniterationstmt/NonBooleanIterationStmt.qll b/cpp/common/src/codingstandards/cpp/rules/nonbooleaniterationstmt/NonBooleanIterationStmt.qll
@@ -16,6 +16,8 @@ query predicate problems(Loop loopStmt, string message) {
     condition = loopStmt.getCondition() and
     explicitConversionType = condition.getExplicitlyConverted().getType().getUnspecifiedType() and
     not explicitConversionType instanceof BoolType and
+    //exclude any generated conditions
+    not condition.isCompilerGenerated() and
     message = "Iteration condition has non boolean type " + explicitConversionType + "."
   )
 }
diff --git a/rule_packages/cpp/Const.json b/rule_packages/cpp/Const.json
@@ -33,19 +33,6 @@
         "obligation": "required"
       },
       "queries": [
-        {
-          "description": "`Constexpr`/`const` specifiers prevent unintentional data modification for parameters intended as immutable.",
-          "kind": "problem",
-          "name": "Constexpr or const specifiers shall be used for immutable parameter usage",
-          "precision": "high",
-          "severity": "warning",
-          "short_name": "DeclarationUnmodifiedParamMissingConstSpecifier",
-          "tags": [
-            "correctness",
-            "maintainability",
-            "readability"
-          ]
-        },
         {
           "description": "`Constexpr`/`const` specifiers prevent unintentional data modification for data intended as immutable.",
           "kind": "problem",
@@ -57,7 +44,10 @@
             "correctness",
             "maintainability",
             "readability"
-          ]
+          ],
+          "implementation_scope": {
+            "description": "We exclude function parameters from this rule in line with the rule intention as described in the C++ Core Guidelines Con.1 which excludes function parameters."
+          }
         }
       ],
       "title": "Constexpr or const specifiers shall be used for immutable data declaration."

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1 @@`
	`1`	+ * `A7-1-1` - no longer report parameters as contravening this rule. This is inline with the rule intent as described in the referenced C++ Core Guidelines rule [CON.1](https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#con1-by-default-make-objects-immutable), which states "To avoid confusion and lots of false positives, don’t enforce this rule for function parameters."
Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,2 @@`
	`1`	+`M9-3-3` - `MemberFunctionConstIfPossible.ql`, `MemberFunctionStaticIfPossible.ql`:
	`2`	`+ - Fixes #413. Exclude deleted member functions.`
Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,3 @@`
	`1`	+`A8-4-7` - `InParametersForCheapToCopyTypesNotPassedByValue.ql`, `InParametersForNotCheapToCopyTypesNotPassedByReference.ql`:
	`2`	+ - Fixes #397. Exclude user defined operators and move constructors.`
	`3`	`+ - Exclude parameters for instantiated templates because the declaration ___location of the function does not contain enough information about the type used in the instantiation to make an actionable alert.`
Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,2 @@`
	`1`	+`A5-0-2` - `NonBooleanIfStmt.qll`, `NonBooleanIterationStmt.qll`:
	`2`	`+ - Exclude compiler generated conditions.`
Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,2 @@`
	`1`	+`M9-3-3`: `MemberFunctionConstIfPossible.ql`:
	`2`	`+ - Fix FP reported in 467. Excluding candidates in uninstantiated templates.`