Next query first pass working

Author: MichaelRFairhurst

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

@@ -18,24 +18,26 @@
 
 import cpp
 import codingstandards.cpp.misra
+import codingstandards.cpp.sideeffect.DefaultEffects
 import codingstandards.cpp.SideEffect
 import codingstandards.cpp.types.Predicate
 
-predicate functionHasSideEffect(Function f) {
-  hasExternalOrGlobalSideEffectInFunction(f)
-}
-
-predicate isPredicateObject(PredicateFunctionObject obj, PredicateType pred, Locatable usageSite, Function callOperator) {
-  obj.getSubstitution().getASubstitutionSite() = usageSite and
-  pred = obj.getPredicateType() and
-  callOperator = obj.getCallOperator()
-}
-
-predicate isPredicateFunctionPointerUse(PredicateFunctionPointerUse predPtrUse, PredicateType pred, Function func) {
-  pred = predPtrUse.getPredicateType() and
-  func = predPtrUse.getTarget()
-}
-
+from Locatable usageSite, Function f, SideEffect effect
 where
-  not isExcluded(x, SideEffects6Package::predicateWithPersistentSideEffectsQuery()) and
-select
+  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/PredicateWithPersistentSideEffects.expected

@@ -1 +1,3 @@
-No expected results have yet been specified
+| 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/test.cpp

@@ -40,4 +40,70 @@ 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
 }