-
Notifications
You must be signed in to change notification settings - Fork 77
Implement Rule 28-6-4, require use of std::remove_if and similar functions #1046
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
MichaelRFairhurst
merged 10 commits into
main
from
michaelrfairhurst/implement-deadcode-11-use-result-of-remove-algorithms-etc
Feb 26, 2026
Merged
Changes from 1 commit
Commits
Show all changes
10 commits
Select commit
Hold shift + click to select a range
46cb4ff
Implement Rule 28-6-4, require use of std::remove_if and similar func…
MichaelRFairhurst 5d7db89
Regenerate ql query header
MichaelRFairhurst 54418e4
Copilot feedback
MichaelRFairhurst b44c675
Add default ctor to deque which was breaking tests.
MichaelRFairhurst 40cbbae
Merge branch 'main' into michaelrfairhurst/implement-deadcode-11-use-…
mbaluda 62ea25f
Merge branch 'main' into michaelrfairhurst/implement-deadcode-11-use-…
mbaluda 7912c40
Update cpp/misra/src/rules/RULE-28-6-4/PotentiallyErroneousContainerU…
MichaelRFairhurst 92fdedf
Update cpp/misra/src/rules/RULE-28-6-4/PotentiallyErroneousContainerU…
MichaelRFairhurst fa01368
Merge remote-tracking branch 'origin/main' into michaelrfairhurst/imp…
MichaelRFairhurst a10f4e7
Update Iterators.qll include path
MichaelRFairhurst File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
26 changes: 26 additions & 0 deletions
26
cpp/common/src/codingstandards/cpp/exclusions/cpp/DeadCode11.qll
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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())) | ||
| } | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,6 @@ | ||
| #include "iterator.h" | ||
| #include <bits/stl_function.h> | ||
| #include <empty.h> | ||
|
|
||
| namespace std { | ||
|
|
||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,6 +3,7 @@ | |
| #define _GHLIBCPP_STRING_VIEW | ||
|
|
||
| #include "stddef.h" | ||
| #include <empty.h> | ||
|
|
||
| namespace std { | ||
|
|
||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
32 changes: 32 additions & 0 deletions
32
cpp/misra/src/rules/RULE-28-6-4/PotentiallyErroneousContainerUsage.ql
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,32 @@ | ||
| import cpp | ||
| import codingstandards.cpp.misra | ||
|
|
||
| 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", _) | ||
MichaelRFairhurst marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| } | ||
|
|
||
| predicate isEmptyCall(FunctionCall fc) { | ||
| fc.getTarget().getName() = "empty" and | ||
| ( | ||
| fc.getTarget().hasQualifiedName("std", "empty") or | ||
| fc.getTarget() instanceof MemberFunction | ||
MichaelRFairhurst marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| ) | ||
| } | ||
|
|
||
| from FunctionCall fc, string message | ||
| where | ||
| not isExcluded(fc, DeadCode11Package::potentiallyErroneousContainerUsageQuery()) and | ||
| exists(ExprStmt es | es.getExpr() = fc) and | ||
mbaluda marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| ( | ||
| 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." | ||
| ) | ||
MichaelRFairhurst marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| select fc, message | ||
8 changes: 8 additions & 0 deletions
8
cpp/misra/test/rules/RULE-28-6-4/PotentiallyErroneousContainerUsage.expected
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,8 @@ | ||
| | 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. | |
1 change: 1 addition & 0 deletions
1
cpp/misra/test/rules/RULE-28-6-4/PotentiallyErroneousContainerUsage.qlref
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| rules/RULE-28-6-4/PotentiallyErroneousContainerUsage.ql |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,144 @@ | ||
| #include <algorithm> | ||
| #include <cstdint> | ||
| #include <deque> | ||
| #include <string> | ||
| #include <vector> | ||
|
|
||
| // Test cases for std::remove | ||
| void test_remove_result_unused() { | ||
| std::vector<std::int32_t> v1 = {1, 2, 3, 2, 4}; | ||
| std::remove(v1.begin(), v1.end(), 2); // NON_COMPLIANT | ||
| } | ||
|
|
||
| void test_remove_result_used() { | ||
| std::vector<std::int32_t> v1 = {1, 2, 3, 2, 4}; | ||
| auto l1 = std::remove(v1.begin(), v1.end(), 2); // COMPLIANT | ||
| v1.erase(l1, v1.end()); | ||
| } | ||
|
|
||
| void test_remove_result_used_in_erase() { | ||
| std::vector<std::int32_t> v1 = {1, 2, 3, 2, 4}; | ||
| v1.erase(std::remove(v1.begin(), v1.end(), 2), v1.end()); // COMPLIANT | ||
| } | ||
|
|
||
| // Test cases for std::remove_if | ||
| void test_remove_if_result_unused() { | ||
| std::vector<std::int32_t> v1 = {1, 2, 3, 4, 5}; | ||
| std::remove_if(v1.begin(), v1.end(), | ||
| [](std::int32_t l1) { return l1 % 2 == 0; }); // NON_COMPLIANT | ||
| } | ||
|
|
||
| void test_remove_if_result_used() { | ||
| std::vector<std::int32_t> v1 = {1, 2, 3, 4, 5}; | ||
| auto l1 = std::remove_if(v1.begin(), v1.end(), [](std::int32_t l2) { | ||
| return l2 % 2 == 0; | ||
| }); // COMPLIANT | ||
| v1.erase(l1, v1.end()); | ||
| } | ||
|
|
||
| void test_remove_if_result_used_in_erase() { | ||
| std::vector<std::int32_t> v1 = {1, 2, 3, 4, 5}; | ||
| v1.erase(std::remove_if(v1.begin(), v1.end(), | ||
| [](std::int32_t l1) { return l1 % 2 == 0; }), | ||
| v1.end()); // COMPLIANT | ||
| } | ||
|
|
||
| // Test cases for std::unique | ||
| void test_unique_result_unused() { | ||
| std::vector<std::int32_t> v1 = {0, 0, 1, 1, 2, 2, 3, 3}; | ||
| std::unique(v1.begin(), v1.end()); // NON_COMPLIANT | ||
| } | ||
|
|
||
| void test_unique_result_used() { | ||
| std::vector<std::int32_t> v1 = {0, 0, 1, 1, 2, 2, 3, 3}; | ||
| auto l1 = std::unique(v1.begin(), v1.end()); // COMPLIANT | ||
| v1.erase(l1, v1.end()); | ||
| } | ||
|
|
||
| void test_unique_result_used_in_erase() { | ||
| std::vector<std::int32_t> v1 = {0, 0, 1, 1, 2, 2, 3, 3}; | ||
| v1.erase(std::unique(v1.begin(), v1.end()), v1.end()); // COMPLIANT | ||
| } | ||
|
|
||
| void test_unique_with_predicate_result_unused() { | ||
| std::vector<std::int32_t> v1 = {1, 2, 3, 4, 5, 6}; | ||
| std::unique(v1.begin(), v1.end(), [](std::int32_t l1, std::int32_t l2) { | ||
| return (l1 % 2) == (l2 % 2); | ||
| }); // NON_COMPLIANT | ||
| } | ||
|
|
||
| void test_unique_with_predicate_result_used() { | ||
| std::vector<std::int32_t> v1 = {1, 2, 3, 4, 5, 6}; | ||
| auto l1 = | ||
| std::unique(v1.begin(), v1.end(), [](std::int32_t l2, std::int32_t l3) { | ||
| return (l2 % 2) == (l3 % 2); | ||
| }); // COMPLIANT | ||
| v1.erase(l1, v1.end()); | ||
| } | ||
|
|
||
| // Test cases for empty (member function) | ||
| void test_empty_member_result_unused() { | ||
| std::vector<std::int32_t> v1 = {1, 2, 3}; | ||
| v1.empty(); // NON_COMPLIANT | ||
| } | ||
|
|
||
| void test_empty_member_result_used_in_condition() { | ||
| std::vector<std::int32_t> v1 = {1, 2, 3}; | ||
| if (v1.empty()) { // COMPLIANT | ||
| v1.clear(); | ||
| } | ||
| } | ||
|
|
||
| void test_empty_member_result_used_in_assignment() { | ||
| std::vector<std::int32_t> v1 = {1, 2, 3}; | ||
| bool l1 = v1.empty(); // COMPLIANT | ||
| } | ||
|
|
||
| // Test cases for std::empty (non-member function) | ||
| void test_empty_nonmember_result_unused() { | ||
| std::vector<std::int32_t> v1 = {1, 2, 3}; | ||
| std::empty(v1); // NON_COMPLIANT | ||
| } | ||
|
|
||
| void test_empty_nonmember_result_used_in_condition() { | ||
| std::vector<std::int32_t> v1 = {1, 2, 3}; | ||
| if (!std::empty(v1)) { // COMPLIANT | ||
| v1.clear(); | ||
| } | ||
| } | ||
|
|
||
| void test_empty_nonmember_result_used_in_assignment() { | ||
| std::vector<std::int32_t> v1 = {1, 2, 3}; | ||
| bool l1 = std::empty(v1); // COMPLIANT | ||
| } | ||
|
|
||
| void test_remove_with_deque() { | ||
| std::deque<std::int32_t> d1 = {1, 2, 3, 2, 4}; | ||
| std::remove(d1.begin(), d1.end(), 2); // NON_COMPLIANT | ||
| } | ||
|
|
||
| void test_empty_with_string() { | ||
| std::string s1 = "hello"; | ||
| s1.empty(); // NON_COMPLIANT | ||
| } | ||
|
|
||
| void test_empty_with_string_compliant() { | ||
| std::string s1 = "hello"; | ||
| if (s1.empty()) { // COMPLIANT | ||
| s1 = "default"; | ||
| } | ||
| } | ||
|
|
||
| // Edge case: result stored but not used | ||
| void test_remove_result_stored_but_not_used() { | ||
| std::vector<std::int32_t> v1 = {1, 2, 3, 2, 4}; | ||
| auto l1 = std::remove(v1.begin(), v1.end(), 2); // COMPLIANT | ||
| } | ||
|
|
||
| // Edge case: chaining operations | ||
| void test_remove_chained_operations() { | ||
| std::vector<std::int32_t> v1 = {1, 2, 3, 2, 4}; | ||
| std::vector<std::int32_t> v2 = {5, 6, 7}; | ||
| v1.erase(std::remove(v1.begin(), v1.end(), 2), v1.end()); // COMPLIANT | ||
| v2.erase(std::remove(v2.begin(), v2.end(), 6), v2.end()); // COMPLIANT | ||
| } |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.