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); @@ -136,7 +137,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) { @@ -291,6 +293,7 @@ llvm::StringMap FileReplacements; ClangTidyContext &Context; bool ApplyFixes; + bool ApplyAnyFix; unsigned TotalFixes; unsigned AppliedFixes; unsigned WarningsAsErrors; @@ -502,7 +505,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); @@ -529,7 +533,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); @@ -576,10 +580,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 Diagnostic. +/// 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 @@ -247,11 +247,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()) { @@ -359,6 +359,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 @@ -643,7 +656,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 @@ -219,21 +219,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( @@ -241,11 +232,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) { @@ -270,11 +274,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 @@ -127,6 +127,13 @@ )"), 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. +)"), + 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 @@ -481,18 +488,18 @@ AllowEnablingAnalyzerAlphaCheckers); std::vector Errors = runClangTidy(Context, OptionsParser->getCompilations(), PathList, BaseFS, - EnableCheckProfile, ProfilePrefix); + FixNotes, EnableCheckProfile, ProfilePrefix); bool FoundErrors = llvm::find_if(Errors, [](const ClangTidyError &E) { return E.DiagLevel == ClangTidyError::Error; }) != Errors.end(); - const bool DisableFixes = Fix && FoundErrors && !FixErrors; + const bool DisableFixes = (Fix || FixNotes) && FoundErrors && !FixErrors; unsigned WErrorCount = 0; // -fix-errors implies -fix. - handleErrors(Errors, Context, (FixErrors || Fix) && !DisableFixes, WErrorCount, - BaseFS); + handleErrors(Errors, Context, (FixErrors || Fix || FixNotes) && !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,6 +67,9 @@ Improvements to clang-tidy -------------------------- + - Added command line option `-fix-notes` to apply fixes found in notes + attached to warnings. + - Checks that allow configuring names of headers to include now support wrapping the include in angle brackets to create a system include. For example, :doc:`cppcoreguidelines-init-variables 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 @@ -171,6 +171,10 @@ 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. --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]