Change definition of out parameter and add exclusions

An out parameter is a non-const reference or pointer that is modified.
We removed assignment operators and crement operators as exclusions to
the modified property, because they should still be used to qualify an
out parameter.

Additionally we exclude user-defined non-member assignment operator and
stream related operator parameters that are required to be passed by
non-const reference.

Author: rvermeulen

cpp/autosar/src/rules/A8-4-8/OutputParametersUsed.ql

@@ -23,31 +23,60 @@ import codingstandards.cpp.ConstHelpers
 import codingstandards.cpp.Operator
 
 /**
- * Non-const T& and T* `Parameter`s to `Function`s
+ * Holds if p is passed as a non-const reference or pointer and is modified.
+ * This holds for in-out or out-only parameters.
  */
-class NonConstReferenceOrPointerParameterCandidate extends FunctionParameter {
-  NonConstReferenceOrPointerParameterCandidate() {
-    this instanceof NonConstReferenceParameter
-    or
-    this instanceof NonConstPointerParameter
-  }
+predicate isOutParameter(NonConstPointerorReferenceParameter p) {
+  any(VariableEffect ve).getTarget() = p
+}
+
+/**
+ * Holds if parameter `p` is a parameter to a user defined assignment operator that
+ * is defined outside of a class body.
+ * These require an in-out parameter as the first argument.
+ */
+predicate isNonMemberUserAssignmentParameter(NonConstPointerorReferenceParameter p) {
+  p.getFunction() instanceof UserAssignmentOperator and
+  not p.isMember()
+}
+
+/**
+ * Holds if parameter `p` is a parameter to a stream insertion operator that
+ * is defined outside of a class body.
+ * These require an in-out parameter as the first argument.
+ *
+ * e.g., `std::ostream& operator<<(std::ostream& os, const T& obj)`
+ */
+predicate isStreamInsertionStreamParameter(NonConstPointerorReferenceParameter p) {
+  exists(StreamInsertionOperator op | not op.isMember() | op.getParameter(0) = p)
 }
 
-pragma[inline]
-predicate isFirstAccess(VariableAccess va) {
-  not exists(VariableAccess otherVa |
-    otherVa.getTarget() = va.getTarget() or
-    otherVa.getQualifier().(VariableAccess).getTarget() = va.getTarget()
-  |
-    otherVa.getASuccessor() = va
+/**
+ * Holds if parameter `p` is a parameter to a stream insertion operator that
+ * is defined outside of a class body.
+ * These require an in-out parameter as the first argument and an out parameter for the second.
+ *
+ * e.g., `std::istream& operator>>(std::istream& is, T& obj)`
+ */
+predicate isStreamExtractionParameter(NonConstPointerorReferenceParameter p) {
+  exists(StreamExtractionOperator op | not op.isMember() |
+    op.getParameter(0) = p
+    or
+    op.getParameter(1) = p
   )
 }
 
-from NonConstReferenceOrPointerParameterCandidate p, VariableEffect ve
+predicate isException(NonConstPointerorReferenceParameter p) {
+  isNonMemberUserAssignmentParameter(p) and p.getIndex() = 0
+  or
+  isStreamInsertionStreamParameter(p)
+  or
+  isStreamExtractionParameter(p)
+}
+
+from NonConstPointerorReferenceParameter p
 where
   not isExcluded(p, ConstPackage::outputParametersUsedQuery()) and
-  ve.getTarget() = p and
-  isFirstAccess(ve.getAnAccess()) and
-  not ve instanceof AnyAssignOperation and
-  not ve instanceof CrementOperation
-select p, "Out parameter " + p.getName() + " that is modified before being read."
+  isOutParameter(p) and
+  not isException(p)
+select p, "Out parameter '" + p.getName() + "' used."

cpp/autosar/test/rules/A8-4-8/OutputParametersUsed.expected

@@ -1,5 +1,6 @@
-| test.cpp:5:22:5:24 | str | Out parameter str that is modified before being read. |
-| test.cpp:16:14:16:14 | i | Out parameter i that is modified before being read. |
-| test.cpp:21:14:21:14 | i | Out parameter i that is modified before being read. |
-| test.cpp:29:12:29:12 | a | Out parameter a that is modified before being read. |
-| test.cpp:33:12:33:12 | a | Out parameter a that is modified before being read. |
+| test.cpp:5:22:5:24 | str | Out parameter 'str' used. |
+| test.cpp:8:22:8:24 | str | Out parameter 'str' used. |
+| test.cpp:16:14:16:14 | i | Out parameter 'i' used. |
+| test.cpp:21:14:21:14 | i | Out parameter 'i' used. |
+| test.cpp:29:12:29:12 | a | Out parameter 'a' used. |
+| test.cpp:33:12:33:12 | a | Out parameter 'a' used. |

cpp/autosar/test/rules/A8-4-8/test.cpp

@@ -5,7 +5,7 @@ void f(int &i) { // COMPLIANT
 void f1(std::string &str) { // NON_COMPLIANT
   str = "replacement";
 }
-void f2(std::string &str) { // COMPLIANT
+void f2(std::string &str) { // NON_COMPLIANT
   str += "suffix";
 }
 
@@ -37,3 +37,38 @@ void f7(A &a) { // NON_COMPLIANT
 void f8(int i) { // COMPLIANT
   i += 1;
 }
+
+constexpr A &operator|=(
+    A &lhs,
+    const A &rhs) noexcept { // COMPLIANT - non-member user defined assignment
+                             // operators are considered an exception.
+  return lhs;
+}
+
+enum class byte : unsigned char {};
+constexpr byte &operator|(const byte &lhs, const byte &rhs) noexcept {
+  return lhs | rhs;
+}
+constexpr byte &operator|=(
+    byte &lhs,
+    const byte rhs) noexcept { // COMPLIANT - non-member user defined assignment
+                               // operators are considered an exception.
+  lhs = (lhs | rhs);
+  return lhs;
+}
+
+#include <iostream>
+std::ostream &operator<<(std::ostream &os,
+                         const byte &obj) { // COMPLIANT - insertion operators
+                                            // are considered an exception.
+  std::ostream other;
+  os = other; // simulate modification
+  return os;
+}
+
+std::istream &operator>>(std::istream &is,
+                         byte &obj) { // COMPLIANT - extraction operators are
+                                      // considered an exception.
+  obj = static_cast<byte>('a');       // simulate modification
+  return is;
+}