This revision was automatically updated to reflect the committed changes.
Closed by commit rG27553933a869: [clang-tidy] Add support for diagnostics with
no location (authored by njames93).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D91885/new/
https://reviews.llvm.org/D91885
Files:
clang-tools-extra/clang-tidy/ClangTidyCheck.cpp
clang-tools-extra/clang-tidy/ClangTidyCheck.h
clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
clang-tools-extra/clang-tidy/bugprone/DynamicStaticInitializersCheck.cpp
clang-tools-extra/clang-tidy/bugprone/SuspiciousIncludeCheck.cpp
clang-tools-extra/clang-tidy/cppcoreguidelines/PreferMemberInitializerCheck.cpp
clang-tools-extra/clang-tidy/google/GlobalNamesInHeadersCheck.cpp
clang-tools-extra/clang-tidy/google/UnnamedNamespaceInHeaderCheck.cpp
clang-tools-extra/clang-tidy/misc/DefinitionsInHeadersCheck.cpp
clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp
clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-case-violation.cpp
clang-tools-extra/unittests/clang-tidy/ClangTidyDiagnosticConsumerTest.cpp
clang-tools-extra/unittests/clang-tidy/ClangTidyOptionsTest.cpp
Index: clang-tools-extra/unittests/clang-tidy/ClangTidyOptionsTest.cpp
===================================================================
--- clang-tools-extra/unittests/clang-tidy/ClangTidyOptionsTest.cpp
+++ clang-tools-extra/unittests/clang-tidy/ClangTidyOptionsTest.cpp
@@ -166,6 +166,10 @@
ClangTidyOptions Options;
ClangTidyContext Context(std::make_unique<DefaultOptionsProvider>(
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<DefaultOptionsProvider>(
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) \
Index: clang-tools-extra/unittests/clang-tidy/ClangTidyDiagnosticConsumerTest.cpp
===================================================================
--- clang-tools-extra/unittests/clang-tidy/ClangTidyDiagnosticConsumerTest.cpp
+++ 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<ClangTidyError> Errors;
runCheckOnCode<TestCheck>("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
Index: clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-case-violation.cpp
===================================================================
--- clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-case-violation.cpp
+++ 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]
Index: clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
===================================================================
--- clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
+++ 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();
}
Index: clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp
===================================================================
--- clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp
+++ 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 <cassert>
#include <cstring>
@@ -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;
Index: clang-tools-extra/clang-tidy/misc/DefinitionsInHeadersCheck.cpp
===================================================================
--- clang-tools-extra/clang-tidy/misc/DefinitionsInHeadersCheck.cpp
+++ 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;
}
}
Index: clang-tools-extra/clang-tidy/google/UnnamedNamespaceInHeaderCheck.cpp
===================================================================
--- clang-tools-extra/clang-tidy/google/UnnamedNamespaceInHeaderCheck.cpp
+++ 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;
}
}
Index: clang-tools-extra/clang-tidy/google/GlobalNamesInHeadersCheck.cpp
===================================================================
--- clang-tools-extra/clang-tidy/google/GlobalNamesInHeadersCheck.cpp
+++ 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;
}
}
Index: clang-tools-extra/clang-tidy/cppcoreguidelines/PreferMemberInitializerCheck.cpp
===================================================================
--- clang-tools-extra/clang-tidy/cppcoreguidelines/PreferMemberInitializerCheck.cpp
+++ 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(
Index: clang-tools-extra/clang-tidy/bugprone/SuspiciousIncludeCheck.cpp
===================================================================
--- clang-tools-extra/clang-tidy/bugprone/SuspiciousIncludeCheck.cpp
+++ 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;
}
}
Index: clang-tools-extra/clang-tidy/bugprone/DynamicStaticInitializersCheck.cpp
===================================================================
--- clang-tools-extra/clang-tidy/bugprone/DynamicStaticInitializersCheck.cpp
+++ 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;
}
}
Index: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
===================================================================
--- clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
+++ 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.
Index: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
===================================================================
--- clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
+++ 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) {
Index: clang-tools-extra/clang-tidy/ClangTidyCheck.h
===================================================================
--- clang-tools-extra/clang-tidy/ClangTidyCheck.h
+++ 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<T> ValueOr = get<T>(LocalName))
return *ValueOr;
else
- logIfOptionParsingError(ValueOr.takeError());
+ reportOptionParsingError(ValueOr.takeError());
return Default;
}
@@ -314,7 +324,7 @@
if (llvm::Expected<T> ValueOr = getLocalOrGlobal<T>(LocalName))
return *ValueOr;
else
- logIfOptionParsingError(ValueOr.takeError());
+ reportOptionParsingError(ValueOr.takeError());
return Default;
}
@@ -353,7 +363,7 @@
if (auto ValueOr = get<T>(LocalName, IgnoreCase))
return *ValueOr;
else
- logIfOptionParsingError(ValueOr.takeError());
+ reportOptionParsingError(ValueOr.takeError());
return Default;
}
@@ -395,7 +405,7 @@
if (auto ValueOr = getLocalOrGlobal<T>(LocalName, IgnoreCase))
return *ValueOr;
else
- logIfOptionParsingError(ValueOr.takeError());
+ reportOptionParsingError(ValueOr.takeError());
return Default;
}
@@ -407,7 +417,7 @@
if (auto ValueOr = get<T>(LocalName))
return *ValueOr;
else
- logIfOptionParsingError(ValueOr.takeError());
+ reportOptionParsingError(ValueOr.takeError());
return llvm::None;
}
@@ -420,7 +430,7 @@
if (auto ValueOr = getLocalOrGlobal<T>(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:
Index: clang-tools-extra/clang-tidy/ClangTidyCheck.cpp
===================================================================
--- clang-tools-extra/clang-tidy/ClangTidyCheck.cpp
+++ 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<std::string>
ClangTidyCheck::OptionsView::get(StringRef LocalName) const {
@@ -121,7 +134,7 @@
llvm::Expected<bool> ValueOr = get<bool>(LocalName);
if (ValueOr)
return *ValueOr;
- logIfOptionParsingError(ValueOr.takeError());
+ reportOptionParsingError(ValueOr.takeError());
return Default;
}
@@ -140,7 +153,7 @@
llvm::Expected<bool> ValueOr = getLocalOrGlobal<bool>(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 <>
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
