diff --git a/clang-tools-extra/clang-tidy/ClangTidyCheck.h b/clang-tools-extra/clang-tidy/ClangTidyCheck.h --- a/clang-tools-extra/clang-tidy/ClangTidyCheck.h +++ b/clang-tools-extra/clang-tidy/ClangTidyCheck.h @@ -176,6 +176,15 @@ DiagnosticBuilder diag(SourceLocation Loc, StringRef Description, DiagnosticIDs::Level Level = DiagnosticIDs::Warning); + /// Add a diagnostic with the check's name. + DiagnosticBuilder diag(StringRef Description, + DiagnosticIDs::Level Level = DiagnosticIDs::Warning); + + /// Adds a diagnostic to report errors in the check's configuration. + DiagnosticBuilder + configurationDiag(StringRef Description, + DiagnosticIDs::Level Level = DiagnosticIDs::Warning); + /// Should store all options supported by this check with their /// current values or default values for options that haven't been overridden. /// @@ -192,7 +201,8 @@ public: /// Initializes the instance using \p CheckName + "." as a prefix. OptionsView(StringRef CheckName, - const ClangTidyOptions::OptionMap &CheckOptions); + const ClangTidyOptions::OptionMap &CheckOptions, + ClangTidyContext *Context); /// Read a named option from the ``Context``. /// @@ -268,7 +278,7 @@ if (llvm::Expected ValueOr = get(LocalName)) return *ValueOr; else - logIfOptionParsingError(ValueOr.takeError()); + reportOptionParsingError(ValueOr.takeError()); return Default; } @@ -314,7 +324,7 @@ if (llvm::Expected ValueOr = getLocalOrGlobal(LocalName)) return *ValueOr; else - logIfOptionParsingError(ValueOr.takeError()); + reportOptionParsingError(ValueOr.takeError()); return Default; } @@ -353,7 +363,7 @@ if (auto ValueOr = get(LocalName, IgnoreCase)) return *ValueOr; else - logIfOptionParsingError(ValueOr.takeError()); + reportOptionParsingError(ValueOr.takeError()); return Default; } @@ -395,7 +405,7 @@ if (auto ValueOr = getLocalOrGlobal(LocalName, IgnoreCase)) return *ValueOr; else - logIfOptionParsingError(ValueOr.takeError()); + reportOptionParsingError(ValueOr.takeError()); return Default; } @@ -407,7 +417,7 @@ if (auto ValueOr = get(LocalName)) return *ValueOr; else - logIfOptionParsingError(ValueOr.takeError()); + reportOptionParsingError(ValueOr.takeError()); return llvm::None; } @@ -420,7 +430,7 @@ if (auto ValueOr = getLocalOrGlobal(LocalName)) return *ValueOr; else - logIfOptionParsingError(ValueOr.takeError()); + reportOptionParsingError(ValueOr.takeError()); return llvm::None; } @@ -481,11 +491,12 @@ void storeInt(ClangTidyOptions::OptionMap &Options, StringRef LocalName, int64_t Value) const; - /// Logs an Error to stderr if a \p Err is not a MissingOptionError. - static void logIfOptionParsingError(llvm::Error &&Err); + /// Emits a diagnostic if \p Err is not a MissingOptionError. + void reportOptionParsingError(llvm::Error &&Err) const; std::string NamePrefix; const ClangTidyOptions::OptionMap &CheckOptions; + ClangTidyContext *Context; }; private: diff --git a/clang-tools-extra/clang-tidy/ClangTidyCheck.cpp b/clang-tools-extra/clang-tidy/ClangTidyCheck.cpp --- a/clang-tools-extra/clang-tidy/ClangTidyCheck.cpp +++ b/clang-tools-extra/clang-tidy/ClangTidyCheck.cpp @@ -42,7 +42,7 @@ ClangTidyCheck::ClangTidyCheck(StringRef CheckName, ClangTidyContext *Context) : CheckName(CheckName), Context(Context), - Options(CheckName, Context->getOptions().CheckOptions) { + Options(CheckName, Context->getOptions().CheckOptions, Context) { assert(Context != nullptr); assert(!CheckName.empty()); } @@ -52,6 +52,17 @@ return Context->diag(CheckName, Loc, Message, Level); } +DiagnosticBuilder ClangTidyCheck::diag(StringRef Message, + DiagnosticIDs::Level Level) { + return Context->diag(CheckName, Message, Level); +} + +DiagnosticBuilder +ClangTidyCheck::configurationDiag(StringRef Description, + DiagnosticIDs::Level Level) { + return Context->configurationDiag(Description, Level); +} + void ClangTidyCheck::run(const ast_matchers::MatchFinder::MatchResult &Result) { // For historical reasons, checks don't implement the MatchFinder run() // callback directly. We keep the run()/check() distinction to avoid interface @@ -59,9 +70,11 @@ check(Result); } -ClangTidyCheck::OptionsView::OptionsView(StringRef CheckName, - const ClangTidyOptions::OptionMap &CheckOptions) - : NamePrefix(CheckName.str() + "."), CheckOptions(CheckOptions) {} +ClangTidyCheck::OptionsView::OptionsView( + StringRef CheckName, const ClangTidyOptions::OptionMap &CheckOptions, + ClangTidyContext *Context) + : NamePrefix(CheckName.str() + "."), CheckOptions(CheckOptions), + Context(Context) {} llvm::Expected ClangTidyCheck::OptionsView::get(StringRef LocalName) const { @@ -121,7 +134,7 @@ llvm::Expected ValueOr = get(LocalName); if (ValueOr) return *ValueOr; - logIfOptionParsingError(ValueOr.takeError()); + reportOptionParsingError(ValueOr.takeError()); return Default; } @@ -140,7 +153,7 @@ llvm::Expected ValueOr = getLocalOrGlobal(LocalName); if (ValueOr) return *ValueOr; - logIfOptionParsingError(ValueOr.takeError()); + reportOptionParsingError(ValueOr.takeError()); return Default; } @@ -199,11 +212,11 @@ Iter->getValue().Value); } -void ClangTidyCheck::OptionsView::logIfOptionParsingError(llvm::Error &&Err) { +void ClangTidyCheck::OptionsView::reportOptionParsingError( + llvm::Error &&Err) const { if (auto RemainingErrors = llvm::handleErrors(std::move(Err), [](const MissingOptionError &) {})) - llvm::logAllUnhandledErrors(std::move(RemainingErrors), - llvm::WithColor::warning()); + Context->configurationDiag(llvm::toString(std::move(RemainingErrors))); } template <> diff --git a/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h b/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h --- a/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h +++ b/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h @@ -96,6 +96,14 @@ StringRef Message, DiagnosticIDs::Level Level = DiagnosticIDs::Warning); + DiagnosticBuilder diag(StringRef CheckName, StringRef Message, + DiagnosticIDs::Level Level = DiagnosticIDs::Warning); + + /// Report any errors to do with reading the configuration using this method. + DiagnosticBuilder + configurationDiag(StringRef Message, + DiagnosticIDs::Level Level = DiagnosticIDs::Warning); + /// Sets the \c SourceManager of the used \c DiagnosticsEngine. /// /// This is called from the \c ClangTidyCheck base class. diff --git a/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp b/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp --- a/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp +++ b/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp @@ -177,6 +177,21 @@ return DiagEngine->Report(Loc, ID); } +DiagnosticBuilder ClangTidyContext::diag( + StringRef CheckName, StringRef Description, + DiagnosticIDs::Level Level /* = DiagnosticIDs::Warning*/) { + unsigned ID = DiagEngine->getDiagnosticIDs()->getCustomDiagID( + Level, (Description + " [" + CheckName + "]").str()); + CheckNamesByDiagnosticID.try_emplace(ID, CheckName); + return DiagEngine->Report(ID); +} + +DiagnosticBuilder ClangTidyContext::configurationDiag( + StringRef Message, + DiagnosticIDs::Level Level /* = DiagnosticIDs::Warning*/) { + return diag("clang-tidy-config", Message, Level); +} + void ClangTidyContext::setSourceManager(SourceManager *SourceMgr) { DiagEngine->setSourceManager(SourceMgr); } @@ -256,8 +271,10 @@ void ClangTidyDiagnosticConsumer::finalizeLastError() { if (!Errors.empty()) { ClangTidyError &Error = Errors.back(); - if (!Context.isCheckEnabled(Error.DiagnosticName) && - Error.DiagLevel != ClangTidyError::Error) { + if (Error.DiagnosticName == "clang-tidy-config") { + // Never ignore these. + } else if (!Context.isCheckEnabled(Error.DiagnosticName) && + Error.DiagLevel != ClangTidyError::Error) { ++Context.Stats.ErrorsIgnoredCheckFilter; Errors.pop_back(); } else if (!LastErrorRelatesToUserCode) { diff --git a/clang-tools-extra/clang-tidy/bugprone/DynamicStaticInitializersCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/DynamicStaticInitializersCheck.cpp --- a/clang-tools-extra/clang-tidy/bugprone/DynamicStaticInitializersCheck.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/DynamicStaticInitializersCheck.cpp @@ -34,8 +34,8 @@ if (!utils::parseFileExtensions(RawStringHeaderFileExtensions, HeaderFileExtensions, utils::defaultFileExtensionDelimiters())) { - llvm::errs() << "Invalid header file extension: " - << RawStringHeaderFileExtensions << "\n"; + this->configurationDiag("Invalid header file extension: '%0'") + << RawStringHeaderFileExtensions; } } diff --git a/clang-tools-extra/clang-tidy/bugprone/SuspiciousIncludeCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/SuspiciousIncludeCheck.cpp --- a/clang-tools-extra/clang-tidy/bugprone/SuspiciousIncludeCheck.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/SuspiciousIncludeCheck.cpp @@ -46,15 +46,15 @@ if (!utils::parseFileExtensions(RawStringImplementationFileExtensions, ImplementationFileExtensions, utils::defaultFileExtensionDelimiters())) { - llvm::errs() << "Invalid implementation file extension: " - << RawStringImplementationFileExtensions << "\n"; + this->configurationDiag("Invalid implementation file extension: '%0'") + << RawStringImplementationFileExtensions; } if (!utils::parseFileExtensions(RawStringHeaderFileExtensions, HeaderFileExtensions, utils::defaultFileExtensionDelimiters())) { - llvm::errs() << "Invalid header file extension: " - << RawStringHeaderFileExtensions << "\n"; + this->configurationDiag("Invalid header file extension: '%0'") + << RawStringHeaderFileExtensions; } } diff --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/PreferMemberInitializerCheck.cpp b/clang-tools-extra/clang-tidy/cppcoreguidelines/PreferMemberInitializerCheck.cpp --- a/clang-tools-extra/clang-tidy/cppcoreguidelines/PreferMemberInitializerCheck.cpp +++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/PreferMemberInitializerCheck.cpp @@ -97,7 +97,7 @@ IsUseDefaultMemberInitEnabled( Context->isCheckEnabled("modernize-use-default-member-init")), UseAssignment(OptionsView("modernize-use-default-member-init", - Context->getOptions().CheckOptions) + Context->getOptions().CheckOptions, Context) .get("UseAssignment", false)) {} void PreferMemberInitializerCheck::storeOptions( diff --git a/clang-tools-extra/clang-tidy/google/GlobalNamesInHeadersCheck.cpp b/clang-tools-extra/clang-tidy/google/GlobalNamesInHeadersCheck.cpp --- a/clang-tools-extra/clang-tidy/google/GlobalNamesInHeadersCheck.cpp +++ b/clang-tools-extra/clang-tidy/google/GlobalNamesInHeadersCheck.cpp @@ -27,8 +27,8 @@ if (!utils::parseFileExtensions(RawStringHeaderFileExtensions, HeaderFileExtensions, utils::defaultFileExtensionDelimiters())) { - llvm::errs() << "Invalid header file extension: " - << RawStringHeaderFileExtensions << "\n"; + this->configurationDiag("Invalid header file extension: '%0'") + << RawStringHeaderFileExtensions; } } diff --git a/clang-tools-extra/clang-tidy/google/UnnamedNamespaceInHeaderCheck.cpp b/clang-tools-extra/clang-tidy/google/UnnamedNamespaceInHeaderCheck.cpp --- a/clang-tools-extra/clang-tidy/google/UnnamedNamespaceInHeaderCheck.cpp +++ b/clang-tools-extra/clang-tidy/google/UnnamedNamespaceInHeaderCheck.cpp @@ -26,8 +26,8 @@ if (!utils::parseFileExtensions(RawStringHeaderFileExtensions, HeaderFileExtensions, utils::defaultFileExtensionDelimiters())) { - llvm::errs() << "Invalid header file extension: " - << RawStringHeaderFileExtensions << "\n"; + this->configurationDiag("Invalid header file extension: '%0'") + << RawStringHeaderFileExtensions; } } diff --git a/clang-tools-extra/clang-tidy/misc/DefinitionsInHeadersCheck.cpp b/clang-tools-extra/clang-tidy/misc/DefinitionsInHeadersCheck.cpp --- a/clang-tools-extra/clang-tidy/misc/DefinitionsInHeadersCheck.cpp +++ b/clang-tools-extra/clang-tidy/misc/DefinitionsInHeadersCheck.cpp @@ -36,10 +36,8 @@ if (!utils::parseFileExtensions(RawStringHeaderFileExtensions, HeaderFileExtensions, utils::defaultFileExtensionDelimiters())) { - // FIXME: Find a more suitable way to handle invalid configuration - // options. - llvm::errs() << "Invalid header file extension: " - << RawStringHeaderFileExtensions << "\n"; + this->configurationDiag("Invalid header file extension: '%0'") + << RawStringHeaderFileExtensions; } } diff --git a/clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp b/clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp --- a/clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp +++ b/clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp @@ -19,7 +19,6 @@ #include "llvm/ADT/StringRef.h" #include "llvm/ADT/StringSwitch.h" #include "llvm/Support/Casting.h" -#include "llvm/Support/WithColor.h" #include "llvm/Support/raw_ostream.h" #include #include @@ -501,10 +500,10 @@ ReverseHeader(Options.get("MakeReverseRangeHeader", "")) { if (ReverseFunction.empty() && !ReverseHeader.empty()) { - llvm::WithColor::warning() - << "modernize-loop-convert: 'MakeReverseRangeHeader' is set but " - "'MakeReverseRangeFunction' is not, disabling reverse loop " - "transformation\n"; + configurationDiag( + "modernize-loop-convert: 'MakeReverseRangeHeader' is set but " + "'MakeReverseRangeFunction' is not, disabling reverse loop " + "transformation"); UseReverseRanges = false; } else if (ReverseFunction.empty()) { UseReverseRanges = UseCxx20IfAvailable && getLangOpts().CPlusPlus20; diff --git a/clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp b/clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp --- a/clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp +++ b/clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp @@ -773,7 +773,8 @@ ClangTidyOptions Options = Context->getOptionsForFile(FileName); if (Options.Checks && GlobList(*Options.Checks).contains(CheckName)) { auto It = NamingStylesCache.try_emplace( - Parent, getFileStyleFromOptions({CheckName, Options.CheckOptions})); + Parent, + getFileStyleFromOptions({CheckName, Options.CheckOptions, Context})); assert(It.second); return It.first->getValue(); } diff --git a/clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-case-violation.cpp b/clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-case-violation.cpp --- a/clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-case-violation.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-case-violation.cpp @@ -5,11 +5,11 @@ // RUN: {key: readability-identifier-naming.ClassCase, value: UUPER_CASE}, \ // RUN: {key: readability-identifier-naming.StructCase, value: CAMEL}, \ // RUN: {key: readability-identifier-naming.EnumCase, value: AnY_cASe}, \ -// RUN: ]}" 2>&1 | FileCheck %s --implicit-check-not warning +// RUN: ]}" 2>&1 | FileCheck %s --implicit-check-not="{{warning|error}}:" -// CHECK-DAG: warning: invalid configuration value 'camelback' for option 'readability-identifier-naming.FunctionCase'; did you mean 'camelBack'?{{$}} -// CHECK-DAG: warning: invalid configuration value 'UUPER_CASE' for option 'readability-identifier-naming.ClassCase'; did you mean 'UPPER_CASE'?{{$}} +// CHECK-DAG: warning: invalid configuration value 'camelback' for option 'readability-identifier-naming.FunctionCase'; did you mean 'camelBack'? [clang-tidy-config] +// CHECK-DAG: warning: invalid configuration value 'UUPER_CASE' for option 'readability-identifier-naming.ClassCase'; did you mean 'UPPER_CASE'? [clang-tidy-config] // Don't try to suggest an alternative for 'CAMEL' -// CHECK-DAG: warning: invalid configuration value 'CAMEL' for option 'readability-identifier-naming.StructCase'{{$}} +// CHECK-DAG: warning: invalid configuration value 'CAMEL' for option 'readability-identifier-naming.StructCase' [clang-tidy-config] // This fails on the EditDistance, but as it matches ignoring case suggest the correct value -// CHECK-DAG: warning: invalid configuration value 'AnY_cASe' for option 'readability-identifier-naming.EnumCase'; did you mean 'aNy_CasE'?{{$}} +// CHECK-DAG: warning: invalid configuration value 'AnY_cASe' for option 'readability-identifier-naming.EnumCase'; did you mean 'aNy_CasE'? [clang-tidy-config] diff --git a/clang-tools-extra/unittests/clang-tidy/ClangTidyDiagnosticConsumerTest.cpp b/clang-tools-extra/unittests/clang-tidy/ClangTidyDiagnosticConsumerTest.cpp --- a/clang-tools-extra/unittests/clang-tidy/ClangTidyDiagnosticConsumerTest.cpp +++ b/clang-tools-extra/unittests/clang-tidy/ClangTidyDiagnosticConsumerTest.cpp @@ -10,7 +10,9 @@ class TestCheck : public ClangTidyCheck { public: TestCheck(StringRef Name, ClangTidyContext *Context) - : ClangTidyCheck(Name, Context) {} + : ClangTidyCheck(Name, Context) { + diag("DiagWithNoLoc"); + } void registerMatchers(ast_matchers::MatchFinder *Finder) override { Finder->addMatcher(ast_matchers::varDecl().bind("var"), this); } @@ -26,9 +28,10 @@ TEST(ClangTidyDiagnosticConsumer, SortsErrors) { std::vector Errors; runCheckOnCode("int a;", &Errors); - EXPECT_EQ(2ul, Errors.size()); - EXPECT_EQ("type specifier", Errors[0].Message.Message); - EXPECT_EQ("variable", Errors[1].Message.Message); + EXPECT_EQ(3ul, Errors.size()); + EXPECT_EQ("DiagWithNoLoc", Errors[0].Message.Message); + EXPECT_EQ("type specifier", Errors[1].Message.Message); + EXPECT_EQ("variable", Errors[2].Message.Message); } } // namespace test diff --git a/clang-tools-extra/unittests/clang-tidy/ClangTidyOptionsTest.cpp b/clang-tools-extra/unittests/clang-tidy/ClangTidyOptionsTest.cpp --- a/clang-tools-extra/unittests/clang-tidy/ClangTidyOptionsTest.cpp +++ b/clang-tools-extra/unittests/clang-tidy/ClangTidyOptionsTest.cpp @@ -166,6 +166,10 @@ ClangTidyOptions Options; ClangTidyContext Context(std::make_unique( ClangTidyGlobalOptions(), Options)); + ClangTidyDiagnosticConsumer DiagConsumer(Context); + DiagnosticsEngine DE(new DiagnosticIDs(), new DiagnosticOptions, + &DiagConsumer, false); + Context.setDiagnosticsEngine(&DE); TestCheck TestCheck(&Context); CHECK_ERROR(TestCheck.getLocal("Opt"), MissingOptionError, "option not found 'test.Opt'"); @@ -191,6 +195,10 @@ ClangTidyContext Context(std::make_unique( ClangTidyGlobalOptions(), Options)); + ClangTidyDiagnosticConsumer DiagConsumer(Context); + DiagnosticsEngine DE(new DiagnosticIDs(), new DiagnosticOptions, + &DiagConsumer, false); + Context.setDiagnosticsEngine(&DE); TestCheck TestCheck(&Context); #define CHECK_ERROR_INT(Name, Expected) \