[PATCH] D131084: Add support for specifying the severity of a SARIF Result.

[Vaibhav Yenamandra via Phabricator via cfe-commits]

vaibhav.y created this revision.
vaibhav.y added reviewers: aaron.ballman, cjdb, denik, abrahamcd, dbeer1.
Herald added a project: All.
vaibhav.y edited the summary of this revision.
vaibhav.y edited the summary of this revision.
vaibhav.y updated this revision to Diff 449713.
vaibhav.y added a comment.
vaibhav.y updated this revision to Diff 449717.
vaibhav.y updated this revision to Diff 449718.
vaibhav.y updated this revision to Diff 449721.
vaibhav.y edited projects, added clang; removed All.
vaibhav.y added a subscriber: clang.
Herald added a project: All.
vaibhav.y removed a project: All.
Herald added a project: All.
vaibhav.y published this revision for review.
Herald added a subscriber: cfe-commits.

Update diff


vaibhav.y added a comment.

Rebase on main, set result level to warning by default.


vaibhav.y added a comment.

- Add differential revision to individual commits
- Remove unused header


vaibhav.y added a comment.

Fix typo in comment


vaibhav.y added a comment.

Submitting for review:

Some notes:

There are a couple of ways I think we can acheive this, per the spec:

1. The reportingDescriptor (rule) objects can be given a default configuration property <https://docs.oasis-open.org/sarif/sarif/v2.1.0/os/sarif-v2.1.0-os.html#_Toc34317850>, which can set the default warning level and other data such as rule parameters etc.
2. The reportingDescriptor objects can omit the default configuration (which then allows operating with warning as default), and the level is then set when the result is reported.

The first approach would be "more correct", what are your thoughts on this? Would we benefit from having per-diagnostic configuration?

There is also the question about the "kind" of results in clang. From my reading of the spec <https://docs.oasis-open.org/sarif/sarif/v2.1.0/os/sarif-v2.1.0-os.html#_Toc34317647>, it seems that "fail" is the only case that applies to us because:

- "pass": Implies no issue was found.
- "open": This value is used by proof-based tools. It could also mean additional assertions required
- "informational": The specified rule was evaluated and produced a purely informational result that does not indicate the presence of a problem
- "notApplicable": The rule specified by ruleId was not evaluated, because it does not apply to the analysis target.

Of these "open" and "notApplicable" seem to be ones that *could* come to use but I'm not sure where / what kind of diagnostics would use these. Probably clang-tidy's `bugprone-*` suite?

Let me know what you think is a good way to approach this wrt clang's diagnostics system.


Previously it was not possible to specify the level of a SarifResult.

This changeset adds support for setting the `level` property[1] of a result. If unset, it defaults to "warning", which is the result of an empty default configuration on rules[2].

[1]: https://docs.oasis-open.org/sarif/sarif/v2.1.0/os/sarif-v2.1.0-os.html#_Toc34317648
[2]: https://docs.oasis-open.org/sarif/sarif/v2.1.0/os/sarif-v2.1.0-os.html#_Toc34317855

Test Plan: Updated test cases in BasicTests


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D131084

Files:
  clang/include/clang/Basic/Sarif.h
  clang/lib/Basic/Sarif.cpp
  clang/unittests/Basic/SarifTest.cpp

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D131084.449721.patch
Type: text/x-patch
Size: 11870 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20220803/48b8d13e/attachment.bin>