[PATCH] D55792: Allow direct navigation to static analysis checker documentation through SARIF exports

Tue Dec 18 04:42:30 PST 2018

Szelethus added a comment.

Thank you so much for working on this! The lack of a proper documentation is indeed a weak points of the project.

I guess it'd be better to generate the entire URI in the tblgen file, and also allow plugins to register their own through `CheckerRegistry`. Sarif lacking the support for them is one thing, but adding a new field to the `Checker` class in `CheckerBase.td` I think should be accompanied by extending `CheckerRegistry::CheckerInfo` and `CheckerRegistry::getChecker`.

While I dislike the use of `<a id="some.Checker"></a>`, until Sphinx documentation becomes a thing, I guess we can't do much about it.

But the overall direction of this patch is great.

================
Comment at: lib/StaticAnalyzer/Core/SarifDiagnostics.cpp:270-275
+#define GET_CHECKERS
+#define CHECKER(FULLNAME, CLASS, HELPTEXT, DOCS)                               \
+  .Case(FULLNAME, DOCS)
+#include "clang/StaticAnalyzer/Checkers/Checkers.inc"
+#undef CHECKER
+#undef GET_CHECKERS
----------------
As per usual, this won't handle plugin checkers, but the solution is in an arms reach -- in the last couple months, I changed a lot around checker registration, and once things settle down a bit.

One idea is to store the list of registered checkers (now, in this case, registered means that the analyzer even knows about its existence, not that it's enabled) in `AnalyzerOptions`, however it'd force you to put an extra parameter to almost all functions here. What do you think?

================
Comment at: lib/StaticAnalyzer/Core/SarifDiagnostics.cpp:284-286
+  return (llvm::Twine("https://clang-analyzer.llvm.org/") + DocLoc + "#" +
+          CheckName)
+      .str();
----------------
Why not do this in `ClangSACheckersEmitter.cpp`?

================
Comment at: utils/TableGen/ClangSACheckersEmitter.cpp:90-91

+  OS << "// This file is automatically generated. Do not edit this file by "
+        "hand.\n";
+
----------------
Cheers!

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55792/new/

https://reviews.llvm.org/D55792