[PATCH] D53814: Allow the analyzer to output to a SARIF file

2018-10-30 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman closed this revision. aaron.ballman added a comment. In https://reviews.llvm.org/D53814#1280581, @george.karpenkov wrote: > I much prefer this version. > We've had the same problem with diffing plist output. > One thing we have learned is using a FileCheck was definitely a bad

[PATCH] D53814: Allow the analyzer to output to a SARIF file

2018-10-30 Thread George Karpenkov via Phabricator via cfe-commits
george.karpenkov accepted this revision. george.karpenkov added a comment. This revision is now accepted and ready to land. I much prefer this version. We've had the same problem with diffing plist output. One thing we have learned is using a FileCheck was definitely a bad idea, as it leads to

[PATCH] D53814: Allow the analyzer to output to a SARIF file

2018-10-30 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments. Comment at: Analysis/diagnostics/sarif-diagnostics-taint-test.c:18 +// Test the basics for sanity. +// CHECK: sarifLog['version'].startswith("2.0.0") +// CHECK: sarifLog['runs'][0]['tool']['fullName'] == "clang static analyzer"

[PATCH] D53814: Allow the analyzer to output to a SARIF file

2018-10-30 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman updated this revision to Diff 171712. aaron.ballman added a comment. Switched to using diff for testing. https://reviews.llvm.org/D53814 Files: Analysis/diagnostics/Inputs/expected-sarif/sarif-diagnostics-taint-test.c.sarif Analysis/diagnostics/sarif-diagnostics-taint-test.c

[PATCH] D53814: Allow the analyzer to output to a SARIF file

2018-10-30 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman updated this revision to Diff 171685. aaron.ballman added a comment. Updated based on review feedback -- removed the `Sarif` path generation scheme as it isn't currently needed, and replaced a FIXME with a better comment. Testing remains an open question, however.

[PATCH] D53814: Allow the analyzer to output to a SARIF file

2018-10-30 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman marked 9 inline comments as done. aaron.ballman added inline comments. Comment at: Analysis/diagnostics/sarif-diagnostics-taint-test.c:18 +// Test the basics for sanity. +// CHECK: sarifLog['version'].startswith("2.0.0") +// CHECK:

[PATCH] D53814: Allow the analyzer to output to a SARIF file

2018-10-29 Thread George Karpenkov via Phabricator via cfe-commits
george.karpenkov requested changes to this revision. george.karpenkov added a comment. This revision now requires changes to proceed. I don't think a new PathGenerationScheme is needed, unless you plan changes to BugReporter.cpp. The code is fine otherwise, but could we try to use `diff` for

[PATCH] D53814: Allow the analyzer to output to a SARIF file

2018-10-29 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman updated this revision to Diff 171545. aaron.ballman marked 3 inline comments as done. aaron.ballman added a comment. Updated based on initial review feedback, and added more context to the patch. https://reviews.llvm.org/D53814 Files: Analysis/diagnostics/sarif-check.py

[PATCH] D53814: Allow the analyzer to output to a SARIF file

2018-10-29 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments. Comment at: Analysis/diagnostics/sarif-check.py:22 +passes = 0 +with open(testfile) as testfh: +lineno = 0 george.karpenkov wrote: > Wow, this is super neat! > Since you are quite active in LLVM community, would you think

[PATCH] D53814: Allow the analyzer to output to a SARIF file

2018-10-29 Thread George Karpenkov via Phabricator via cfe-commits
george.karpenkov requested changes to this revision. george.karpenkov added a comment. This revision now requires changes to proceed. Minor style notes + context missing. I think using `diff` would be better than a custom python tool. https://reviews.llvm.org/D53814

[PATCH] D53814: Allow the analyzer to output to a SARIF file

2018-10-29 Thread George Karpenkov via Phabricator via cfe-commits
george.karpenkov added a comment. Patch context is missing. Comment at: Analysis/diagnostics/sarif-check.py:22 +passes = 0 +with open(testfile) as testfh: +lineno = 0 Wow, this is super neat! Since you are quite active in LLVM community, would you think

[PATCH] D53814: Allow the analyzer to output to a SARIF file

2018-10-29 Thread Alexander Zaitsev via Phabricator via cfe-commits
ZaMaZaN4iK added inline comments. Comment at: StaticAnalyzer/Core/CMakeLists.txt:43 PlistDiagnostics.cpp + SarifDiagnostics.cpp ProgramState.cpp Sort alphabetically Comment at: StaticAnalyzer/Core/SarifDiagnostics.cpp:88 +

[PATCH] D53814: Allow the analyzer to output to a SARIF file

2018-10-29 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman created this revision. aaron.ballman added reviewers: dcoughlin, zaks.anna. Herald added subscribers: dkrupp, donat.nagy, Szelethus, a.sidorin, mgorny. Herald added a reviewer: george.karpenkov. SARIF (https://github.com/oasis-tcs/sarif-spec) is a new draft standard interchange