Guideline recategorization by rvermeulen · Pull Request #68 · github/codeql-coding-standards

rvermeulen · 2022-08-23T22:15:32Z

Description

This PR adds support for a Guideline Recategorization Plan as described in chapter 5 The guideline re-categorization plan of the MISRA Compliance:2020 and documented in this design

This includes:

The addition of a guideline-recategorizations section to the coding-standards.yml.
The inclusion of a rule's category to the query meta data model.
The definition of an effective category based on a rule's category and a guideline recategorization according to the rules in chapter 5 of the standard MISRA Compliance:2020.
Update to the exclusion mechanism to determine if we are allowed to deviate or to suppress disapplied rules.
Sarif post-processing to reflect the effective category in the resulting Sarif file.
Update to the user manual on how to use recategorizations.
Update to the coding standard pack generation to include the post-processing script.
Update to the analysis report generation to include guideline recategorizations.

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 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!)

jsinglet

Note: @rvermeulen @mbaluda and I reviewed this PR together. Here are a few additional notes. Great work! 🚀

mbaluda

Thanks for the walkthrough, looks good to me!

The category information enables evaluation time computation of the effective category of a rule. The effective category is determined by the rule's category and a possible recategorization through a Guideline Recategorization Plan. The effective category will be used to determine how to handle a deviation applied to a query implementing the rule.

This commit adds: - A new section `guideline-recategorizations` to the configuration specification for `coding-standards.yml`. - Two tests to list all the indexed guideline recategorizations and all the invalid guideline recategorizations. This commit does not include the application of the effective category.

With the additional support of a Guideline Recategorization Plan and possible other further configuration options we are moving the script into the generic configuration folder instead of the deviations folder.

The format depends on the length of the package name so we format the generated file with the CLI instead of mirror the formating behavior in the template.

This allows us to reduce the number of CodeQL CLI invocations for performance improvements.

Each file generation for a package will update the shared meta data value for the language the package belongs to. Running generation in parallel will result in a race condition which may result in inconsistent meta data files.

In this commit we expand the meta data `Query` class to provide an effective category that is determined by combining a query's category and a possible active Guideline Recategorization Plan. The exclusion process now validates for a query with an associated deviation record whether the effective category allows a deviation. If this is not allowed, then the deviation record is not applied. Any deviation record applied to a query with an effective category not permitting deviation is considered invalid and will be listed in the analysis report.

This is used for debugging and testing purposes and cannot be used in our standard queries due to the required negation and the possible infinite possible values for reason.

jsinglet

Great work Remco!

Co-authored-by: Robert C. Seacord <rcseacord@gmail.com>

Remove the note discussing the granularity options of applying a recategorization.

rvermeulen force-pushed the guideline-recategorization branch 5 times, most recently from 6b10b82 to ba35ca1 Compare August 31, 2022 00:37

rvermeulen force-pushed the guideline-recategorization branch from 8e4acc3 to 423fd38 Compare October 25, 2022 01:23

rvermeulen force-pushed the guideline-recategorization branch 8 times, most recently from a86c1cc to 3beccc4 Compare November 5, 2022 01:26

rvermeulen marked this pull request as ready for review November 5, 2022 01:29

rvermeulen requested review from jsinglet and mbaluda November 5, 2022 01:54

jsinglet suggested changes Nov 17, 2022

View reviewed changes

Comment thread .github/workflows/validate-coding-standards.yml

Comment thread docs/user_manual.md Outdated

mbaluda approved these changes Nov 17, 2022

View reviewed changes

rvermeulen added 11 commits November 28, 2022 13:20

Move coding standards config processing script

ebf64e6

With the additional support of a Guideline Recategorization Plan and possible other further configuration options we are moving the script into the generic configuration folder instead of the deviations folder.

Format generated exclusion files

86bc3f8

The format depends on the length of the package name so we format the generated file with the CLI instead of mirror the formating behavior in the template.

Add support for generating files for multiple packages

8738fe1

This allows us to reduce the number of CodeQL CLI invocations for performance improvements.

Address incorrect format exclusion files

1d4b947

Switch to GH managed CodeQL cli

aeab8c3

Address race condition in package file generation

acb0ff1

Each file generation for a package will update the shared meta data value for the language the package belongs to. Running generation in parallel will result in a race condition which may result in inconsistent meta data files.

Address incorrect formatting

0bdf011

Add isExcluded/3 that includes the reason for exclusion

c33952e

This is used for debugging and testing purposes and cannot be used in our standard queries due to the required negation and the possible infinite possible values for reason.

Add guideline recategorization design document.

fab2b27

rvermeulen requested a review from jsinglet November 28, 2022 22:51

jsinglet approved these changes Nov 29, 2022

View reviewed changes

Clarify the category of non Misra standard guidelines.

a85c6ff