diff --git a/clang-tools-extra/clang-tidy/ClangTidy.h b/clang-tools-extra/clang-tidy/ClangTidy.h --- a/clang-tools-extra/clang-tidy/ClangTidy.h +++ b/clang-tools-extra/clang-tidy/ClangTidy.h @@ -79,7 +79,7 @@ const tooling::CompilationDatabase &Compilations, ArrayRef InputFiles, llvm::IntrusiveRefCntPtr BaseFS, - bool EnableCheckProfile = false, + bool ApplyAnyFix, bool EnableCheckProfile = false, llvm::StringRef StoreCheckProfile = StringRef()); // FIXME: This interface will need to be significantly extended to be useful. @@ -89,7 +89,7 @@ /// Errors containing fixes are automatically applied and reformatted. If no /// clang-format configuration file is found, the given \P FormatStyle is used. void handleErrors(llvm::ArrayRef Errors, - ClangTidyContext &Context, bool Fix, + ClangTidyContext &Context, bool Fix, bool AnyFix, unsigned &WarningsAsErrorsCount, llvm::IntrusiveRefCntPtr BaseFS); diff --git a/clang-tools-extra/clang-tidy/ClangTidy.cpp b/clang-tools-extra/clang-tidy/ClangTidy.cpp --- a/clang-tools-extra/clang-tidy/ClangTidy.cpp +++ b/clang-tools-extra/clang-tidy/ClangTidy.cpp @@ -99,14 +99,15 @@ class ErrorReporter { public: - ErrorReporter(ClangTidyContext &Context, bool ApplyFixes, + ErrorReporter(ClangTidyContext &Context, bool ApplyFixes, bool ApplyAnyFix, llvm::IntrusiveRefCntPtr BaseFS) : Files(FileSystemOptions(), BaseFS), DiagOpts(new DiagnosticOptions()), DiagPrinter(new TextDiagnosticPrinter(llvm::outs(), &*DiagOpts)), Diags(IntrusiveRefCntPtr(new DiagnosticIDs), &*DiagOpts, DiagPrinter), SourceMgr(Diags, Files), Context(Context), ApplyFixes(ApplyFixes), - TotalFixes(0), AppliedFixes(0), WarningsAsErrors(0) { + ApplyAnyFix(ApplyAnyFix), TotalFixes(0), AppliedFixes(0), + WarningsAsErrors(0) { DiagOpts->ShowColors = Context.getOptions().UseColor.getValueOr( llvm::sys::Process::StandardOutHasColors()); DiagPrinter->BeginSourceFile(LangOpts); @@ -133,7 +134,8 @@ auto Diag = Diags.Report(Loc, Diags.getCustomDiagID(Level, "%0 [%1]")) << Message.Message << Name; // FIXME: explore options to support interactive fix selection. - const llvm::StringMap *ChosenFix = selectFirstFix(Error); + const llvm::StringMap *ChosenFix = + getFixIt(Error, ApplyAnyFix); if (ApplyFixes && ChosenFix) { for (const auto &FileAndReplacements : *ChosenFix) { for (const auto &Repl : FileAndReplacements.second) { @@ -288,6 +290,7 @@ llvm::StringMap FileReplacements; ClangTidyContext &Context; bool ApplyFixes; + bool ApplyAnyFix; unsigned TotalFixes; unsigned AppliedFixes; unsigned WarningsAsErrors; @@ -500,7 +503,8 @@ const CompilationDatabase &Compilations, ArrayRef InputFiles, llvm::IntrusiveRefCntPtr BaseFS, - bool EnableCheckProfile, llvm::StringRef StoreCheckProfile) { + bool ApplyAnyFix, bool EnableCheckProfile, + llvm::StringRef StoreCheckProfile) { ClangTool Tool(Compilations, InputFiles, std::make_shared(), BaseFS); @@ -527,7 +531,7 @@ Context.setEnableProfiling(EnableCheckProfile); Context.setProfileStoragePrefix(StoreCheckProfile); - ClangTidyDiagnosticConsumer DiagConsumer(Context); + ClangTidyDiagnosticConsumer DiagConsumer(Context, nullptr, true, ApplyAnyFix); DiagnosticsEngine DE(new DiagnosticIDs(), new DiagnosticOptions(), &DiagConsumer, /*ShouldOwnClient=*/false); Context.setDiagnosticsEngine(&DE); @@ -574,10 +578,10 @@ } void handleErrors(llvm::ArrayRef Errors, - ClangTidyContext &Context, bool Fix, + ClangTidyContext &Context, bool Fix, bool AnyFix, unsigned &WarningsAsErrorsCount, llvm::IntrusiveRefCntPtr BaseFS) { - ErrorReporter Reporter(Context, Fix, BaseFS); + ErrorReporter Reporter(Context, Fix, AnyFix, BaseFS); llvm::vfs::FileSystem &FileSystem = Reporter.getSourceManager().getFileManager().getVirtualFileSystem(); auto InitialWorkingDir = FileSystem.getCurrentWorkingDirectory(); 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 @@ -218,6 +218,13 @@ const Diagnostic &Info, ClangTidyContext &Context, bool AllowIO = true); +/// Gets the FixIt attached to \p Message. +/// If \p AnyFix is true and there is no FixIt attached to the Message, +/// returns the first FixIt attached to any notes in the message. +/// If no FixIt is found, returns nullptr. +const llvm::StringMap * +getFixIt(const tooling::Diagnostic &Diagnostic, bool AnyFix); + /// A diagnostic consumer that turns each \c Diagnostic into a /// \c SourceManager-independent \c ClangTidyError. // @@ -227,7 +234,8 @@ public: ClangTidyDiagnosticConsumer(ClangTidyContext &Ctx, DiagnosticsEngine *ExternalDiagEngine = nullptr, - bool RemoveIncompatibleErrors = true); + bool RemoveIncompatibleErrors = true, + bool FindAnyFixIt = false); // FIXME: The concept of converting between FixItHints and Replacements is // more generic and should be pulled out into a more useful Diagnostics @@ -257,6 +265,7 @@ ClangTidyContext &Context; DiagnosticsEngine *ExternalDiagEngine; bool RemoveIncompatibleErrors; + bool FindAnyFixIt; std::vector Errors; std::unique_ptr HeaderFilter; bool LastErrorRelatesToUserCode; 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 @@ -246,11 +246,11 @@ ClangTidyDiagnosticConsumer::ClangTidyDiagnosticConsumer( ClangTidyContext &Ctx, DiagnosticsEngine *ExternalDiagEngine, - bool RemoveIncompatibleErrors) + bool RemoveIncompatibleErrors, bool FindAnyFixit) : Context(Ctx), ExternalDiagEngine(ExternalDiagEngine), RemoveIncompatibleErrors(RemoveIncompatibleErrors), - LastErrorRelatesToUserCode(false), LastErrorPassesLineFilter(false), - LastErrorWasIgnored(false) {} + FindAnyFixIt(FindAnyFixit), LastErrorRelatesToUserCode(false), + LastErrorPassesLineFilter(false), LastErrorWasIgnored(false) {} void ClangTidyDiagnosticConsumer::finalizeLastError() { if (!Errors.empty()) { @@ -372,6 +372,19 @@ Context, AllowIO); } +const llvm::StringMap * +getFixIt(const tooling::Diagnostic &Diagnostic, bool AnyFix) { + if (!Diagnostic.Message.Fix.empty()) + return &Diagnostic.Message.Fix; + if (AnyFix) { + for (const auto &Note : Diagnostic.Notes) { + if (!Note.Fix.empty()) + return &Note.Fix; + } + } + return nullptr; +} + } // namespace tidy } // namespace clang @@ -648,7 +661,7 @@ std::pair *>> ErrorFixes; for (auto &Error : Errors) { - if (const auto *Fix = tooling::selectFirstFix(Error)) + if (const auto *Fix = getFixIt(Error, FindAnyFixIt)) ErrorFixes.emplace_back( &Error, const_cast *>(Fix)); } diff --git a/clang-tools-extra/clang-tidy/misc/UnusedUsingDeclsCheck.cpp b/clang-tools-extra/clang-tidy/misc/UnusedUsingDeclsCheck.cpp --- a/clang-tools-extra/clang-tidy/misc/UnusedUsingDeclsCheck.cpp +++ b/clang-tools-extra/clang-tidy/misc/UnusedUsingDeclsCheck.cpp @@ -181,10 +181,7 @@ for (const auto &Context : Contexts) { if (!Context.IsUsed) { diag(Context.FoundUsingDecl->getLocation(), "using decl %0 is unused") - << Context.FoundUsingDecl; - // Emit a fix and a fix description of the check; - diag(Context.FoundUsingDecl->getLocation(), - /*Description=*/"remove the using", DiagnosticIDs::Note) + << Context.FoundUsingDecl << FixItHint::CreateRemoval(Context.UsingDeclRange); } } diff --git a/clang-tools-extra/clang-tidy/readability/InconsistentDeclarationParameterNameCheck.cpp b/clang-tools-extra/clang-tidy/readability/InconsistentDeclarationParameterNameCheck.cpp --- a/clang-tools-extra/clang-tidy/readability/InconsistentDeclarationParameterNameCheck.cpp +++ b/clang-tools-extra/clang-tidy/readability/InconsistentDeclarationParameterNameCheck.cpp @@ -221,21 +221,12 @@ return ParamInfo.SourceName; }; - auto ParamDiag = - Check->diag(Location, - "differing parameters are named here: (%0), in %1: (%2)", - DiagnosticIDs::Level::Note) + Check->diag(Location, + "differing parameters are named here: (%0), in %1: (%2)", + DiagnosticIDs::Level::Note) << joinParameterNames(DifferingParams, ChooseOtherName) << OtherDeclarationDescription << joinParameterNames(DifferingParams, ChooseSourceName); - - for (const DifferingParamInfo &ParamInfo : DifferingParams) { - if (ParamInfo.GenerateFixItHint) { - ParamDiag << FixItHint::CreateReplacement( - CharSourceRange::getTokenRange(ParamInfo.OtherNameRange), - ParamInfo.SourceName); - } - } } void formatDiagnosticsForDeclarations( @@ -243,11 +234,24 @@ const FunctionDecl *ParameterSourceDeclaration, const FunctionDecl *OriginalDeclaration, const InconsistentDeclarationsContainer &InconsistentDeclarations) { - Check->diag( - OriginalDeclaration->getLocation(), - "function %q0 has %1 other declaration%s1 with different parameter names") - << OriginalDeclaration - << static_cast(InconsistentDeclarations.size()); + { + auto MainDiag = Check->diag(OriginalDeclaration->getLocation(), + "function %q0 has %1 other declaration%s1 with " + "different parameter names") + << OriginalDeclaration + << static_cast(InconsistentDeclarations.size()); + for (const InconsistentDeclarationInfo &InconsistentDeclaration : + InconsistentDeclarations) { + for (const DifferingParamInfo &ParamInfo : + InconsistentDeclaration.DifferingParams) { + if (ParamInfo.GenerateFixItHint) { + MainDiag << FixItHint::CreateReplacement( + CharSourceRange::getTokenRange(ParamInfo.OtherNameRange), + ParamInfo.SourceName); + } + } + } + } int Count = 1; for (const InconsistentDeclarationInfo &InconsistentDeclaration : InconsistentDeclarations) { @@ -272,11 +276,21 @@ StringRef FunctionDescription, StringRef ParameterSourceDescription) { for (const InconsistentDeclarationInfo &InconsistentDeclaration : InconsistentDeclarations) { - Check->diag(InconsistentDeclaration.DeclarationLocation, - "%0 %q1 has a %2 with different parameter names") - << FunctionDescription << OriginalDeclaration - << ParameterSourceDescription; - + { + auto MainDiag = + Check->diag(InconsistentDeclaration.DeclarationLocation, + "%0 %q1 has a %2 with different parameter names") + << FunctionDescription << OriginalDeclaration + << ParameterSourceDescription; + for (const DifferingParamInfo &ParamInfo : + InconsistentDeclaration.DifferingParams) { + if (ParamInfo.GenerateFixItHint) { + MainDiag << FixItHint::CreateReplacement( + CharSourceRange::getTokenRange(ParamInfo.OtherNameRange), + ParamInfo.SourceName); + } + } + } Check->diag(ParameterSourceDeclaration->getLocation(), "the %0 seen here", DiagnosticIDs::Level::Note) << ParameterSourceDescription; diff --git a/clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp b/clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp --- a/clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp +++ b/clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp @@ -19,6 +19,7 @@ #include "../ClangTidyForceLinker.h" #include "../GlobList.h" #include "clang/Tooling/CommonOptionsParser.h" +#include "llvm/ADT/STLExtras.h" #include "llvm/Support/InitLLVM.h" #include "llvm/Support/Process.h" #include "llvm/Support/Signals.h" @@ -127,6 +128,14 @@ )"), cl::init(false), cl::cat(ClangTidyCategory)); +static cl::opt FixNotes("fix-notes", cl::desc(R"( +If a warning has no fix, but has notes attached +which contain fixes, apply the first fix found +in any notes. +Requires -fix to be specified. +)"), + cl::init(false), cl::cat(ClangTidyCategory)); + static cl::opt FormatStyle("format-style", cl::desc(R"( Style for formatting code around applied fixes: - 'none' (default) turns off formatting @@ -450,18 +459,18 @@ AllowEnablingAnalyzerAlphaCheckers); std::vector Errors = runClangTidy(Context, OptionsParser->getCompilations(), PathList, BaseFS, - EnableCheckProfile, ProfilePrefix); - bool FoundErrors = llvm::find_if(Errors, [](const ClangTidyError &E) { - return E.DiagLevel == ClangTidyError::Error; - }) != Errors.end(); + FixNotes, EnableCheckProfile, ProfilePrefix); + bool FoundErrors = llvm::any_of(Errors, [](const ClangTidyError &E) { + return E.DiagLevel == ClangTidyError::Error; + }); const bool DisableFixes = Fix && FoundErrors && !FixErrors; unsigned WErrorCount = 0; // -fix-errors implies -fix. - handleErrors(Errors, Context, (FixErrors || Fix) && !DisableFixes, WErrorCount, - BaseFS); + handleErrors(Errors, Context, (FixErrors || Fix) && !DisableFixes, FixNotes, + WErrorCount, BaseFS); if (!ExportFixes.empty() && !Errors.empty()) { std::error_code EC; diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -67,7 +67,8 @@ Improvements to clang-tidy -------------------------- -The improvements are... + - Added command line option `-fix-notes` to apply fixes found in notes + attached to warnings. Improvements to include-fixer ----------------------------- diff --git a/clang-tools-extra/docs/clang-tidy/index.rst b/clang-tools-extra/docs/clang-tidy/index.rst --- a/clang-tools-extra/docs/clang-tidy/index.rst +++ b/clang-tools-extra/docs/clang-tidy/index.rst @@ -170,6 +170,11 @@ errors were found. If compiler errors have attached fix-its, clang-tidy will apply them as well. + --fix-notes - + If a warning has no fix, but has notes attached + which contain fixes, apply the first fix found + in any notes. + Requires -fix to be specified. --format-style= - Style for formatting code around applied fixes: - 'none' (default) turns off formatting diff --git a/clang-tools-extra/test/clang-tidy/checkers/misc-definitions-in-headers.hpp b/clang-tools-extra/test/clang-tidy/checkers/misc-definitions-in-headers.hpp --- a/clang-tools-extra/test/clang-tidy/checkers/misc-definitions-in-headers.hpp +++ b/clang-tools-extra/test/clang-tidy/checkers/misc-definitions-in-headers.hpp @@ -1,4 +1,4 @@ -// RUN: %check_clang_tidy %s misc-definitions-in-headers %t +// RUN: %check_clang_tidy %s misc-definitions-in-headers %t -- -fix-notes int f() { // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: function 'f' defined in a header file; function definitions in header files can lead to ODR violations [misc-definitions-in-headers] diff --git a/clang-tools-extra/test/clang-tidy/checkers/misc-unused-using-decls.cpp b/clang-tools-extra/test/clang-tidy/checkers/misc-unused-using-decls.cpp --- a/clang-tools-extra/test/clang-tidy/checkers/misc-unused-using-decls.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/misc-unused-using-decls.cpp @@ -81,7 +81,6 @@ // eol-comments aren't removed (yet) using n::A; // A // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: using decl 'A' is unused -// CHECK-MESSAGES: :[[@LINE-2]]:10: note: remove the using // CHECK-FIXES: {{^}}// A using n::B; using n::C; diff --git a/clang-tools-extra/test/clang-tidy/infrastructure/alternative-fixes.cpp b/clang-tools-extra/test/clang-tidy/infrastructure/alternative-fixes.cpp --- a/clang-tools-extra/test/clang-tidy/infrastructure/alternative-fixes.cpp +++ b/clang-tools-extra/test/clang-tidy/infrastructure/alternative-fixes.cpp @@ -1,4 +1,4 @@ -// RUN: %check_clang_tidy %s "llvm-namespace-comment,clang-diagnostic-*" %t +// RUN: %check_clang_tidy %s "llvm-namespace-comment,clang-diagnostic-*" %t -- -fix-notes void foo(int a) { if (a = 1) { // CHECK-NOTES: [[@LINE-1]]:9: warning: using the result of an assignment as a condition without parentheses [clang-diagnostic-parentheses]