Post-filter clang-tidy diagnostic messages.

Summary:
This patch implements filtering of clang-tidy diagnostic messages by
the check name, so that "clang-tidy -checks=^llvm-" won't output any clang
warnings, for example. This is also helpful to run specific static-analyzer
checks: static analyzer always needs core checks to be enabled, but the user may
be interested only in the checks he asked for.

This patch also exposes warning option names for built-in diagnostics. We need
to have a namespace for these names to avoid collisions and to allow convenient
filtering, so I prefix them with "-W". I'm not sure it's the best thing to do,
and maybe "W" or "clang-diagnostic-" or something like this would be better.

Reviewers: klimek

Reviewed By: klimek

CC: cfe-commits

Differential Revision: http://llvm-reviews.chandlerc.com/D3121

llvm-svn: 204321

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

@@ -93,10 +93,8 @@ private:
 } // namespace
 
 ClangTidyASTConsumerFactory::ClangTidyASTConsumerFactory(
-    StringRef EnableChecksRegex, StringRef DisableChecksRegex,
     ClangTidyContext &Context)
-    : Filter(EnableChecksRegex, DisableChecksRegex), Context(Context),
-      CheckFactories(new ClangTidyCheckFactories) {
+    : Context(Context), CheckFactories(new ClangTidyCheckFactories) {
   for (ClangTidyModuleRegistry::iterator I = ClangTidyModuleRegistry::begin(),
                                          E = ClangTidyModuleRegistry::end();
        I != E; ++I) {
@@ -104,7 +102,7 @@ ClangTidyASTConsumerFactory::ClangTidyASTConsumerFactory(
     Module->addCheckFactories(*CheckFactories);
   }
 
-  CheckFactories->createChecks(Filter, Checks);
+  CheckFactories->createChecks(Context.getChecksFilter(), Checks);
 
   for (ClangTidyCheck *Check : Checks) {
     Check->setContext(&Context);
@@ -149,7 +147,7 @@ clang::ASTConsumer *ClangTidyASTConsumerFactory::CreateASTConsumer(
 std::vector<std::string> ClangTidyASTConsumerFactory::getCheckNames() {
   std::vector<std::string> CheckNames;
   for (const auto &CheckFactory : *CheckFactories) {
-    if (Filter.IsCheckEnabled(CheckFactory.first))
+    if (Context.getChecksFilter().isCheckEnabled(CheckFactory.first))
       CheckNames.push_back(CheckFactory.first);
   }
 
@@ -168,7 +166,8 @@ ClangTidyASTConsumerFactory::getCheckersControlList() {
   for (StringRef CheckName : StaticAnalyzerChecks) {
     std::string Checker((AnalyzerCheckNamePrefix + CheckName).str());
     AnalyzerChecksEnabled |=
-        Filter.IsCheckEnabled(Checker) && !CheckName.startswith("debug");
+        Context.getChecksFilter().isCheckEnabled(Checker) &&
+        !CheckName.startswith("debug");
   }
 
   if (AnalyzerChecksEnabled) {
@@ -183,21 +182,14 @@ ClangTidyASTConsumerFactory::getCheckersControlList() {
       std::string Checker((AnalyzerCheckNamePrefix + CheckName).str());
 
       if (CheckName.startswith("core") ||
-          (!CheckName.startswith("debug") && Filter.IsCheckEnabled(Checker)))
+          (!CheckName.startswith("debug") &&
+           Context.getChecksFilter().isCheckEnabled(Checker)))
         List.push_back(std::make_pair(CheckName, true));
     }
   }
   return List;
 }
 
-ChecksFilter::ChecksFilter(StringRef EnableChecksRegex,
-                           StringRef DisableChecksRegex)
-    : EnableChecks(EnableChecksRegex), DisableChecks(DisableChecksRegex) {}
-
-bool ChecksFilter::IsCheckEnabled(StringRef Name) {
-  return EnableChecks.match(Name) && !DisableChecks.match(Name);
-}
-
 DiagnosticBuilder ClangTidyCheck::diag(SourceLocation Loc, StringRef Message,
                                        DiagnosticIDs::Level Level) {
   return Context->diag(CheckName, Loc, Message, Level);
@@ -216,9 +208,9 @@ void ClangTidyCheck::setName(StringRef Name) {
 std::vector<std::string> getCheckNames(StringRef EnableChecksRegex,
                                        StringRef DisableChecksRegex) {
   SmallVector<ClangTidyError, 8> Errors;
-  clang::tidy::ClangTidyContext Context(&Errors);
-  ClangTidyASTConsumerFactory Factory(EnableChecksRegex, DisableChecksRegex,
-                                      Context);
+  clang::tidy::ClangTidyContext Context(&Errors, EnableChecksRegex,
+                                        DisableChecksRegex);
+  ClangTidyASTConsumerFactory Factory(Context);
   return Factory.getCheckNames();
 }
 
@@ -229,7 +221,8 @@ void runClangTidy(StringRef EnableChecksRegex, StringRef DisableChecksRegex,
   // FIXME: Ranges are currently full files. Support selecting specific
   // (line-)ranges.
   ClangTool Tool(Compilations, Ranges);
-  clang::tidy::ClangTidyContext Context(Errors);
+  clang::tidy::ClangTidyContext Context(Errors, EnableChecksRegex,
+                                        DisableChecksRegex);
   ClangTidyDiagnosticConsumer DiagConsumer(Context);
 
   Tool.setDiagnosticConsumer(&DiagConsumer);
@@ -256,8 +249,7 @@ void runClangTidy(StringRef EnableChecksRegex, StringRef DisableChecksRegex,
     ClangTidyASTConsumerFactory *ConsumerFactory;
   };
 
-  Tool.run(new ActionFactory(new ClangTidyASTConsumerFactory(
-      EnableChecksRegex, DisableChecksRegex, Context)));
+  Tool.run(new ActionFactory(new ClangTidyASTConsumerFactory(Context)));
 }
 
 static SourceLocation getLocation(SourceManager &SourceMgr, StringRef FilePath,

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

@@ -89,24 +89,11 @@ private:
   std::string CheckName;
 };
 
-/// \brief Filters checks by name.
-class ChecksFilter {
-public:
-  ChecksFilter(StringRef EnableChecksRegex, StringRef DisableChecksRegex);
-  bool IsCheckEnabled(StringRef Name);
-
-private:
-  llvm::Regex EnableChecks;
-  llvm::Regex DisableChecks;
-};
-
 class ClangTidyCheckFactories;
 
 class ClangTidyASTConsumerFactory {
 public:
-  ClangTidyASTConsumerFactory(StringRef EnableChecksRegex,
-                              StringRef DisableChecksRegex,
-                              ClangTidyContext &Context);
+  ClangTidyASTConsumerFactory(ClangTidyContext &Context);
   ~ClangTidyASTConsumerFactory();
 
   /// \brief Returns an ASTConsumer that runs the specified clang-tidy checks.
@@ -120,7 +107,6 @@ private:
   typedef std::vector<std::pair<std::string, bool> > CheckersList;
   CheckersList getCheckersControlList();
 
-  ChecksFilter Filter;
   SmallVector<ClangTidyCheck *, 8> Checks;
   ClangTidyContext &Context;
   ast_matchers::MatchFinder Finder;

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

@@ -113,6 +113,19 @@ ClangTidyMessage::ClangTidyMessage(StringRef Message,
 ClangTidyError::ClangTidyError(StringRef CheckName)
     : CheckName(CheckName) {}
 
+ChecksFilter::ChecksFilter(StringRef EnableChecksRegex,
+                           StringRef DisableChecksRegex)
+    : EnableChecks(EnableChecksRegex), DisableChecks(DisableChecksRegex) {}
+
+bool ChecksFilter::isCheckEnabled(StringRef Name) {
+  return EnableChecks.match(Name) && !DisableChecks.match(Name);
+}
+
+ClangTidyContext::ClangTidyContext(SmallVectorImpl<ClangTidyError> *Errors,
+                                   StringRef EnableChecksRegex,
+                                   StringRef DisableChecksRegex)
+    : Errors(Errors), Filter(EnableChecksRegex, DisableChecksRegex) {}
+
 DiagnosticBuilder ClangTidyContext::diag(
     StringRef CheckName, SourceLocation Loc, StringRef Description,
     DiagnosticIDs::Level Level /* = DiagnosticIDs::Warning*/) {
@@ -179,7 +192,14 @@ void ClangTidyDiagnosticConsumer::HandleDiagnostic(
            "A diagnostic note can only be appended to a message.");
   } else {
     finalizeLastError();
-    Errors.push_back(ClangTidyError(Context.getCheckName(Info.getID())));
+    StringRef WarningOption =
+        Context.DiagEngine->getDiagnosticIDs()->getWarningOptionForDiag(
+            Info.getID());
+    std::string CheckName = !WarningOption.empty()
+                                ? ("clang-diagnostic-" + WarningOption).str()
+                                : Context.getCheckName(Info.getID()).str();
+
+    Errors.push_back(ClangTidyError(CheckName));
   }
 
   // FIXME: Provide correct LangOptions for each file.
@@ -218,7 +238,8 @@ void ClangTidyDiagnosticConsumer::finish() {
   finalizeLastError();
   std::set<const ClangTidyError*, LessClangTidyError> UniqueErrors;
   for (const ClangTidyError &Error : Errors) {
-    if (UniqueErrors.insert(&Error).second)
+    if (Context.getChecksFilter().isCheckEnabled(Error.CheckName) &&
+        UniqueErrors.insert(&Error).second)
       Context.storeError(Error);
   }
   Errors.clear();

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

@@ -14,6 +14,7 @@
 #include "clang/Basic/SourceManager.h"
 #include "clang/Tooling/Refactoring.h"
 #include "llvm/ADT/DenseMap.h"
+#include "llvm/Support/Regex.h"
 
 namespace clang {
 
@@ -55,6 +56,17 @@ struct ClangTidyError {
   SmallVector<ClangTidyMessage, 1> Notes;
 };
 
+/// \brief Filters checks by name.
+class ChecksFilter {
+public:
+  ChecksFilter(StringRef EnableChecksRegex, StringRef DisableChecksRegex);
+  bool isCheckEnabled(StringRef Name);
+
+private:
+  llvm::Regex EnableChecks;
+  llvm::Regex DisableChecks;
+};
+
 /// \brief Every \c ClangTidyCheck reports errors through a \c DiagnosticEngine
 /// provided by this context.
 ///
@@ -66,8 +78,8 @@ struct ClangTidyError {
 /// \endcode
 class ClangTidyContext {
 public:
-  ClangTidyContext(SmallVectorImpl<ClangTidyError> *Errors)
-      : Errors(Errors), DiagEngine(0) {}
+  ClangTidyContext(SmallVectorImpl<ClangTidyError> *Errors,
+                   StringRef EnableChecksRegex, StringRef DisableChecksRegex);
 
   /// \brief Report any errors detected using this method.
   ///
@@ -93,6 +105,8 @@ public:
   /// diagnostic ID.
   StringRef getCheckName(unsigned DiagnosticID) const;
 
+  ChecksFilter &getChecksFilter() { return Filter; }
+
 private:
   friend class ClangTidyDiagnosticConsumer; // Calls storeError().
 
@@ -100,7 +114,9 @@ private:
   void storeError(const ClangTidyError &Error);
 
   SmallVectorImpl<ClangTidyError> *Errors;
-  DiagnosticsEngine *DiagEngine;
+  DiagnosticsEngine *DiagEngine{nullptr};
+  ChecksFilter Filter;
+
   llvm::DenseMap<unsigned, std::string> CheckNamesByDiagnosticID;
 };
 

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

@@ -29,7 +29,7 @@ void ClangTidyCheckFactories::addCheckFactory(StringRef Name,
 void ClangTidyCheckFactories::createChecks(
     ChecksFilter &Filter, SmallVectorImpl<ClangTidyCheck *> &Checks) {
   for (const auto &Factory : Factories) {
-    if (Filter.IsCheckEnabled(Factory.first)) {
+    if (Filter.isCheckEnabled(Factory.first)) {
       ClangTidyCheck *Check = Factory.second->createCheck();
       Check->setName(Factory.first);
       Checks.push_back(Check);

clang-tools-extra/test/clang-tidy/diagnostic.cpp

@@ -1,5 +1,21 @@
-// RUN: clang-tidy %s -- -fan-unknown-option | FileCheck -check-prefix=CHECK1 %s
-// RUN: clang-tidy %s.nonexistent.cpp -- | FileCheck -check-prefix=CHECK2 %s
+// RUN: clang-tidy %s.nonexistent.cpp -- | FileCheck -check-prefix=CHECK1 %s
+// RUN: clang-tidy %s -- -fan-unknown-option | FileCheck -check-prefix=CHECK2 %s
+// RUN: clang-tidy -checks='^(google-|clang-diagnostic-literal-conversion)' %s -- -fan-unknown-option | FileCheck -check-prefix=CHECK3 %s
 
-// CHECK1: warning: unknown argument: '-fan-unknown-option'
-// CHECK2: warning: error reading '{{.*}}.nonexistent.cpp'
+// CHECK1-NOT: warning
+// CHECK2-NOT: warning
+// CHECK3-NOT: warning
+
+// CHECK1: warning: error reading '{{.*}}.nonexistent.cpp'
+// CHECK2: warning: unknown argument: '-fan-unknown-option'
+
+// CHECK2: :[[@LINE+2]]:9: warning: implicit conversion from 'double' to 'int' changes value
+// CHECK3: :[[@LINE+1]]:9: warning: implicit conversion from 'double' to 'int' changes value
+int a = 1.5;
+
+// CHECK2: :[[@LINE+2]]:11: warning: Single-argument constructors must be explicit [google-explicit-constructor]
+// CHECK3: :[[@LINE+1]]:11: warning: Single-argument constructors must be explicit [google-explicit-constructor]
+class A { A(int) {} };
+
+// CHECK2-NOT: warning:
+// CHECK3-NOT: warning:

clang-tools-extra/unittests/clang-tidy/ClangTidyTest.h

@@ -42,7 +42,7 @@ private:
 template <typename T> std::string runCheckOnCode(StringRef Code) {
   T Check;
   SmallVector<ClangTidyError, 16> Errors;
-  ClangTidyContext Context(&Errors);
+  ClangTidyContext Context(&Errors, ".*", "");
   ClangTidyDiagnosticConsumer DiagConsumer(Context);
   Check.setContext(&Context);