njames93 created this revision.
njames93 added reviewers: alexfh, aaron.ballman, gribozavr2, Eugene.Zelenko,
hokein.
Herald added subscribers: cfe-commits, arphaman, xazax.hun.
Herald added a project: clang.
njames93 requested review of this revision.
Added an option to control whether to apply the fixes found in notes attached
to clang tidy errors or not.
Diagnostics may contain multiple notes each offering different ways to fix the
issue, for that reason the default behaviour should be to not look at fixes
found in notes.
Instead offer up all the available fix-its in the output but don't try to apply
the first one unless `-fix-notes` is supplied.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D84924
Files:
clang-tools-extra/clang-tidy/ClangTidy.cpp
clang-tools-extra/clang-tidy/ClangTidy.h
clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
clang-tools-extra/clang-tidy/misc/UnusedUsingDeclsCheck.cpp
clang-tools-extra/clang-tidy/readability/InconsistentDeclarationParameterNameCheck.cpp
clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/docs/clang-tidy/index.rst
clang-tools-extra/test/clang-tidy/checkers/misc-definitions-in-headers.hpp
clang-tools-extra/test/clang-tidy/checkers/misc-unused-using-decls.cpp
clang-tools-extra/test/clang-tidy/infrastructure/alternative-fixes.cpp
Index: clang-tools-extra/test/clang-tidy/infrastructure/alternative-fixes.cpp
===================================================================
--- clang-tools-extra/test/clang-tidy/infrastructure/alternative-fixes.cpp
+++ 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]
Index: clang-tools-extra/test/clang-tidy/checkers/misc-unused-using-decls.cpp
===================================================================
--- clang-tools-extra/test/clang-tidy/checkers/misc-unused-using-decls.cpp
+++ 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;
Index: clang-tools-extra/test/clang-tidy/checkers/misc-definitions-in-headers.hpp
===================================================================
--- clang-tools-extra/test/clang-tidy/checkers/misc-definitions-in-headers.hpp
+++ 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]
Index: clang-tools-extra/docs/clang-tidy/index.rst
===================================================================
--- clang-tools-extra/docs/clang-tidy/index.rst
+++ 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=<string> -
Style for formatting code around applied fixes:
- 'none' (default) turns off formatting
Index: clang-tools-extra/docs/ReleaseNotes.rst
===================================================================
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ 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
-----------------------------
Index: clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
===================================================================
--- clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
+++ 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<bool> 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<std::string> 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<ClangTidyError> 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;
Index: clang-tools-extra/clang-tidy/readability/InconsistentDeclarationParameterNameCheck.cpp
===================================================================
--- clang-tools-extra/clang-tidy/readability/InconsistentDeclarationParameterNameCheck.cpp
+++ 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<int>(InconsistentDeclarations.size());
+ {
+ auto MainDiag = Check->diag(OriginalDeclaration->getLocation(),
+ "function %q0 has %1 other declaration%s1 with "
+ "different parameter names")
+ << OriginalDeclaration
+ << static_cast<int>(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;
Index: clang-tools-extra/clang-tidy/misc/UnusedUsingDeclsCheck.cpp
===================================================================
--- clang-tools-extra/clang-tidy/misc/UnusedUsingDeclsCheck.cpp
+++ 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);
}
}
Index: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
===================================================================
--- clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
+++ 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<tooling::Replacements> *
+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<ClangTidyError> Errors;
std::unique_ptr<llvm::Regex> HeaderFilter;
bool LastErrorRelatesToUserCode;
Index: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
===================================================================
--- clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
+++ 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<tooling::Replacements> *
+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<ClangTidyError *, llvm::StringMap<tooling::Replacements> *>>
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<llvm::StringMap<tooling::Replacements> *>(Fix));
}
Index: clang-tools-extra/clang-tidy/ClangTidy.h
===================================================================
--- clang-tools-extra/clang-tidy/ClangTidy.h
+++ clang-tools-extra/clang-tidy/ClangTidy.h
@@ -79,7 +79,7 @@
const tooling::CompilationDatabase &Compilations,
ArrayRef<std::string> InputFiles,
llvm::IntrusiveRefCntPtr<llvm::vfs::OverlayFileSystem> 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<ClangTidyError> Errors,
- ClangTidyContext &Context, bool Fix,
+ ClangTidyContext &Context, bool Fix, bool AnyFix,
unsigned &WarningsAsErrorsCount,
llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> BaseFS);
Index: clang-tools-extra/clang-tidy/ClangTidy.cpp
===================================================================
--- clang-tools-extra/clang-tidy/ClangTidy.cpp
+++ 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<llvm::vfs::FileSystem> BaseFS)
: Files(FileSystemOptions(), BaseFS), DiagOpts(new DiagnosticOptions()),
DiagPrinter(new TextDiagnosticPrinter(llvm::outs(), &*DiagOpts)),
Diags(IntrusiveRefCntPtr<DiagnosticIDs>(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<Replacements> *ChosenFix = selectFirstFix(Error);
+ const llvm::StringMap<Replacements> *ChosenFix =
+ getFixIt(Error, ApplyAnyFix);
if (ApplyFixes && ChosenFix) {
for (const auto &FileAndReplacements : *ChosenFix) {
for (const auto &Repl : FileAndReplacements.second) {
@@ -288,6 +290,7 @@
llvm::StringMap<Replacements> FileReplacements;
ClangTidyContext &Context;
bool ApplyFixes;
+ bool ApplyAnyFix;
unsigned TotalFixes;
unsigned AppliedFixes;
unsigned WarningsAsErrors;
@@ -500,7 +503,8 @@
const CompilationDatabase &Compilations,
ArrayRef<std::string> InputFiles,
llvm::IntrusiveRefCntPtr<llvm::vfs::OverlayFileSystem> BaseFS,
- bool EnableCheckProfile, llvm::StringRef StoreCheckProfile) {
+ bool ApplyAnyFix, bool EnableCheckProfile,
+ llvm::StringRef StoreCheckProfile) {
ClangTool Tool(Compilations, InputFiles,
std::make_shared<PCHContainerOperations>(), 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<ClangTidyError> Errors,
- ClangTidyContext &Context, bool Fix,
+ ClangTidyContext &Context, bool Fix, bool AnyFix,
unsigned &WarningsAsErrorsCount,
llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> BaseFS) {
- ErrorReporter Reporter(Context, Fix, BaseFS);
+ ErrorReporter Reporter(Context, Fix, AnyFix, BaseFS);
llvm::vfs::FileSystem &FileSystem =
Reporter.getSourceManager().getFileManager().getVirtualFileSystem();
auto InitialWorkingDir = FileSystem.getCurrentWorkingDirectory();
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
