Skip to content

Add Banned2, Banned3, and Banned4#1080

Open
jeongsoolee09 wants to merge 14 commits intomainfrom
jeongsoolee09/MISRA-C++-2023-Banned234
Open

Add Banned2, Banned3, and Banned4#1080
jeongsoolee09 wants to merge 14 commits intomainfrom
jeongsoolee09/MISRA-C++-2023-Banned234

Conversation

@jeongsoolee09
Copy link
Collaborator

Description

please enter the description of your change here

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 number here
  • 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!)

@jeongsoolee09 jeongsoolee09 marked this pull request as ready for review March 22, 2026 00:46
Copilot AI review requested due to automatic review settings March 22, 2026 00:46
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds three new MISRA C++ 2023 “Banned*” rule packages and their CodeQL implementations/tests, then wires them into the C++ exclusions/metadata and rules.csv mapping.

Changes:

  • Add new rule packages Banned2, Banned3, Banned4 and map RULE-10-2-2/10-2-3/10-3-1 to them in rules.csv.
  • Add new CodeQL queries implementing the three MISRA rules and corresponding unit tests (.qlref + .expected).
  • Integrate the new packages into C++ exclusions metadata (RuleMetadata.qll + autogenerated Banned{2,3,4}.qll).

Reviewed changes

Copilot reviewed 22 out of 22 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
rules.csv Re-points three MISRA C++ rules from Banned to Banned2/3/4.
rule_packages/cpp/Banned2.json Adds package metadata for RULE-10-2-2 query.
rule_packages/cpp/Banned3.json Adds package metadata for RULE-10-2-3 query.
rule_packages/cpp/Banned4.json Adds package metadata for RULE-10-3-1 query.
cpp/misra/src/rules/RULE-10-2-2/UnscopedEnumerationsShouldNotBeDeclared.ql New query for unscoped enums outside class/struct scope.
cpp/misra/src/rules/RULE-10-2-3/UnscopedEnumWithoutFixedUnderlyingTypeUsed.ql New query for uses of unfixed unscoped enums in numeric contexts.
cpp/misra/src/rules/RULE-10-3-1/UnnamedNamespacesInHeaderFiles.ql New query detecting anonymous namespaces in header files.
cpp/misra/test/rules/RULE-10-2-2/test.cpp Test cases for RULE-10-2-2.
cpp/misra/test/rules/RULE-10-2-2/UnscopedEnumerationsShouldNotBeDeclared.qlref Test wiring to production query.
cpp/misra/test/rules/RULE-10-2-2/UnscopedEnumerationsShouldNotBeDeclared.expected Expected results for RULE-10-2-2 tests.
cpp/misra/test/rules/RULE-10-2-3/test.cpp Test cases for RULE-10-2-3.
cpp/misra/test/rules/RULE-10-2-3/UnscopedEnumWithoutFixedUnderlyingTypeUsed.qlref Test wiring to production query.
cpp/misra/test/rules/RULE-10-2-3/UnscopedEnumWithoutFixedUnderlyingTypeUsed.expected Expected results for RULE-10-2-3 tests.
cpp/misra/test/rules/RULE-10-3-1/test.hpp Header fixture containing compliant/non-compliant namespace patterns.
cpp/misra/test/rules/RULE-10-3-1/test1.cpp TU fixture for header anonymous-namespace behavior.
cpp/misra/test/rules/RULE-10-3-1/test2.cpp TU fixture + main for RULE-10-3-1 test setup.
cpp/misra/test/rules/RULE-10-3-1/UnnamedNamespacesInHeaderFiles.qlref Test wiring to production query.
cpp/misra/test/rules/RULE-10-3-1/UnnamedNamespacesInHeaderFiles.expected Expected results for RULE-10-3-1 tests.
cpp/common/src/codingstandards/cpp/exclusions/cpp/RuleMetadata.qll Registers new packages in C++ exclusions metadata wiring.
cpp/common/src/codingstandards/cpp/exclusions/cpp/Banned2.qll Autogenerated exclusion package module for Banned2.
cpp/common/src/codingstandards/cpp/exclusions/cpp/Banned3.qll Autogenerated exclusion package module for Banned3.
cpp/common/src/codingstandards/cpp/exclusions/cpp/Banned4.qll Autogenerated exclusion package module for Banned4.

Comment on lines +143 to +144
// 64 bits: everything else
result = 64 and signed = true
Copy link

Copilot AI Mar 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

enumMinBits defaults the 64-bit case to signed = true (lines 142-145). This will mis-handle enums whose enumerators require an unsigned 64-bit underlying type (for example, large non-negative values > 2^63-1), potentially causing false positives/negatives in the size/signedness checks. Consider extending the range checks to distinguish signed vs unsigned 64-bit (or derive signedness directly from the enum’s value range / underlying type selection logic).

Suggested change
// 64 bits: everything else
result = 64 and signed = true
// 64 bits: everything else; choose signedness based on value range
if minVal >= "0".toBigInt()
then (
result = 64 and signed = false
) else (
result = 64 and signed = true
)

Copilot uses AI. Check for mistakes.
Comment on lines +921 to +923
cpp,MISRA-C++-2023,RULE-10-2-2,Yes,Advisory,Decidable,Single Translation Unit,Unscoped enumerations should not be declared,A7-2-3,Banned2,Easy,
cpp,MISRA-C++-2023,RULE-10-2-3,Yes,Required,Decidable,Single Translation Unit,The numeric value of an unscoped enumeration with no fixed underlying type shall not be used,A4-5-1,Banned3,Easy,
cpp,MISRA-C++-2023,RULE-10-3-1,Yes,Advisory,Decidable,Single Translation Unit,There should be no unnamed namespaces in header files,"DCL59-CPP, M7-3-3",Banned4,Easy,
Copy link

Copilot AI Mar 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The PR description/checklist is still the template placeholder (no filled-in description and reviewer confirmations are unchecked). Please update the PR metadata to reflect what was changed (new rules/queries added) and confirm whether a change note is required (for new queries it’s typically not).

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants