Implement RULE-2-8, project should not contain unused object definitions

[MichaelRFairhurst]

Description

Implement RULE-2-8, project should not contain unused object definitions

Also add a new AlertReporting shared query library for deduplicating results across macro definitions/invocations/etc.

Split __attribute__((unused)) variables (and similar) to a Strict pair of queries.

Change request type

• Release or process automation (GitHub workflows, internal scripts)
• Internal documentation
• External documentation
• Query files (.ql, .qll, .qls or unit tests)
• External scripts (analysis report or other code shipped as part of a release)

Rules with added or modified queries

• No rules added
• Queries have been added for the following rules:
  • RULE-2-8
• Queries have been modified for the following rules:
  • rule number here

Release change checklist

A change note (development_handbook.md#change-notes) is required for any pull request which modifies:

• The structure or layout of the release artifacts.
• The evaluation performance (memory, execution time) of an existing query.
• The results of an existing query in any circumstance.

If you are only adding new rule queries, a change note is not required.

Author: Is a change note required?

• Yes
• No

🚨🚨🚨
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.

• Confirmed

Reviewer: Confirm that either a change note is not required or the change note is required and has been added.

• Confirmed

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

• Have all the relevant rule package description files been checked in?
• Have you verified that the metadata properties of each new query is set appropriately?
• Do all the unit tests contain both "COMPLIANT" and "NON_COMPLIANT" cases?
• Are the alert messages properly formatted and consistent with the style guide?
• Have you run the queries on OpenPilot and verified that the performance and results are acceptable?
  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.
• Does the query have an appropriate level of in-query comments/documentation?
• Have you considered/identified possible edge cases?
• Does the query not reinvent features in the standard library?
• Can the query be simplified further (not golfed!)

Reviewer

• Have all the relevant rule package description files been checked in?
• Have you verified that the metadata properties of each new query is set appropriately?
• Do all the unit tests contain both "COMPLIANT" and "NON_COMPLIANT" cases?
• Are the alert messages properly formatted and consistent with the style guide?
• Have you run the queries on OpenPilot and verified that the performance and results are acceptable?
  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.
• Does the query have an appropriate level of in-query comments/documentation?
• Have you considered/identified possible edge cases?
• Does the query not reinvent features in the standard library?
• Can the query be simplified further (not golfed!)

[lcartey]

Thanks! Providing some initial feedback. I still need to review the deduplicate macro code.

[lcartey]

I think we should review, and potentially integrate, the queries from previous standards that look for unused local variables.

• UnusedLocalVariable.ql
• UnusedVariables.qll

These are both C++ queries, so not everything will be applicable, but notably:

• Ignoring results in functions that use ASM statements (as we cannot determine if the assembly instructions access the the variable).
• Consider ignoring results in functions with error expressions.
• Excluding compiler generated variables.

[lcartey]

Could you add some examples of const variables to this file?

For C++ one of the big problems with unused variable queries has been related to the value of constexpr variables being inlined in our AST, appearing to make such variables unused, when they are not. I don't think a similar problem exists with const variables, but it would be great to have a test case or two to (a) validate that and (b) protect against any future changes to the AST.

[lcartey]

I think it makes sense to use the tag "strict" here to cover these cases. Although we've added strict tags before (to AUTOSAR queries), we've never integrated them into our query suites, or provided user facing recommendations around them.

There are a few ways we could consider doing so, but I think my preference would be to:

1. Exclude strict queries from the default query suites (for only MISRA C to start with - the AUTOSAR strict queries would need to be re-reviewed)
2. Provide a strict query suite separately.
3. Update the user_manual.md to discuss the option of "strict" mode.
4. Update the development_handbook.md to discuss the criteria for a query to be considered for "strict" mode.

We should also, in this case, put into the "implementation_scope" why we consider this to be a "strict" query - i.e. why it is safe to default disable it.

The alternative to a "strict mode tag", that I think is worth mentioning here, is to provide a default "deviations" configuration that disables such queries that we would consider to be "strict".