Conversation
There was a problem hiding this comment.
Pull request overview
This PR refactors the UnusedParameter queries to use a shared module pattern with signature modules, enabling AUTOSAR and MISRA to have different exceptional cases while sharing core logic. It also adds a new query for MISRA C++ 2023 RULE-0-2-2.
Changes:
- Refactored
UnusedParameter.qllto use a signature module pattern with configurable exclusion logic - Added new query RULE-0-2-2 for MISRA C++ 2023 to check for unused named parameters
- Updated A0-1-4 and RULE-2-7 to use the new shared module pattern with rule-specific exclusions
Reviewed changes
Copilot reviewed 20 out of 20 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| cpp/common/src/codingstandards/cpp/rules/unusedparameter/UnusedParameter.qll | Refactored to use signature module pattern with configurable excludeParameter predicate |
| cpp/misra/src/rules/RULE-0-2-2/UnusedParameterMisraCpp.ql | New query for MISRA C++ RULE-0-2-2 with exclusion for unnamed parameters in definitions |
| cpp/autosar/src/rules/A0-1-4/UnusedParameter.ql | Updated to use new pattern, excludes virtual functions |
| c/misra/src/rules/RULE-2-7/UnusedParameter.ql | Updated to use new pattern, no custom exclusions |
| cpp/common/test/rules/unusedparameter/UnusedParameter.ql | Updated test query to use new pattern |
| c/common/test/rules/unusedparameter/UnusedParameter.ql | Updated test query to use new pattern |
| cpp/misra/test/rules/RULE-0-2-2/* | New test files for RULE-0-2-2 |
| cpp/autosar/test/rules/A0-1-4/* | Updated to use qlref instead of testref, with new test cases |
| cpp/common/test/rules/unusedparameter/test.cpp | Updated comment and expected results to reflect virtual functions now being checked |
| rule_packages/cpp/DeadCode8.json | New rule package metadata for RULE-0-2-2 |
| cpp/common/src/codingstandards/cpp/exclusions/cpp/DeadCode8.qll | Generated metadata file for DeadCode8 package |
| cpp/common/src/codingstandards/cpp/exclusions/cpp/RuleMetadata.qll | Added imports and references for DeadCode8 |
| rules.csv | Updated RULE-0-2-2 to use DeadCode8 package |
| change_notes/2026-02-13-refactor-unused-parameter-queries.md | Change note documenting the refactoring |
Comments suppressed due to low confidence (1)
rule_packages/cpp/DeadCode8.json:24
- According to rules.csv line 831, RULE-0-2-2 maps to both A0-1-4 (non-virtual functions) and A0-1-5 (virtual functions), meaning it should apply to all named function parameters. The title incorrectly restricts this to "non-virtual functions". Update the title to reflect the actual scope.
"title": "There shall be no unused named parameters in non-virtual functions."
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| // This is a violation of A0-1-4, but not a violation of Rule 0-2-2, so it's | ||
| // accepted by the default shared logic. See A0-1-4 tests for coverage. | ||
| virtual void d(int x, int y) {} // COMPLIANT |
There was a problem hiding this comment.
Does not match the expected file
isn't this testing the default logic?
There was a problem hiding this comment.
You're right. This whole comment is backwards:
- It's rejected by the default logic
- It's exceptionally allowed by A0-1-4
- It's still rejected in Rule-0-2-2
Fixed!
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 20 out of 20 changed files in this pull request and generated 3 comments.
Comments suppressed due to low confidence (1)
rule_packages/cpp/DeadCode8.json:13
- Within this package entry, the query
nameis also expected to align with the rule title inrules.csv(and is typically generated from it). Consider updating the query name here to matchrules.csv’s wording forRULE-0-2-2to keep metadata consistent across rules/packages.
"description": "Unused parameters can indicate a mistake when implementing the function.",
"kind": "problem",
"name": "There shall be no unused named parameters in both virtual and non-virtual functions",
"precision": "very-high",
| "title": "There shall be no unused named parameters in both virtual and non-virtual functions." | ||
| } |
There was a problem hiding this comment.
The package title should match the rule title from rules.csv for RULE-0-2-2 (currently: “A named function parameter shall be used at least once”). As written, this uses a different sentence and adds a trailing period, which is likely to break consistency/automation for generated metadata.
This issue also appears on line 10 of the same file.
| @@ -0,0 +1,29 @@ | |||
| /** | |||
| * @id cpp/misra/unused-parameter-misra-cpp | |||
| * @name RULE-0-2-2: There shall be no unused named parameters in both virtual and non-virtual functions | |||
There was a problem hiding this comment.
The @name metadata doesn’t match the rules.csv title for RULE-0-2-2 (“A named function parameter shall be used at least once”). Most MISRA query @name values mirror the corresponding rules.csv rule title; consider aligning this to avoid inconsistencies in generated docs/metadata.
| * @name RULE-0-2-2: There shall be no unused named parameters in both virtual and non-virtual functions | |
| * @name A named function parameter shall be used at least once |
| void c(int) {} // COMPLIANT | ||
|
|
||
| // This is a violation of Rule 0-2-2, and therefore flagged by the default | ||
| // logic. However, this allowed by additional exception in A0-1-4. See A0-1-4 |
There was a problem hiding this comment.
Minor grammar issue in the explanatory comment: “However, this allowed by …” is missing “is”. This is user-facing documentation within tests, so it’s worth fixing for clarity.
| // logic. However, this allowed by additional exception in A0-1-4. See A0-1-4 | |
| // logic. However, this is allowed by additional exception in A0-1-4. See A0-1-4 |
2 participants
Description
Makes
UnusedParametersinto a shared module so AUTOSAR / MISRA can have different exceptional cases but otherwise share logic.CI/CD is failing pending #1035
Change request type
.ql,.qll,.qlsor unit tests)Rules with added or modified queries
RULE-0-2-2A0-1-4RULE-2-7Release change checklist
A change note (development_handbook.md#change-notes) is required for any pull request which modifies:
If you are only adding new rule queries, a change note is not required.
Author: Is a change note required?
🚨🚨🚨
Reviewer: Confirm that format of shared queries (not the .qll file, the
.ql file that imports it) is valid by running them within VS Code.
Reviewer: Confirm that either a change note is not required or the change note is required and has been added.
Query development review checklist
For PRs that add new queries or modify existing queries, the following checklist should be completed by both the author and reviewer:
Author
As a rule of thumb, predicates specific to the query should take no more than 1 minute, and for simple queries be under 10 seconds. If this is not the case, this should be highlighted and agreed in the code review process.
Reviewer
As a rule of thumb, predicates specific to the query should take no more than 1 minute, and for simple queries be under 10 seconds. If this is not the case, this should be highlighted and agreed in the code review process.