wip

Author: HerrCai0907

clang-tools-extra/clang-tidy/ClangTidy.cpp

@@ -58,7 +58,8 @@ LLVM_INSTANTIATE_REGISTRY(clang::tidy::ClangTidyModuleRegistry)
 namespace clang::tidy {
 
 namespace custom {
-extern void setOptions(ClangTidyOptions const &O);
+extern void registerCustomChecks(ClangTidyOptions const &O,
+                                 ClangTidyCheckFactories &Factories);
 } // namespace custom
 
 namespace {
@@ -350,7 +351,7 @@ ClangTidyASTConsumerFactory::ClangTidyASTConsumerFactory(
     IntrusiveRefCntPtr<llvm::vfs::OverlayFileSystem> OverlayFS)
     : Context(Context), OverlayFS(std::move(OverlayFS)),
       CheckFactories(new ClangTidyCheckFactories) {
-  custom::setOptions(Context.getOptions());
+  custom::registerCustomChecks(Context.getOptions(), *CheckFactories);
   for (ClangTidyModuleRegistry::entry E : ClangTidyModuleRegistry::entries()) {
     std::unique_ptr<ClangTidyModule> Module = E.instantiate();
     Module->addCheckFactories(*CheckFactories);
@@ -421,7 +422,7 @@ ClangTidyASTConsumerFactory::createASTConsumer(
                         .getCurrentWorkingDirectory();
   if (WorkingDir)
     Context.setCurrentBuildDirectory(WorkingDir.get());
-
+  custom::registerCustomChecks(Context.getOptions(), *CheckFactories);
   std::vector<std::unique_ptr<ClangTidyCheck>> Checks =
       CheckFactories->createChecksForLanguage(&Context);
 
@@ -664,7 +665,7 @@ getAllChecksAndOptions(bool AllowEnablingAnalyzerAlphaCheckers) {
       std::make_unique<DefaultOptionsProvider>(ClangTidyGlobalOptions(), Opts),
       AllowEnablingAnalyzerAlphaCheckers);
   ClangTidyCheckFactories Factories;
-  custom::setOptions(Context.getOptions());
+  custom::registerCustomChecks(Context.getOptions(), Factories);
   for (const ClangTidyModuleRegistry::entry &Module :
        ClangTidyModuleRegistry::entries()) {
     Module.instantiate()->addCheckFactories(Factories);

clang-tools-extra/clang-tidy/ClangTidyModule.h

@@ -62,6 +62,8 @@ class ClangTidyCheckFactories {
                          });
   }
 
+  void erase(llvm::StringRef CheckName) { Factories.erase(CheckName); }
+
   /// Create instances of checks that are enabled.
   std::vector<std::unique_ptr<ClangTidyCheck>>
   createChecks(ClangTidyContext *Context) const;

clang-tools-extra/clang-tidy/custom/CustomTidyModule.cpp

@@ -3,34 +3,40 @@
 #include "../ClangTidyModuleRegistry.h"
 #include "../ClangTidyOptions.h"
 #include "QueryCheck.h"
+#include "llvm/ADT/SmallSet.h"
+#include "llvm/ADT/SmallString.h"
+#include "llvm/ADT/StringRef.h"
 #include <memory>
 
 namespace clang::tidy {
 namespace custom {
 
-// FIXME: could be clearer to add parameter of addCheckFactories to pass
-// Options?
-static ClangTidyOptions const *Options = nullptr;
-extern void setOptions(ClangTidyOptions const &O) { Options = &O; }
-
 class CustomModule : public ClangTidyModule {
 public:
-  void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {
-    if (Options == nullptr || !Options->CustomChecks.has_value() ||
-        Options->CustomChecks->empty())
-      return;
-    for (const ClangTidyOptions::CustomCheckValue &V :
-         Options->CustomChecks.value()) {
-      CheckFactories.registerCheckFactory(
-          // add custom- prefix to avoid conflicts with builtin checks
-          "custom-" + V.Name,
-          [&V](llvm::StringRef Name, ClangTidyContext *Context) {
-            return std::make_unique<custom::QueryCheck>(Name, V, Context);
-          });
-    }
-  }
+  void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {}
 };
 
+// FIXME: could be clearer to add parameter of addCheckFactories to pass
+// Options?
+extern void registerCustomChecks(ClangTidyOptions const &Options,
+                                 ClangTidyCheckFactories &Factories) {
+  static llvm::SmallSet<llvm::SmallString<32>, 8> CustomCheckNames{};
+  if (!Options.CustomChecks.has_value() || Options.CustomChecks->empty())
+    return;
+  for (llvm::SmallString<32> const &Name : CustomCheckNames)
+    Factories.erase(Name);
+  for (const ClangTidyOptions::CustomCheckValue &V :
+       Options.CustomChecks.value()) {
+    llvm::SmallString<32> Name = llvm::StringRef{"custom-" + V.Name};
+    Factories.registerCheckFactory(
+        // add custom- prefix to avoid conflicts with builtin checks
+        Name, [&V](llvm::StringRef Name, ClangTidyContext *Context) {
+          return std::make_unique<custom::QueryCheck>(Name, V, Context);
+        });
+    CustomCheckNames.insert(std::move(Name));
+  }
+}
+
 } // namespace custom
 
 // Register the AlteraTidyModule using this statically initialized variable.

clang-tools-extra/clang-tidy/custom/QueryCheck.cpp

@@ -9,10 +9,12 @@
 #include "QueryCheck.h"
 #include "../../clang-query/Query.h"
 #include "../../clang-query/QueryParser.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
 #include "clang/ASTMatchers/Dynamic/VariantValue.h"
 #include "clang/Basic/DiagnosticIDs.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringRef.h"
+#include <string>
 
 using namespace clang::ast_matchers;
 
@@ -23,9 +25,16 @@ QueryCheck::QueryCheck(llvm::StringRef Name,
                        ClangTidyContext *Context)
     : ClangTidyCheck(Name, Context) {
   for (const ClangTidyOptions::CustomCheckDiag &D : V.Diags) {
-    auto It = Diags.try_emplace(D.BindName, llvm::SmallVector<Diag>{}).first;
-    It->second.emplace_back(
-        Diag{D.Message, D.Level.value_or(DiagnosticIDs::Warning)});
+    auto DiagnosticIdIt =
+        Diags
+            .try_emplace(D.Level.value_or(DiagnosticIDs::Warning),
+                         llvm::StringMap<llvm::SmallVector<std::string>>{})
+            .first;
+    auto DiagMessageIt =
+        DiagnosticIdIt->getSecond()
+            .try_emplace(D.BindName, llvm::SmallVector<std::string>{})
+            .first;
+    DiagMessageIt->second.emplace_back(D.Message);
   }
 
   clang::query::QuerySession QS({});
@@ -71,10 +80,23 @@ void QueryCheck::registerMatchers(MatchFinder *Finder) {
 }
 
 void QueryCheck::check(const MatchFinder::MatchResult &Result) {
+  auto Emit = [this](DiagMaps const &DiagMaps, std::string const &BindName,
+                     DynTypedNode const &Node, DiagnosticIDs::Level Level) {
+    if (!DiagMaps.contains(Level))
+      return;
+    auto &DiagMap = DiagMaps.at(Level);
+    if (!DiagMap.contains(BindName))
+      return;
+    for (const std::string &Message : DiagMap.at(BindName)) {
+      diag(Node.getSourceRange().getBegin(), Message, Level);
+    }
+  };
+  for (auto &[Name, Node] : Result.Nodes.getMap())
+    Emit(Diags, Name, Node, DiagnosticIDs::Error);
   for (auto &[Name, Node] : Result.Nodes.getMap())
-    if (Diags.contains(Name))
-      for (const Diag &D : Diags[Name])
-        diag(D.Message, D.Level) << Node.getSourceRange();
+    Emit(Diags, Name, Node, DiagnosticIDs::Warning);
+  // place Note last, otherwise it will not be emitted
+  for (auto &[Name, Node] : Result.Nodes.getMap())
+    Emit(Diags, Name, Node, DiagnosticIDs::Note);
 }
-
 } // namespace clang::tidy::custom

clang-tools-extra/clang-tidy/custom/QueryCheck.h

@@ -11,6 +11,8 @@
 
 #include "../ClangTidyCheck.h"
 #include "clang/ASTMatchers/Dynamic/VariantValue.h"
+#include "clang/Basic/DiagnosticIDs.h"
+#include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringMap.h"
 
@@ -29,11 +31,10 @@ class QueryCheck : public ClangTidyCheck {
 
 private:
   llvm::SmallVector<ast_matchers::dynamic::DynTypedMatcher> Matchers{};
-  struct Diag {
-    std::string Message;
-    DiagnosticIDs::Level Level;
-  };
-  llvm::StringMap<llvm::SmallVector<Diag>> Diags{};
+  using DiagMaps =
+      llvm::DenseMap<DiagnosticIDs::Level,
+                     llvm::StringMap<llvm::SmallVector<std::string>>>;
+  DiagMaps Diags;
 };
 
 } // namespace clang::tidy::custom

clang-tools-extra/test/clang-tidy/checkers/custom/Inputs/clang-tidy.yml

@@ -0,0 +1,22 @@
+CustomeChecks:
+  - Name: test-diag-level
+    Query: |
+      match varDecl(
+          hasType(asString("long")),
+          hasTypeLoc(typeLoc().bind("long"))
+      ).bind("decl")
+    Diagnostic:
+      - BindName: long
+        Message: use 'int' instead of 'long'
+        Level: Warning
+      - BindName: decl
+        Message: declaration of 'long'
+        Level: Note
+  - Name: test-let-bind
+    Query: |
+      let expr varDecl(isStaticStorageClass()).bind("vd")
+      match expr
+    Diagnostic:
+      - BindName: vd
+        Message: find static variable
+        Level: Warning

clang-tools-extra/test/clang-tidy/checkers/custom/Inputs/incorrect-clang-tidy.yml

@@ -0,0 +1,10 @@
+CustomeChecks:
+  - Name: test-let-bind
+    Query: |
+      let expr varDecl(isStaticStorageClass()).bind("vd")
+      match expr
+      set output print
+    Diagnostic:
+      - BindName: vd
+        Message: find static variable
+        Level: Warning

clang-tools-extra/test/clang-tidy/checkers/custom/query-incorrect-query.cpp

@@ -0,0 +1,3 @@
+// RUN: %check_clang_tidy %s custom-* %t --config-file=%S/Inputs/incorrect-clang-tidy.yml
+
+// CHECK-MESSAGES: warning: unsupported querry kind [clang-tidy-config]

clang-tools-extra/test/clang-tidy/checkers/custom/query-partially-active-check.cpp

@@ -0,0 +1,5 @@
+// RUN: %check_clang_tidy %s custom-test-let-bind %t --config-file=%S/Inputs/clang-tidy.yml
+
+extern long E;
+static int S;
+// CHECK-MESSAGES: [[@LINE-1]]:1: warning: find static variable [custom-test-let-bind]

clang-tools-extra/test/clang-tidy/checkers/custom/query.cpp

@@ -0,0 +1,7 @@
+// RUN: %check_clang_tidy %s custom-* %t --config-file=%S/Inputs/clang-tidy.yml
+
+extern long E;
+// CHECK-MESSAGES: [[@LINE-1]]:8: warning: use 'int' instead of 'long' [custom-test-diag-level]
+// CHECK-MESSAGES: [[@LINE-2]]:1: note: declaration of 'long'
+static int S;
+// CHECK-MESSAGES: [[@LINE-1]]:1: warning: find static variable [custom-test-let-bind]