Repository navigation
feat(sarif): give each rule a title and a description - #245
Merged
Merged
Conversation
…d a description The SARIF writer sets each rule's shortDescription to the rule id and writes no fullDescription, so GitHub code scanning names an alert after its id and explains nothing. Every rule id the emitters can give, read from their sources as for the CWEs (explicit ids, const-parameter variants, clang classifications, and the error code names a diagnostic without an explicit id takes), must get a title other than its id and a description. Refs #242
describeRule(ruleId), declared in the new public header RuleCatalog.hpp, gives a title and a description for each of the 52 rule ids the emitters can give, the error code names included: a program linking the analyzer, such as CoreTrace, can describe a finding's rule. The catalog lives with the report serialization, which will read it. Refs #242
A SARIF rule took its id as shortDescription and had no fullDescription, so GitHub code scanning named an alert after the id and explained nothing. It now takes the rule's title as shortDescription and its description as fullDescription, from describeRule; an id the catalog does not describe keeps itself as shortDescription, as before. Refs #242
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #242.
A SARIF rule took its id as
shortDescriptionand had nofullDescription, so GitHub code scanning named an alert after the id and explained nothing. CoreTrace, which links the analyzer, had no text for it either.Changes
describeRule(ruleId), in the new public headerinclude/RuleCatalog.hpp, gives a title and a description for each of the 52 rule ids the emitters can give:ConstParameterNotModifiedvariants;shortDescriptionis its title and itsfullDescriptionits description. An id the catalog does not describe keeps itself asshortDescription, as before.Files left untouched on purpose
Open PRs #221 and #239 change
StackUsageAnalyzer.hpp,DiagnosticEmitter.cpp,CMakeLists.txt,README.mdandrun_test.py. This PR changes none of them:src/report/ReportSerialization.cpp, the SARIF writer, so the source list does not change;#243 (the human layout in the JSON and SARIF messages) and #244 (related locations and a confidence level) rework
DiagnosticEmitter.cppand theDiagnosticmodel, so they will come after #221 and #239 are merged.Tests
testEveryRuleIsDescribedwas committed first and failed for every rule (52):testEveryReportedCweIsScoredreads the CWEs, so a new rule without a description fails it;Results:
run_test.py: 2415/2415.