Implement package SideEffects6 with rule 28.3.1, predicate should not have side effects

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

@@ -61,6 +61,7 @@ import Representation
 import Scope
 import SideEffects1
 import SideEffects2
+import SideEffects6
 import SmartPointers1
 import SmartPointers2
 import Statements
@@ -134,6 +135,7 @@ newtype TCPPQuery =
   TScopePackageQuery(ScopeQuery q) or
   TSideEffects1PackageQuery(SideEffects1Query q) or
   TSideEffects2PackageQuery(SideEffects2Query q) or
+  TSideEffects6PackageQuery(SideEffects6Query q) or
   TSmartPointers1PackageQuery(SmartPointers1Query q) or
   TSmartPointers2PackageQuery(SmartPointers2Query q) or
   TStatementsPackageQuery(StatementsQuery q) or
@@ -207,6 +209,7 @@ predicate isQueryMetadata(Query query, string queryId, string ruleId, string cat
   isScopeQueryMetadata(query, queryId, ruleId, category) or
   isSideEffects1QueryMetadata(query, queryId, ruleId, category) or
   isSideEffects2QueryMetadata(query, queryId, ruleId, category) or
+  isSideEffects6QueryMetadata(query, queryId, ruleId, category) or
   isSmartPointers1QueryMetadata(query, queryId, ruleId, category) or
   isSmartPointers2QueryMetadata(query, queryId, ruleId, category) or
   isStatementsQueryMetadata(query, queryId, ruleId, category) or

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

@@ -0,0 +1,44 @@
+//** THIS FILE IS AUTOGENERATED, DO NOT MODIFY DIRECTLY.  **/
+import cpp
+import RuleMetadata
+import codingstandards.cpp.exclusions.RuleMetadata
+
+newtype SideEffects6Query =
+  TPredicateWithPersistentSideEffectsQuery() or
+  TNonConstPredicateFunctionObjectQuery()
+
+predicate isSideEffects6QueryMetadata(Query query, string queryId, string ruleId, string category) {
+  query =
+    // `Query` instance for the `predicateWithPersistentSideEffects` query
+    SideEffects6Package::predicateWithPersistentSideEffectsQuery() and
+  queryId =
+    // `@id` for the `predicateWithPersistentSideEffects` query
+    "cpp/misra/predicate-with-persistent-side-effects" and
+  ruleId = "RULE-28-3-1" and
+  category = "required"
+  or
+  query =
+    // `Query` instance for the `nonConstPredicateFunctionObject` query
+    SideEffects6Package::nonConstPredicateFunctionObjectQuery() and
+  queryId =
+    // `@id` for the `nonConstPredicateFunctionObject` query
+    "cpp/misra/non-const-predicate-function-object" and
+  ruleId = "RULE-28-3-1" and
+  category = "required"
+}
+
+module SideEffects6Package {
+  Query predicateWithPersistentSideEffectsQuery() {
+    //autogenerate `Query` type
+    result =
+      // `Query` type for `predicateWithPersistentSideEffects` query
+      TQueryCPP(TSideEffects6PackageQuery(TPredicateWithPersistentSideEffectsQuery()))
+  }
+
+  Query nonConstPredicateFunctionObjectQuery() {
+    //autogenerate `Query` type
+    result =
+      // `Query` type for `nonConstPredicateFunctionObject` query
+      TQueryCPP(TSideEffects6PackageQuery(TNonConstPredicateFunctionObjectQuery()))
+  }
+}

cpp/common/src/codingstandards/cpp/types/Predicate.qll

@@ -0,0 +1,66 @@
+import cpp
+private import codingstandards.cpp.StdNamespace
+private import codingstandards.cpp.types.Templates
+private import semmle.code.cpp.dataflow.new.DataFlow
+
+Namespace getTemplateParameterNamespace(TypeTemplateParameter param) {
+  exists(Declaration decl |
+    param = decl.(TemplateClass).getATemplateArgument() or
+    param = decl.(TemplateVariable).getATemplateArgument() or
+    param = decl.(TemplateFunction).getATemplateArgument()
+  |
+    result = decl.getNamespace()
+  )
+}
+
+class PredicateType extends TypeTemplateParameter {
+  PredicateType() {
+    this.getName().matches(["%Compare%", "%Predicate%"]) and
+    getTemplateParameterNamespace(this) instanceof StdNS
+  }
+
+  Type getASubstitutedType(Substitution sub) { result = sub.getSubstitutedTypeForParameter(this) }
+}
+
+class PredicateFunctionObject extends Class {
+  PredicateType pred;
+  Function operator;
+  Substitution sub;
+
+  PredicateFunctionObject() {
+    this = pred.getASubstitutedType(sub) and
+    operator.getDeclaringType() = this and
+    operator.getName() = "operator()"
+  }
+
+  PredicateType getPredicateType() { result = pred }
+
+  Function getCallOperator() { result = operator }
+
+  Substitution getSubstitution() { result = sub }
+}
+
+class PredicateFunctionPointerUse extends FunctionAccess {
+  Expr functionPointerArgument;
+  FunctionCall templateFunctionCall;
+  FunctionTemplateInstantiation instantiation;
+  Substitution sub;
+  PredicateType pred;
+  Parameter parameter;
+  int index;
+
+  PredicateFunctionPointerUse() {
+    functionPointerArgument = templateFunctionCall.getArgument(index) and
+    templateFunctionCall.getTarget() = instantiation and
+    parameter = instantiation.getParameter(index) and
+    sub.asFunctionSubstitution() = instantiation and
+    parameter.getType() = sub.getSubstitutedTypeForParameter(pred) and
+    exists(DataFlow::Node func, DataFlow::Node arg |
+      func.asExpr() = this and
+      arg.asExpr() = functionPointerArgument and
+      DataFlow::localFlow(func, arg)
+    )
+  }
+
+  PredicateType getPredicateType() { result = pred }
+}

cpp/common/src/codingstandards/cpp/types/Templates.qll

@@ -0,0 +1,49 @@
+import cpp
+
+private newtype TSubstitution =
+  TClassSubstitution(ClassTemplateInstantiation cti) or
+  TFunctionSubstitution(FunctionTemplateInstantiation fti) or
+  TVariableSubstitution(VariableTemplateInstantiation vti)
+
+class Substitution extends TSubstitution {
+  ClassTemplateInstantiation asClassSubstitution() { this = TClassSubstitution(result) }
+
+  FunctionTemplateInstantiation asFunctionSubstitution() { this = TFunctionSubstitution(result) }
+
+  VariableTemplateInstantiation asVariableSubstitution() { this = TVariableSubstitution(result) }
+
+  TypeTemplateParameter getTemplateParameter(int index) {
+    result = this.asClassSubstitution().getTemplate().getTemplateArgument(index) or
+    result = this.asFunctionSubstitution().getTemplate().getTemplateArgument(index) or
+    result = this.asVariableSubstitution().getTemplate().getTemplateArgument(index)
+  }
+
+  Type getSubstitutedType(int index) {
+    result = this.asClassSubstitution().getTemplateArgument(index) or
+    result = this.asFunctionSubstitution().getTemplateArgument(index) or
+    result = this.asVariableSubstitution().getTemplateArgument(index)
+  }
+
+  Type getSubstitutedTypeForParameter(TypeTemplateParameter param) {
+    exists(int idx |
+      this.getTemplateParameter(idx) = param and
+      result = this.getSubstitutedType(idx)
+    )
+  }
+
+  string toString() {
+    result = this.asClassSubstitution().toString() or
+    result = this.asFunctionSubstitution().toString() or
+    result = this.asVariableSubstitution().toString()
+  }
+
+  Locatable getASubstitutionSite() {
+    result.(TypeMention).getMentionedType() = this.asClassSubstitution()
+    or
+    result.(Call).getTarget() = this.asFunctionSubstitution()
+    or
+    result.(FunctionAccess).getTarget() = this.asFunctionSubstitution()
+    or
+    result.(VariableAccess).getTarget() = this.asVariableSubstitution()
+  }
+}

cpp/misra/src/rules/RULE-28-3-1/NonConstPredicateFunctionObject.ql

@@ -0,0 +1,33 @@
+/**
+ * @id cpp/misra/non-const-predicate-function-object
+ * @name RULE-28-3-1: Predicates shall not have persistent side effects
+ * @description Much of the behavior of predicates is implementation defined, such as how and when
+ *              it is invoked with which argument values, and if it is copied or moved. Therefore,
+ *              predicate function objects should be declared const.
+ * @kind problem
+ * @precision very-high
+ * @problem.severity error
+ * @tags external/misra/id/rule-28-3-1
+ *       scope/system
+ *       correctness
+ *       maintainability
+ *       portability
+ *       external/misra/enforcement/undecidable
+ *       external/misra/obligation/required
+ */
+
+import cpp
+import codingstandards.cpp.misra
+import codingstandards.cpp.SideEffect
+import codingstandards.cpp.types.Predicate
+
+from MemberFunction callOperator, PredicateFunctionObject obj, Locatable usageSite
+where
+  not isExcluded([callOperator, usageSite],
+    SideEffects6Package::nonConstPredicateFunctionObjectQuery()) and
+  obj.getSubstitution().getASubstitutionSite() = usageSite and
+  callOperator = obj.getCallOperator() and
+  not callOperator instanceof ConstMemberFunction
+select usageSite, "Predicate usage of $@ has $@", callOperator.getDeclaringType(),
+  "callable object " + callOperator.getDeclaringType().getName(), callOperator,
+  "non const operator()."

cpp/misra/src/rules/RULE-28-3-1/PredicateWithPersistentSideEffects.ql

@@ -0,0 +1,43 @@
+/**
+ * @id cpp/misra/predicate-with-persistent-side-effects
+ * @name RULE-28-3-1: Predicates shall not have persistent side effects
+ * @description Much of the behavior of predicates is implementation defined, such as how and when
+ *              it is invoked with which argument values, and if it is copied or moved. Therefore,
+ *              persistent side effects in a predicate cannot be relied upon and should not occur.
+ * @kind problem
+ * @precision very-high
+ * @problem.severity error
+ * @tags external/misra/id/rule-28-3-1
+ *       scope/system
+ *       correctness
+ *       maintainability
+ *       portability
+ *       external/misra/enforcement/undecidable
+ *       external/misra/obligation/required
+ */
+
+import cpp
+import codingstandards.cpp.misra
+import codingstandards.cpp.sideeffect.DefaultEffects
+import codingstandards.cpp.SideEffect
+import codingstandards.cpp.types.Predicate
+
+from Locatable usageSite, Function f, SideEffect effect
+where
+  not isExcluded([usageSite, effect], SideEffects6Package::predicateWithPersistentSideEffectsQuery()) and
+  effect = getAnExternalOrGlobalSideEffectInFunction(f) and
+  not effect instanceof ConstructorFieldInit and
+  (
+    // Case 1: a function pointer used directly as a predicate argument
+    exists(PredicateFunctionPointerUse use |
+      use = usageSite and
+      f = use.getTarget()
+    )
+    or
+    // Case 2: a function object whose call operator has side effects
+    exists(PredicateFunctionObject obj |
+      usageSite = obj.getSubstitution().getASubstitutionSite() and
+      f = obj.getCallOperator()
+    )
+  )
+select usageSite, "Predicate $@ has a $@.", f, f.getName(), effect, "persistent side effect"

cpp/misra/test/rules/RULE-28-3-1/NonConstPredicateFunctionObject.expected

@@ -0,0 +1 @@
+| test.cpp:21:3:21:11 | call to sort | Predicate usage of $@ has $@ | test.cpp:10:8:10:9 | F1 | callable object F1 | test.cpp:11:8:11:17 | operator() | non const operator(). |

cpp/misra/test/rules/RULE-28-3-1/NonConstPredicateFunctionObject.qlref

@@ -0,0 +1 @@
+rules/RULE-28-3-1/NonConstPredicateFunctionObject.ql

cpp/misra/test/rules/RULE-28-3-1/PredicateWithPersistentSideEffects.expected

@@ -0,0 +1,3 @@
+| test.cpp:60:35:60:49 | cmp_with_static | Predicate $@ has a $@. | test.cpp:52:6:52:20 | cmp_with_static | cmp_with_static | test.cpp:54:3:54:11 | ++ ... | persistent side effect |
+| test.cpp:73:35:73:49 | cmp_with_global | Predicate $@ has a $@. | test.cpp:66:6:66:20 | cmp_with_global | cmp_with_global | test.cpp:67:3:67:26 | ++ ... | persistent side effect |
+| test.cpp:86:3:86:11 | call to sort | Predicate $@ has a $@. | test.cpp:78:8:78:17 | operator() | operator() | test.cpp:79:5:79:28 | ++ ... | persistent side effect |

cpp/misra/test/rules/RULE-28-3-1/PredicateWithPersistentSideEffects.qlref

@@ -0,0 +1 @@
+rules/RULE-28-3-1/PredicateWithPersistentSideEffects.ql

cpp/misra/test/rules/RULE-28-3-1/test.cpp

@@ -0,0 +1,109 @@
+#include <algorithm>
+#include <cstdint>
+#include <functional>
+// #include <set>
+#include <vector>
+
+// Test cases for Rule 28.3.1#NonConstPredicateFunctionObject
+// This query checks that predicate function objects have const operator()
+
+struct F1 {
+  bool operator()(std::int32_t l1, std::int32_t l2) { return l1 > l2; }
+};
+
+void test_function_object_non_const_in_set() {
+  // TODO: implement stubs for set comparator.
+  // std::set<std::int32_t, F1> l1; // NON_COMPLIANT
+}
+
+void test_function_object_non_const_in_sort() {
+  std::vector<std::int32_t> l1 = {5, 2, 8, 1, 9};
+  std::sort(l1.begin(), l1.end(), F1{}); // NON_COMPLIANT
+}
+
+struct F2 {
+  bool operator()(std::int32_t l1, std::int32_t l2) const { return l1 > l2; }
+};
+
+void test_function_object_const_in_set() {
+  // TODO: implement stubs for set comparator.
+  // std::set<std::int32_t, F2> l1; // COMPLIANT
+}
+
+void test_function_object_const_in_sort() {
+  std::vector<std::int32_t> l1 = {5, 2, 8, 1, 9};
+  std::sort(l1.begin(), l1.end(), F2{}); // COMPLIANT
+}
+
+// Compliant: Using standard library predicates (always have const operator())
+void test_standard_library_predicates() {
+  std::vector<std::int32_t> l1 = {1, 2, 3, 4, 5};
+  // TODO: implement stubs for greater comparator.
+  // std::sort(l1.begin(), l1.end(), std::greater<std::int32_t>()); // COMPLIANT
+}
+
+// =============================================================================
+// Test cases for Rule 28.3.1#PredicateWithPersistentSideEffects
+// This query checks that predicates do not have persistent (global/static)
+// side effects.
+// =============================================================================
+
+// Non-compliant: free function predicate modifying a static local variable
+bool cmp_with_static(std::int32_t l1, std::int32_t l2) {
+  static std::int32_t g_count = 0;
+  ++g_count; // NON_COMPLIANT
+  return l1 < l2;
+}
+
+void test_predicate_fn_static_local() {
+  std::vector<std::int32_t> l1 = {5, 2, 8, 1, 9};
+  std::sort(l1.begin(), l1.end(), cmp_with_static); // NON_COMPLIANT
+}
+
+// Non-compliant: free function predicate modifying a global variable
+std::int32_t g_predicate_call_count = 0;
+
+bool cmp_with_global(std::int32_t l1, std::int32_t l2) {
+  ++g_predicate_call_count; // NON_COMPLIANT
+  return l1 < l2;
+}
+
+void test_predicate_fn_global() {
+  std::vector<std::int32_t> l1 = {5, 2, 8, 1, 9};
+  std::sort(l1.begin(), l1.end(), cmp_with_global); // NON_COMPLIANT
+}
+
+// Non-compliant: function object whose operator() modifies a global variable
+struct F3_SideEffect {
+  bool operator()(std::int32_t l1, std::int32_t l2) const {
+    ++g_predicate_call_count; // NON_COMPLIANT
+    return l1 < l2;
+  }
+};
+
+void test_function_object_with_global_side_effect() {
+  std::vector<std::int32_t> l1 = {5, 2, 8, 1, 9};
+  std::sort(l1.begin(), l1.end(), F3_SideEffect{}); // NON_COMPLIANT
+}
+
+// Compliant: free function predicate with no side effects
+bool cmp_pure(std::int32_t l1, std::int32_t l2) { // COMPLIANT
+  return l1 < l2;
+}
+
+void test_predicate_fn_pure() {
+  std::vector<std::int32_t> l1 = {5, 2, 8, 1, 9};
+  std::sort(l1.begin(), l1.end(), cmp_pure); // COMPLIANT
+}
+
+// Compliant: function object with const operator() and no side effects
+struct F4_Pure {
+  bool operator()(std::int32_t l1, std::int32_t l2) const { // COMPLIANT
+    return l1 < l2;
+  }
+};
+
+void test_function_object_pure() {
+  std::vector<std::int32_t> l1 = {5, 2, 8, 1, 9};
+  std::sort(l1.begin(), l1.end(), F4_Pure{}); // COMPLIANT
+}