Implement Rule 0.2.4, unused functions with limited visibility.#1044
Implement Rule 0.2.4, unused functions with limited visibility.#1044MichaelRFairhurst wants to merge 2 commits intomainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Adds MISRA-C++-2023 RULE-0-2-4 (“unused functions with limited visibility”) by factoring the existing AUTOSAR A0-1-3 logic into a shared implementation and extending it to correctly handle class members with internal linkage (e.g., in anonymous namespaces).
Changes:
- Introduces shared query implementation
UnusedLocalFunctionand migrates AUTOSAR A0-1-3 to use it. - Adds MISRA RULE-0-2-4 wrapper query and a new
DeadCode10rule package/exclusions metadata to host it. - Extends unit tests/expected outputs to cover unused public member functions in anonymous namespaces.
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| rules.csv | Maps RULE-0-2-4 to new DeadCode10 package. |
| rule_packages/cpp/DeadCode10.json | Adds metadata for MISRA RULE-0-2-4 query package. |
| rule_packages/cpp/DeadCode.json | Adds shared_implementation_short_name for A0-1-3 metadata. |
| cpp/misra/test/rules/RULE-0-2-4/UnusedLimitedVisibilityFunction.testref | Wires MISRA test to shared test query. |
| cpp/misra/src/rules/RULE-0-2-4/UnusedLimitedVisibilityFunction.ql | Adds MISRA wrapper query that instantiates shared implementation. |
| cpp/common/test/rules/unusedlocalfunction/test.cpp | Adds C++ test cases for anonymous-namespace class member functions. |
| cpp/common/test/rules/unusedlocalfunction/UnusedLocalFunction.ql | Adds generated test query wrapper for the shared implementation. |
| cpp/common/test/rules/unusedlocalfunction/UnusedLocalFunction.expected | Updates expected results for new test cases. |
| cpp/common/src/codingstandards/cpp/rules/unusedlocalfunction/UnusedLocalFunction.qll | Adds shared implementation, including new anonymous-namespace member handling. |
| cpp/common/src/codingstandards/cpp/exclusions/cpp/RuleMetadata.qll | Registers new DeadCode10 package metadata. |
| cpp/common/src/codingstandards/cpp/exclusions/cpp/DeadCode10.qll | Adds autogenerated exclusions/metadata module for DeadCode10. |
| cpp/autosar/test/rules/A0-1-3/UnusedLocalFunction.testref | Wires AUTOSAR test to shared test query. |
| cpp/autosar/src/rules/A0-1-3/UnusedLocalFunctionOld.ql | Adds a copy of the old query implementation. |
| cpp/autosar/src/rules/A0-1-3/UnusedLocalFunction.ql | Refactors AUTOSAR query to instantiate shared implementation. |
| change_notes/2026-02-18-unused-member-functions-with-internal-linkage.md | Adds change note describing the behavior change for A0-1-3. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| /** | ||
| * @id cpp/autosar/unused-local-function | ||
| * @name A0-1-3: Unused local function | ||
| * @description Every function defined in an anonymous namespace, or static function with internal | ||
| * linkage, or private member function shall be used. | ||
| * @kind problem | ||
| * @precision very-high | ||
| * @problem.severity warning | ||
| * @tags external/autosar/id/a0-1-3 | ||
| * readability |
There was a problem hiding this comment.
UnusedLocalFunctionOld.ql is a full query (has @id, @kind, etc.) that duplicates the existing query id cpp/autosar/unused-local-function from UnusedLocalFunction.ql. Because cpp/autosar/src/qlpack.yml doesn't exclude files, this will be included in the pack and can cause duplicate-query-id conflicts / unintended extra shipped query. Remove this file, or convert it into non-query documentation (e.g., move out of the pack, rename to .qll, or remove query metadata) if you need to keep it for reference.
| // Class members in anonymous namespaces also have internal linkage. | ||
| this instanceof AnonymousNamespaceFunction and | ||
| this instanceof MemberFunction and |
There was a problem hiding this comment.
LocalFunction() assigns localFunctionType in multiple disjuncts that can overlap (e.g., a private member function inside an anonymous namespace satisfies both the "Private member" branch and the "Anonymous namespace class member" branch). This can give a single function multiple localFunctionType values and lead to duplicate results with different messages. Make these cases mutually exclusive (for example, exclude private members from the anonymous-namespace-member branch, or restructure with an if/else-style chain) and add/adjust a unit test to guard against duplicated findings.
| // Class members in anonymous namespaces also have internal linkage. | |
| this instanceof AnonymousNamespaceFunction and | |
| this instanceof MemberFunction and | |
| // Class members in anonymous namespaces also have internal linkage. | |
| // Exclude private members, which are already covered by the "Private member" case above. | |
| this instanceof AnonymousNamespaceFunction and | |
| this instanceof MemberFunction and | |
| not this.(MemberFunction).isPrivate() and |
| "correctness" | ||
| ], | ||
| "implementation_scope": { | ||
| "description": "Use of any overload of a function in an overload set constitute a use of all members of the set. An overload set is a set of functions with the same name that differ in the number, type and/or qualifiers of their parameters, and, for the purpose of this query, are limited to functions which are declared in the same scope (namespace or class). Functions defined in anonymous (unnamed) namespaces and global namespaces are therefore not currently considered to be part of the same overload set." |
There was a problem hiding this comment.
Grammar: the subject is singular (“Use ...”), so "constitute" should be "constitutes" in this sentence.
|
Erroneously added, PR closed and session canceled. Fix format: /src/debug/protocol/controller.cpp:206:61: error: code should be clang-formatted [-Wclang-format-violations] |
|
@MichaelRFairhurst I've opened a new pull request, #1045, to work on those changes. Once the pull request is ready, I'll request review from you. |
Description
Logic is effectively the same as A0-1-3, though A0-1-3 handled class member linkage incorrectly.
Change request type
.ql,.qll,.qlsor unit tests)Rules with added or modified queries
Release 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.