Reset SARIFDiag during EndSourceFile().

Deleted copy and move for Printer and added asserts.

Resolved pending comments.

Ran clang-format.

Addressed some refactoring and reformatting comments.

Deleted copy/move from renderer and returned OS to reference.

Removed use of pointer reset in Printer.

Added use of std::make_unique.

Documented issue with representing PresumedLocs modified by #line directives.

Modified Printer's interface to use unique pointers when setting the SARIF writer.

Addressed review comments on formatting, style, C++ best practices.

Formatting fixes and synchronization of documentation.

Addressed review style comments.

Added diagnostic level and default configuration to SARIF output.
Removed unit test file.

In D131632#3733068, @aaron.ballman wrote:

Precommit CI found a relevant failure.

Commented out unfinished test case

Removed Clang name from FileCheck test.

Hi, checking in on this again, thanks!

Fixed FileCheck test case and added multiple source range error.

Naming fixes.

Minor cleanup.

Removed unused remaining parts of traditional text diagnostics.

Minor formatting/naming fixes.

Adds check for the compatibility of qualifiers on the clear method and the object it is called on.

Adds non-dependent base class test case.

Checks for clear() in base classes as well.

In D131084#3697211, @vaibhav.y wrote:

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

Hi all, I just wanted to check in on this again and see if any more feedback or progress could be made. Thank you!

Removed unintended formatting.

Enabled both "sarif" and "SARIF" as flag spellings.

Update test file to check for warning and address other minor comments

In D129886#3656221, @vaibhav.y wrote:

Might be worth hiding it from --help, despite the instability warning.

Adds test file and fixes warning-flags test failure by adding warning group.

Commit message edits did not go through

Edited commit message

Gentle ping!

In D109701#3648168, @vaibhav.y wrote:

In D109701#3646856, @abrahamcd wrote:

Hi! I'm interning with @cjdb and @denik this summer and I was working on adding a -fdiagnostics-format=sarif option to start off my project, but I just found that a previous abandoned version of this change (D109697) was intending to add it. Seeing as the flag is no longer included in this version of the change, is it okay for me to go on and add it myself, or are you still planning on adding it here? Thanks!

Sure, feel free to use D109697 as you see fit!

Hi! I'm interning with @cjdb and @denik this summer and I was working on adding a -fdiagnostics-format=sarif option to start off my project, but I just found that a previous abandoned version of this change (D109697) was intending to add it. Seeing as the flag is no longer included in this version of the change, is it okay for me to go on and add it myself, or are you still planning on adding it here? Thanks!

Adds argument 0 test case and refactoring based on feedback.

Adds functionality for only warning on the case of unused return value
of the call to empty() and removes using-directive.

Stuck on determining unused return value, uploaded current state for @denik and @cjdb to take a look.

Progress Update

Addressed review edits and clarity feedback.

Removed old test file.

Added functionality to check if member function clear() exists before
suggesting it as a fix, including updated tests to reflect that. Fixed
path change and rebase errors from the initial revision.