Add `Banned2`, `Banned3`, and `Banned4` by jeongsoolee09 · Pull Request #1080 · github/codeql-coding-standards

jeongsoolee09 · 2026-03-13T21:06:46Z

Description

Add package Banned2, Banned3, and Banned4, which are RULE-10-2-2, RULE-10-2-3, and RULE-10-3-1 respectively.

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-10-2-2
- RULE-10-2-3
- RULE-10-3-1
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!)

Copilot

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

cpp/misra/src/rules/RULE-10-2-3/UnscopedEnumWithoutFixedUnderlyingTypeUsed.ql

rules.csv

…ithub/codeql-coding-standards into jeongsoolee09/MISRA-C++-2023-Banned234

For some weird reason clang-format decided that the `&` operator is an address-of unary-operator and not a bitwise-and binary operator.

MichaelRFairhurst

Only some comments about static alert, and tags, otherwise LGTM!

MichaelRFairhurst · 2026-03-24T16:26:56Z

cpp/common/src/codingstandards/cpp/exclusions/cpp/Banned2.qll

+
+newtype Banned2Query = TUnscopedEnumerationsShouldNotBeDeclaredQuery()
+
+predicate isBanned2QueryMetadata(Query query, string queryId, string ruleId, string category) {


Just FYI, we don't need to split rules into their own packages unless they're being split into different PRs. In this case, these can all be part of Banned2.

Also, in this case, three PRs would likely be preferable over one, since the three rules don't have any shared imports in common/ or updates to test/includes/, etc. It is especially valuable to separate 10-2-2, 10-3-1, which are quick to review and approve, from 10-2-3 which has over 200LOC and might take a few back-and-forths to land.

Even the small queries, such as 10-2-2 and 10-3-1, are probably easier to review in isolation going forward because they often a lot of test code each. All in all, one review of 1300loc is harder to find time for than three reviews of ~430loc each, and will take longer to land.

No action required in this case, but to keep in mind going forward 👍

MichaelRFairhurst · 2026-03-24T23:34:46Z

cpp/misra/src/rules/RULE-10-2-2/UnscopedEnumerationsShouldNotBeDeclared.ql

+ *              types.
+ * @kind problem
+ * @precision very-high
+ * @problem.severity error


Probably should be a warning, and either include maintainability or readability tags.

MichaelRFairhurst · 2026-03-24T23:34:56Z

cpp/misra/src/rules/RULE-10-2-2/UnscopedEnumerationsShouldNotBeDeclared.ql

+where
+  not isExcluded(enum, Banned2Package::unscopedEnumerationsShouldNotBeDeclaredQuery()) and
+  not (enum instanceof ScopedEnum or enum instanceof NestedUnscopedEnum)
+select enum, "This enumeration is an unscoped enum not enclosed in a class or a struct."


Let's include the enum name here, e.g. "Enum " + enum.getName() + " is an unscoped ..., though we'll maybe want to treat anonymous enums specially.

Generally speaking, we want to avoid constant alert messages.

MichaelRFairhurst · 2026-03-25T01:11:19Z

cpp/misra/test/rules/RULE-10-3-1/test1.cpp

+#include "test.hpp"
+
+/*
+ * This file demonstrates that anonymous namespace variables


Just FYI, our existing tests don't generally do this kind of demonstration/documentation. Usually we leave the MISRA document to justify itself, and our test cases are purely there to produce alerts (or not). I think the main exception that comes to mind, is that sometimes we will intentionally decide to implement a rule a little differently than described in the standard, and in this case we're more likely to add these kind of demonstrative "test" cases.

MichaelRFairhurst · 2026-03-25T01:21:53Z

cpp/misra/src/rules/RULE-10-2-3/UnscopedEnumWithoutFixedUnderlyingTypeUsed.ql

+ * ``` C++
+ * static_cast<int>(u) == static_cast<int>(s); // COMPLIANT: comparing ints
+ * ```
+ * ^^^ To solve this, we use `getExplicitlyConverted`:


Nice! Yes. Conversions are annoying and this is exactly the correct approach 👍 👍 👍

(I didn't know getExplicitlyConverted existed for a good while when I started..!!!)

jeongsoolee09 added 14 commits March 12, 2026 20:09

Add rule package of Banned2

6dc64be

Add implementation of Banned2

21af40d

Renumber rules in rules.csv

20f7399

Draft out first three cases

447d961

Add query files

1a08d11

Add enumFitsInType and related test cases

ba3afac

Debug enumFitsInType

c4179a0

Finish the detection logic

c91e5c5

Minor edits of comments

cfc6de7

Finalize 10.2.3 and update expected results

a1f17d3

Update expected results of 10.2.2

96fdc0c

Use NestedEnum instead of hand-rolled definition

4ae8c1d

Add query files for Banned4

7f4ebd4

Merge branch 'main' into jeongsoolee09/MISRA-C++-2023-Banned234

075c51f

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

Copilot started reviewing on behalf of jeongsoolee09 March 22, 2026 00:47 View session

Copilot AI reviewed Mar 22, 2026

View reviewed changes

cpp/misra/src/rules/RULE-10-2-3/UnscopedEnumWithoutFixedUnderlyingTypeUsed.ql Outdated Show resolved Hide resolved

rules.csv Show resolved Hide resolved

jeongsoolee09 self-assigned this Mar 23, 2026

jeongsoolee09 added 8 commits March 23, 2026 11:28

Fix formatting of test case for 10.2.2

2e62897

Merge branch 'jeongsoolee09/MISRA-C++-2023-Banned234' of github.com:g…

b4683d2

…ithub/codeql-coding-standards into jeongsoolee09/MISRA-C++-2023-Banned234

Disable clang-format for these two

538fafa

For some weird reason clang-format decided that the `&` operator is an address-of unary-operator and not a bitwise-and binary operator.

Avoid the default case by asserting the existence of case expr

db276dc

Include unsigned long long in result = 64

f7d6a49

Update expected files of 10.2.2 and 10.2.3

4ab4507

Fix formatting

113c464

Merge branch 'main' into jeongsoolee09/MISRA-C++-2023-Banned234

2cf4b66

MichaelRFairhurst requested changes Mar 25, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add `Banned2`, `Banned3`, and `Banned4`#1080

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

jeongsoolee09 commented Mar 13, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

MichaelRFairhurst left a comment

Uh oh!

MichaelRFairhurst Mar 24, 2026

Uh oh!

MichaelRFairhurst Mar 24, 2026

Uh oh!

MichaelRFairhurst Mar 24, 2026

Uh oh!

MichaelRFairhurst Mar 25, 2026

Uh oh!

MichaelRFairhurst Mar 25, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants


		newtype Banned2Query = TUnscopedEnumerationsShouldNotBeDeclaredQuery()

		predicate isBanned2QueryMetadata(Query query, string queryId, string ruleId, string category) {

Conversation

jeongsoolee09 commented Mar 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Change request type

Rules with added or modified queries

Release change checklist

Query development review checklist

Author

Reviewer

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

MichaelRFairhurst left a comment

Choose a reason for hiding this comment

Uh oh!

MichaelRFairhurst Mar 24, 2026

Choose a reason for hiding this comment

Uh oh!

MichaelRFairhurst Mar 24, 2026

Choose a reason for hiding this comment

Uh oh!

MichaelRFairhurst Mar 24, 2026

Choose a reason for hiding this comment

Uh oh!

MichaelRFairhurst Mar 25, 2026

Choose a reason for hiding this comment

Uh oh!

MichaelRFairhurst Mar 25, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

jeongsoolee09 commented Mar 13, 2026 •

edited

Loading