Implement Rule 28-6-4, require use of std::remove_if and similar functions

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

@@ -0,0 +1,26 @@
+//** THIS FILE IS AUTOGENERATED, DO NOT MODIFY DIRECTLY.  **/
+import cpp
+import RuleMetadata
+import codingstandards.cpp.exclusions.RuleMetadata
+
+newtype DeadCode11Query = TPotentiallyErroneousContainerUsageQuery()
+
+predicate isDeadCode11QueryMetadata(Query query, string queryId, string ruleId, string category) {
+  query =
+    // `Query` instance for the `potentiallyErroneousContainerUsage` query
+    DeadCode11Package::potentiallyErroneousContainerUsageQuery() and
+  queryId =
+    // `@id` for the `potentiallyErroneousContainerUsage` query
+    "cpp/misra/potentially-erroneous-container-usage" and
+  ruleId = "RULE-28-6-4" and
+  category = "required"
+}
+
+module DeadCode11Package {
+  Query potentiallyErroneousContainerUsageQuery() {
+    //autogenerate `Query` type
+    result =
+      // `Query` type for `potentiallyErroneousContainerUsage` query
+      TQueryCPP(TDeadCode11PackageQuery(TPotentiallyErroneousContainerUsageQuery()))
+  }
+}

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

@@ -17,6 +17,7 @@ import Const
 import Conversions
 import Conversions2
 import DeadCode
+import DeadCode11
 import DeadCode3
 import DeadCode4
 import Declarations
@@ -89,6 +90,7 @@ newtype TCPPQuery =
   TConversionsPackageQuery(ConversionsQuery q) or
   TConversions2PackageQuery(Conversions2Query q) or
   TDeadCodePackageQuery(DeadCodeQuery q) or
+  TDeadCode11PackageQuery(DeadCode11Query q) or
   TDeadCode3PackageQuery(DeadCode3Query q) or
   TDeadCode4PackageQuery(DeadCode4Query q) or
   TDeclarationsPackageQuery(DeclarationsQuery q) or
@@ -161,6 +163,7 @@ predicate isQueryMetadata(Query query, string queryId, string ruleId, string cat
   isConversionsQueryMetadata(query, queryId, ruleId, category) or
   isConversions2QueryMetadata(query, queryId, ruleId, category) or
   isDeadCodeQueryMetadata(query, queryId, ruleId, category) or
+  isDeadCode11QueryMetadata(query, queryId, ruleId, category) or
   isDeadCode3QueryMetadata(query, queryId, ruleId, category) or
   isDeadCode4QueryMetadata(query, queryId, ruleId, category) or
   isDeclarationsQueryMetadata(query, queryId, ruleId, category) or

cpp/common/test/includes/standard-library/algorithm

@@ -10,9 +10,34 @@ template <class T> constexpr const T &min(const T &a, const T &b);
 template <class InputIterator, class Function>
 Function for_each(InputIterator first, InputIterator last, Function f);
 
+/** std::remove */
 template <class ForwardIterator, class T>
 ForwardIterator remove(ForwardIterator first, ForwardIterator last,
                        const T &value);
+template <class ExecutionPolicy, class ForwardIterator, class T>
+ForwardIterator remove(ExecutionPolicy &&policy, ForwardIterator first,
+    ForwardIterator last, const T &value);
+
+/** std::remove_if */
+template <class ForwardIterator, class Predicate>
+ForwardIterator remove_if(ForwardIterator first, ForwardIterator last,
+                          Predicate pred);
+template <class ExecutionPolicy, class ForwardIterator, class Predicate>
+ForwardIterator remove_if(ExecutionPolicy &&policy, ForwardIterator first,
+    ForwardIterator last, Predicate pred);
+
+/** std::unique */
+template <class ForwardIterator>
+ForwardIterator unique(ForwardIterator first, ForwardIterator last);
+template <class ExecutionPolicy, class ForwardIterator>
+ForwardIterator unique(ExecutionPolicy &&policy, ForwardIterator first,
+    ForwardIterator last);
+template <class ForwardIterator, class BinaryPredicate>
+ForwardIterator unique(ForwardIterator first, ForwardIterator last,
+    BinaryPredicate pred);
+template <class ExecutionPolicy, class ForwardIterator, class BinaryPredicate>
+ForwardIterator unique(ExecutionPolicy &&policy, ForwardIterator first,
+    ForwardIterator last, BinaryPredicate pred);
 
 template <class InputIterator, class OutputIterator>
 OutputIterator copy(InputIterator first, InputIterator last,

cpp/common/test/includes/standard-library/array

@@ -2,6 +2,7 @@
 #define _GHLIBCPP_ARRAY
 #include <iterator>
 #include <string>
+#include <empty.h>
 
 // Note: a few features currently unused by tests are commented out
 namespace std {

cpp/common/test/includes/standard-library/deque.h

@@ -2,6 +2,8 @@
 #define _GHLIBCPP_DEQUE
 #include <iterator>
 #include <string>
+#include <initializer_list>
+#include <empty.h>
 
 namespace std {
 template <class T, class Allocator = std::allocator<T>> class deque {
@@ -11,6 +13,9 @@ template <class T, class Allocator = std::allocator<T>> class deque {
   typedef value_type &reference;
   typedef const value_type &const_reference;
 
+  deque() = default;
+  deque(std::initializer_list<T>, const Allocator & = Allocator());
+
   typedef __iterator<T> iterator;
   typedef __iterator<T> const_iterator;
 

cpp/common/test/includes/standard-library/empty.h

@@ -0,0 +1,21 @@
+#ifndef _GHLIBCPP_EMPTY
+#define _GHLIBCPP_EMPTY
+#include <initializer_list>
+#include <stddef.h>
+/**
+ * Not intended to be used via #include <empty.h>, and not part of the public
+ * API.
+ *
+ * However, multiple libraries such as <array>, <deque>, etc define std::empty,
+ * so this is the singular header to define it in the test suite.
+ */
+
+namespace std {
+template <class C> constexpr auto empty(const C &c) -> decltype(c.empty());
+
+template <class T, size_t N> constexpr bool empty(const T (&)[N]) noexcept;
+
+template <class T> constexpr bool empty(std::initializer_list<T>) noexcept;
+} // namespace std
+
+#endif // _GHLIBCPP_EMPTY

cpp/common/test/includes/standard-library/map

@@ -1,5 +1,6 @@
 #include "iterator.h"
 #include <bits/stl_function.h>
+#include <empty.h>
 
 namespace std {
 

cpp/common/test/includes/standard-library/set

@@ -1,5 +1,6 @@
 #include "iterator.h"
 #include <bits/stl_function.h>
+#include <empty.h>
 
 namespace std {
 template <typename _Key, typename _Compare = std::less<_Key>,

cpp/common/test/includes/standard-library/string

@@ -6,6 +6,7 @@
 #include "iosfwd.h"
 #include "iterator.h"
 #include "stddef.h"
+#include <empty.h>
 
 namespace std {
 template <class charT> struct char_traits {

cpp/common/test/includes/standard-library/string_view

@@ -3,6 +3,7 @@
 #define _GHLIBCPP_STRING_VIEW
 
 #include "stddef.h"
+#include <empty.h>
 
 namespace std {
 

cpp/common/test/includes/standard-library/vector.h

@@ -2,6 +2,7 @@
 #define _GHLIBCPP_VECTOR
 #include <iterator>
 #include <string>
+#include <empty.h>
 
 namespace std {
 
@@ -52,6 +53,11 @@ template <class T, class Allocator = std::allocator<T>> class vector {
                   InputIterator last);
   iterator insert(const_iterator position, initializer_list<T> il);
 
+  iterator erase(iterator position);
+  iterator erase(const_iterator position);
+  iterator erase(iterator first, iterator last);
+  iterator erase(const_iterator first, const_iterator last);
+
   reference operator[](size_type n);
   const_reference operator[](size_type n) const;
 

cpp/misra/src/rules/RULE-28-6-4/PotentiallyErroneousContainerUsage.ql

@@ -0,0 +1,50 @@
+/**
+ * @id cpp/misra/potentially-erroneous-container-usage
+ * @name RULE-28-6-4: The result of std::remove, std::remove_if, std::unique and empty shall be used
+ * @description The `empty` method may be confused with `clear`, and the family of algorithms
+ *              similar to `std::remove` require the resulting iterator to be used in order to
+ *              properly handle the modifications performed.
+ * @kind problem
+ * @precision very-high
+ * @problem.severity error
+ * @tags external/misra/id/rule-28-6-4
+ *       scope/single-translation-unit
+ *       correctness
+ *       external/misra/enforcement/decidable
+ *       external/misra/obligation/required
+ */
+
+import cpp
+import codingstandards.cpp.misra
+import codingstandards.cpp.Iterators
+
+predicate isRemoveOrUniqueCall(FunctionCall fc) {
+  exists(string name | name = fc.getTarget().getName() |
+    name = "remove" or
+    name = "remove_if" or
+    name = "unique"
+  ) and
+  fc.getTarget().hasQualifiedName("std", _) and
+  fc.getAnArgument().getUnderlyingType() instanceof IteratorType
+}
+
+predicate isEmptyCall(FunctionCall fc) {
+  fc.getTarget().getName() = "empty" and
+  (
+    fc.getTarget().hasQualifiedName("std", "empty") or
+    fc.getTarget().(MemberFunction).getDeclaringType().hasQualifiedName("std", _)
+  )
+}
+
+from FunctionCall fc, string message
+where
+  not isExcluded(fc, DeadCode11Package::potentiallyErroneousContainerUsageQuery()) and
+  exists(ExprStmt es | es.getExpr() = fc) and
+  (
+    isRemoveOrUniqueCall(fc) and
+    message = "Result of call to '" + fc.getTarget().getName() + "' is not used."
+    or
+    isEmptyCall(fc) and
+    message = "Result of call to 'empty' is not used."
+  )
+select fc, message

cpp/misra/test/rules/RULE-28-6-4/PotentiallyErroneousContainerUsage.expected

@@ -0,0 +1,10 @@
+| test.cpp:10:3:10:13 | call to remove | Result of call to 'remove' is not used. |
+| test.cpp:27:3:27:16 | call to remove_if | Result of call to 'remove_if' is not used. |
+| test.cpp:49:3:49:13 | call to unique | Result of call to 'unique' is not used. |
+| test.cpp:65:3:65:13 | call to unique | Result of call to 'unique' is not used. |
+| test.cpp:82:6:82:10 | call to empty | Result of call to 'empty' is not used. |
+| test.cpp:100:3:100:12 | call to empty | Result of call to 'empty' is not used. |
+| test.cpp:117:3:117:13 | call to remove | Result of call to 'remove' is not used. |
+| test.cpp:122:6:122:10 | call to empty | Result of call to 'empty' is not used. |
+| test.cpp:148:21:148:31 | call to remove | Result of call to 'remove' is not used. |
+| test.cpp:149:9:149:19 | call to remove | Result of call to 'remove' is not used. |

cpp/misra/test/rules/RULE-28-6-4/PotentiallyErroneousContainerUsage.qlref

@@ -0,0 +1 @@
+rules/RULE-28-6-4/PotentiallyErroneousContainerUsage.ql