Conversation
There was a problem hiding this comment.
Pull request overview
Adds three new MISRA C++:2023 rule packages (Banned5/6/8) and corresponding queries/tests, while refactoring existing AUTOSAR union/bit-field queries into shared implementations under cpp/common.
Changes:
- Add new MISRA rules/packages for
RULE-8-3-2,RULE-12-2-1, andRULE-12-3-1(Banned8/5/6), including query implementations and tests. - Factor union and bit-field logic into shared common
.qllmodules and point MISRA/AUTOSAR wrappers + tests at the shared implementations. - Introduce
codingstandards.cpp.CommonTypesinto the common pack and adjustHardwareOrProtocolInterfaceto live incpp/common.
Reviewed changes
Copilot reviewed 42 out of 42 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| rules.csv | Repoint MISRA rules to new packages Banned5/6/8. |
| rule_packages/cpp/Representation.json | Update short name + add shared implementation short name for bit-field shared query. |
| rule_packages/cpp/BannedSyntax.json | Fix description typo and switch union query metadata to shared implementation naming. |
| rule_packages/cpp/Banned8.json | New rule package metadata for RULE-8-3-2. |
| rule_packages/cpp/Banned6.json | New rule package metadata for RULE-12-3-1. |
| rule_packages/cpp/Banned5.json | New rule package metadata for RULE-12-2-1. |
| cpp/misra/test/rules/RULE-8-3-2/test.cpp | New test cases for built-in unary +. |
| cpp/misra/test/rules/RULE-8-3-2/BuiltInUnaryPlusOperatorShouldNotBeUsed.qlref | Test harness reference to the production query. |
| cpp/misra/test/rules/RULE-8-3-2/BuiltInUnaryPlusOperatorShouldNotBeUsed.expected | Expected results for unary + query. |
| cpp/misra/test/rules/RULE-12-3-1/UnionKeywordUsedMisraCpp.testref | Point MISRA wrapper testing to shared common test. |
| cpp/misra/test/rules/RULE-12-3-1/UnionKeywordUsed.testref | Point MISRA wrapper testing to shared common test. |
| cpp/misra/test/rules/RULE-12-2-1/BitFieldsShouldNotBeDeclaredMisraCpp.testref | Point MISRA wrapper testing to shared common test. |
| cpp/misra/src/rules/RULE-8-3-2/BuiltInUnaryPlusOperatorShouldNotBeUsed.ql | New MISRA query for built-in unary +. |
| cpp/misra/src/rules/RULE-12-3-1/UnionKeywordUsedMisraCpp.ql | MISRA wrapper instantiating shared union query with MISRA config. |
| cpp/misra/src/rules/RULE-12-3-1/UnionKeywordUsed.ql | MISRA wrapper instantiating shared union query (non “-misra-cpp” id). |
| cpp/misra/src/rules/RULE-12-2-1/BitFieldsShouldNotBeDeclaredMisraCpp.ql | MISRA wrapper instantiating shared bit-field query with MISRA config. |
| cpp/misra/src/rules/RULE-12-2-1/BitFieldsShouldNotBeDeclared.ql | MISRA wrapper instantiating shared bit-field query (non “-misra-cpp” id). |
| cpp/common/test/rules/unionkeywordused/test.cpp | Shared test source for union keyword rule. |
| cpp/common/test/rules/unionkeywordused/UnionKeywordUsed.ql | Shared test query driver for union keyword rule. |
| cpp/common/test/rules/unionkeywordused/UnionKeywordUsed.expected | Expected results for union keyword shared rule. |
| cpp/common/test/rules/bitfieldsshallbeusedonlywheninterfacingtohardwareorconformingtocommunicationprotocols/test.cpp | Shared test source for bit-field rule. |
| cpp/common/test/rules/bitfieldsshallbeusedonlywheninterfacingtohardwareorconformingtocommunicationprotocols/BitFieldsShallBeUsedOnlyWhenInterfacingToHardwareOrConformingToCommunicationProtocols.ql | Shared test query driver for bit-field rule. |
| cpp/common/test/rules/bitfieldsshallbeusedonlywheninterfacingtohardwareorconformingtocommunicationprotocols/BitFieldsShallBeUsedOnlyWhenInterfacingToHardwareOrConformingToCommunicationProtocols.expected | Expected results for bit-field shared rule. |
| cpp/common/src/codingstandards/cpp/rules/unionkeywordused/UnionKeywordUsed.qll | New shared union implementation module. |
| cpp/common/src/codingstandards/cpp/rules/bitfieldsshallbeusedonlywheninterfacingtohardwareorconformingtocommunicationprotocols/BitFieldsShallBeUsedOnlyWhenInterfacingToHardwareOrConformingToCommunicationProtocols.qll | New shared bit-field implementation module. |
| cpp/common/src/codingstandards/cpp/exclusions/cpp/RuleMetadata.qll | Register new packages Banned5/6/8 with query metadata plumbing. |
| cpp/common/src/codingstandards/cpp/exclusions/cpp/Representation.qll | Update representation package query wiring after renaming wrapper. |
| cpp/common/src/codingstandards/cpp/exclusions/cpp/BannedSyntax.qll | Update banned syntax package query wiring after union wrapper rename. |
| cpp/common/src/codingstandards/cpp/exclusions/cpp/Banned8.qll | New autogenerated exclusions/metadata module for Banned8. |
| cpp/common/src/codingstandards/cpp/exclusions/cpp/Banned6.qll | New autogenerated exclusions/metadata module for Banned6. |
| cpp/common/src/codingstandards/cpp/exclusions/cpp/Banned5.qll | New autogenerated exclusions/metadata module for Banned5. |
| cpp/common/src/codingstandards/cpp/HardwareOrProtocolInterface.qll | Move to common pack; update imports to rely on common cpp/CommonTypes. |
| cpp/common/src/codingstandards/cpp/CommonTypes.qll | Add common-pack CommonTypes module (copied from autosar). |
| cpp/autosar/test/rules/A9-6-2/BitFieldsShouldNotBeDeclaredAutosarCpp.testref | Point AUTOSAR wrapper testing to shared common test. |
| cpp/autosar/test/rules/A9-6-2/BitFieldsShallBeUsedOnlyWhenInterfacingToHardwareOrConformingToCommunicationProtocols.qlref | Remove direct test reference to the old production query. |
| cpp/autosar/test/rules/A9-6-2/BitFieldsShallBeUsedOnlyWhenInterfacingToHardwareOrConformingToCommunicationProtocols.expected | Remove old expected file (now covered by shared common test). |
| cpp/autosar/test/rules/A9-5-1/UnionsUsed.testref | Point AUTOSAR wrapper testing to shared common test. |
| cpp/autosar/test/rules/A9-5-1/UnionKeywordUsedAutosarCpp.testref | Point AUTOSAR wrapper testing to shared common test. |
| cpp/autosar/src/rules/A9-6-2/BitFieldsShouldNotBeDeclaredAutosarCpp.ql | New AUTOSAR wrapper instantiating shared bit-field query. |
| cpp/autosar/src/rules/A9-6-2/BitFieldsShallBeUsedOnlyWhenInterfacingToHardwareOrConformingToCommunicationProtocols.ql | Remove old non-shared AUTOSAR query implementation. |
| cpp/autosar/src/rules/A9-5-1/UnionsUsed.ql | Remove old non-shared AUTOSAR union query implementation. |
| cpp/autosar/src/rules/A9-5-1/UnionKeywordUsedAutosarCpp.ql | New AUTOSAR wrapper instantiating shared union query. |
Comments suppressed due to low confidence (1)
cpp/common/test/rules/bitfieldsshallbeusedonlywheninterfacingtohardwareorconformingtocommunicationprotocols/test.cpp:50
- These lines are marked
NON_COMPLIANT, but the.expectedfile in this directory does not include corresponding results. Please either add expected rows for these bit-fields, or (if they are allowed due to@HardwareOrProtocolInterface) change the markers toCOMPLIANTso the test annotations match the expected output.
int a1;
MYINT a2 : 2; // NON_COMPLIANT
char b1;
char c1 : 2; // NON_COMPLIANT
| */ | ||
| class HW_A2 : HW_A { | ||
| public: | ||
| int a1 : 2; // NON_COMPLIANT |
There was a problem hiding this comment.
This line is marked NON_COMPLIANT, but the .expected file for this test directory only contains an alert for B2::c (line 57). Either update the expected results to include an alert here, or (if bit-fields are meant to be permitted in @HardwareOrProtocolInterface classes) change this marker to COMPLIANT so annotations and expectations stay consistent.
This issue also appears on line 46 of the same file.
| * @id cpp/autosar/union-keyword-used-autosar-cpp | ||
| * @name A9-5-1: Unions shall not be used | ||
| * @description Unions shall not be used. Tagged unions can be used if 'std::variant' is not | ||
| * available. |
There was a problem hiding this comment.
This PR replaces existing AUTOSAR queries (including changing/removing previous @ids) rather than only adding new queries. That changes the released artifacts layout, so a change note should be added under change_notes/ (per docs/development_handbook.md) and the PR checklist updated accordingly.
Description
Add
Banned8,Banned5, andBanned6that corresponds toRULE-8-3-2,RULE-12-2-1, andRULE-12-3-1, respectively.Notes
RULE-8-3-2could not be numbered 1, since theBanned1name is already taken (RULE-6-7-2).HardwareOrProtocolInterfaceis moved fromcpp/autosar/src/codingstandards/cpp/tocpp/common/src/codingstandards/cpp/as the query became a shared one.cpp/common/src/codingstandards/cpp/CommonTypes.qllis an ancient file that was apparently created when the CodeQL Standard Library for C/C++ didn't have the types it had. It is copied over tocpp/common/src/codingstandards/cpp/as a temporary solution to appease the compiler, but it needs a rewrite at some point as the standard library should have all of those.Change request type
.ql,.qll,.qlsor unit tests)Rules with added or modified queries
RULE-8-3-2RULE-12-2-1RULE-12-3-1Release 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.