Index: clang-tools-extra/clang-tidy/ClangTidy.h =================================================================== --- clang-tools-extra/clang-tidy/ClangTidy.h +++ clang-tools-extra/clang-tidy/ClangTidy.h @@ -88,7 +88,8 @@ const tooling::CompilationDatabase &Compilations, ArrayRef InputFiles, llvm::IntrusiveRefCntPtr BaseFS, - bool ApplyAnyFix, bool EnableCheckProfile = false, + bool ApplyAnyFix, FixType Type, StringRef NoLintPrefix, + bool EnableCheckProfile = false, llvm::StringRef StoreCheckProfile = StringRef()); /// Controls what kind of fixes clang-tidy is allowed to apply. @@ -109,7 +110,7 @@ /// reformatted. If no clang-format configuration file is found, the given \P /// FormatStyle is used. void handleErrors(llvm::ArrayRef Errors, - ClangTidyContext &Context, FixBehaviour Fix, + ClangTidyContext &Context, FixBehaviour Fix, FixType Type, unsigned &WarningsAsErrorsCount, llvm::IntrusiveRefCntPtr BaseFS); @@ -117,7 +118,7 @@ /// output stream. void exportReplacements(StringRef MainFilePath, const std::vector &Errors, - raw_ostream &OS); + raw_ostream &OS, FixType Type); } // end namespace tidy } // end namespace clang Index: clang-tools-extra/clang-tidy/ClangTidy.cpp =================================================================== --- clang-tools-extra/clang-tidy/ClangTidy.cpp +++ clang-tools-extra/clang-tidy/ClangTidy.cpp @@ -41,6 +41,7 @@ #include "clang/Tooling/Tooling.h" #include "llvm/Support/Process.h" #include +#include #include #if CLANG_TIDY_ENABLE_STATIC_ANALYZER @@ -95,6 +96,7 @@ class ErrorReporter { public: ErrorReporter(ClangTidyContext &Context, FixBehaviour ApplyFixes, + FixType Type, llvm::IntrusiveRefCntPtr BaseFS) : Files(FileSystemOptions(), std::move(BaseFS)), DiagOpts(new DiagnosticOptions()), @@ -102,7 +104,7 @@ Diags(IntrusiveRefCntPtr(new DiagnosticIDs), &*DiagOpts, DiagPrinter), SourceMgr(Diags, Files), Context(Context), ApplyFixes(ApplyFixes), - TotalFixes(0), AppliedFixes(0), WarningsAsErrors(0) { + Type(Type), TotalFixes(0), AppliedFixes(0), WarningsAsErrors(0) { DiagOpts->ShowColors = Context.getOptions().UseColor.value_or( llvm::sys::Process::StandardOutHasColors()); DiagPrinter->BeginSourceFile(LangOpts); @@ -118,7 +120,7 @@ SourceLocation Loc = getLocation(Message.FilePath, Message.FileOffset); // Contains a pair for each attempted fix: location and whether the fix was // applied successfully. - SmallVector, 4> FixLocations; + SmallVector, 4> FixLocations; { auto Level = static_cast(Error.DiagLevel); std::string Name = Error.DiagnosticName; @@ -134,10 +136,11 @@ for (const FileByteRange &FBR : Error.Message.Ranges) Diag << getRange(FBR); // FIXME: explore options to support interactive fix selection. - const llvm::StringMap *ChosenFix; + std::pair *, FixType> ChosenFix; if (ApplyFixes != FB_NoFix && - (ChosenFix = getFixIt(Error, ApplyFixes == FB_FixNotes))) { - for (const auto &FileAndReplacements : *ChosenFix) { + std::get<0>(ChosenFix = + getFixIt(Error, Type, ApplyFixes == FB_FixNotes))) { + for (const auto &FileAndReplacements : *std::get<0>(ChosenFix)) { for (const auto &Repl : FileAndReplacements.second) { ++TotalFixes; bool CanBeApplied = false; @@ -174,14 +177,18 @@ ++AppliedFixes; } FixLoc = getLocation(FixAbsoluteFilePath, Repl.getOffset()); - FixLocations.push_back(std::make_pair(FixLoc, CanBeApplied)); + FixLocations.emplace_back(FixLoc, std::get<1>(ChosenFix), + CanBeApplied); } } } reportFix(Diag, Error.Message.Fix); } for (auto Fix : FixLocations) { - Diags.Report(Fix.first, Fix.second ? diag::note_fixit_applied + Diags.Report(std::get<0>(Fix), std::get<2>(Fix) + ? (std::get<1>(Fix) == FT_FixIt + ? diag::note_fixit_applied + : diag::note_fixit_added_nolint) : diag::note_fixit_failed); } for (const auto &Note : Error.Notes) @@ -299,6 +306,7 @@ llvm::StringMap FileReplacements; ClangTidyContext &Context; FixBehaviour ApplyFixes; + FixType Type; unsigned TotalFixes; unsigned AppliedFixes; unsigned WarningsAsErrors; @@ -507,8 +515,8 @@ const CompilationDatabase &Compilations, ArrayRef InputFiles, llvm::IntrusiveRefCntPtr BaseFS, - bool ApplyAnyFix, bool EnableCheckProfile, - llvm::StringRef StoreCheckProfile) { + bool ApplyAnyFix, FixType Type, StringRef NoLintPrefix, + bool EnableCheckProfile, llvm::StringRef StoreCheckProfile) { ClangTool Tool(Compilations, InputFiles, std::make_shared(), BaseFS); @@ -535,7 +543,8 @@ Context.setEnableProfiling(EnableCheckProfile); Context.setProfileStoragePrefix(StoreCheckProfile); - ClangTidyDiagnosticConsumer DiagConsumer(Context, nullptr, true, ApplyAnyFix); + ClangTidyDiagnosticConsumer DiagConsumer(Context, nullptr, true, ApplyAnyFix, + Type, NoLintPrefix); DiagnosticsEngine DE(new DiagnosticIDs(), new DiagnosticOptions(), &DiagConsumer, /*ShouldOwnClient=*/false); Context.setDiagnosticsEngine(&DE); @@ -582,10 +591,10 @@ } void handleErrors(llvm::ArrayRef Errors, - ClangTidyContext &Context, FixBehaviour Fix, + ClangTidyContext &Context, FixBehaviour Fix, FixType Type, unsigned &WarningsAsErrorsCount, llvm::IntrusiveRefCntPtr BaseFS) { - ErrorReporter Reporter(Context, Fix, std::move(BaseFS)); + ErrorReporter Reporter(Context, Fix, Type, std::move(BaseFS)); llvm::vfs::FileSystem &FileSystem = Reporter.getSourceManager().getFileManager().getVirtualFileSystem(); auto InitialWorkingDir = FileSystem.getCurrentWorkingDirectory(); @@ -610,11 +619,14 @@ void exportReplacements(const llvm::StringRef MainFilePath, const std::vector &Errors, - raw_ostream &OS) { + raw_ostream &OS, FixType Type) { TranslationUnitDiagnostics TUD; TUD.MainSourceFile = std::string(MainFilePath); for (const auto &Error : Errors) { tooling::Diagnostic Diag = Error; + if (Type == FT_NoLint || + (Type == FT_FixItOrNoLint && Error.Message.Fix.empty())) + Diag.Message.Fix = Error.NoLintReplacements; TUD.Diagnostics.insert(TUD.Diagnostics.end(), Diag); } Index: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h =================================================================== --- clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h +++ clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h @@ -40,6 +40,7 @@ bool IsWarningAsError; std::vector EnabledDiagnosticAliases; + llvm::StringMap NoLintReplacements; }; /// Contains displayed and ignored diagnostic counters for a ClangTidy run. @@ -240,12 +241,22 @@ llvm::StringSet<> *OptionsCollector = nullptr; }; -/// Gets the Fix attached to \p Diagnostic. +/// Controls how clang-tidy applies fixes. +enum FixType { + /// Only apply fix-its. + FT_FixIt, + /// Only add NOLINT lines. + FT_NoLint, + /// Add NOLINT if fix-it is not available. + FT_FixItOrNoLint +}; + +/// Gets the Fix attached to \p ClangTidyError. /// If there isn't a Fix attached to the diagnostic and \p AnyFix is true, Check /// to see if exactly one note has a Fix and return it. Otherwise return /// nullptr. -const llvm::StringMap * -getFixIt(const tooling::Diagnostic &Diagnostic, bool AnyFix); +std::pair *, FixType> +getFixIt(const ClangTidyError &Error, FixType Type, bool AnyFix); /// A diagnostic consumer that turns each \c Diagnostic into a /// \c SourceManager-independent \c ClangTidyError. @@ -259,6 +270,8 @@ DiagnosticsEngine *ExternalDiagEngine = nullptr, bool RemoveIncompatibleErrors = true, bool GetFixesFromNotes = false, + FixType Type = FT_FixIt, + StringRef NoLintPrefix = "", bool EnableNolintBlocks = true); // FIXME: The concept of converting between FixItHints and Replacements is @@ -290,6 +303,8 @@ DiagnosticsEngine *ExternalDiagEngine; bool RemoveIncompatibleErrors; bool GetFixesFromNotes; + FixType Type; + StringRef NoLintPrefix; bool EnableNolintBlocks; std::vector Errors; std::unique_ptr HeaderFilter; Index: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp =================================================================== --- clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp +++ clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp @@ -22,6 +22,7 @@ #include "clang/AST/ASTContext.h" #include "clang/AST/ASTDiagnostic.h" #include "clang/AST/Attr.h" +#include "clang/Basic/CharInfo.h" #include "clang/Basic/Diagnostic.h" #include "clang/Basic/DiagnosticOptions.h" #include "clang/Basic/FileManager.h" @@ -44,12 +45,29 @@ using namespace tidy; namespace { +void computeNoLintReplacements(FullSourceLoc Loc, ClangTidyError &Error, + StringRef NoLintPrefix) { + if (!Loc.isValid()) + return; + + StringRef Buf = Loc.getBufferData(); + const char *Begin = Lexer::findBeginningOfLine(Buf, Error.Message.FileOffset); + tooling::Replacement Repl( + Error.Message.FilePath, Begin - Buf.data(), 0, + (Lexer::getIndentationForLine(Loc, Loc.getManager()) + "/* " + + NoLintPrefix + "NOLINTNEXTLINE(" + Error.DiagnosticName + ") */\n") + .str()); + auto Err = Error.NoLintReplacements[Error.Message.FilePath].add(Repl); + assert(!Err); +} + class ClangTidyDiagnosticRenderer : public DiagnosticRenderer { public: ClangTidyDiagnosticRenderer(const LangOptions &LangOpts, DiagnosticOptions *DiagOpts, - ClangTidyError &Error) - : DiagnosticRenderer(LangOpts, DiagOpts), Error(Error) {} + ClangTidyError &Error, StringRef NoLintPrefix) + : DiagnosticRenderer(LangOpts, DiagOpts), Error(Error), + NoLintPrefix(NoLintPrefix) {} protected: void emitDiagnosticMessage(FullSourceLoc Loc, PresumedLoc PLoc, @@ -100,6 +118,7 @@ for (const CharSourceRange &SourceRange : ValidRanges) Error.Message.Ranges.emplace_back(Loc.getManager(), ToCharRange(SourceRange)); + computeNoLintReplacements(Loc, Error, NoLintPrefix); } void emitDiagnosticLoc(FullSourceLoc Loc, PresumedLoc PLoc, @@ -149,6 +168,7 @@ private: ClangTidyError &Error; + StringRef NoLintPrefix; }; } // end anonymous namespace @@ -288,13 +308,14 @@ ClangTidyDiagnosticConsumer::ClangTidyDiagnosticConsumer( ClangTidyContext &Ctx, DiagnosticsEngine *ExternalDiagEngine, - bool RemoveIncompatibleErrors, bool GetFixesFromNotes, - bool EnableNolintBlocks) + bool RemoveIncompatibleErrors, bool GetFixesFromNotes, FixType Type, + StringRef NoLintPrefix, bool EnableNolintBlocks) : Context(Ctx), ExternalDiagEngine(ExternalDiagEngine), RemoveIncompatibleErrors(RemoveIncompatibleErrors), - GetFixesFromNotes(GetFixesFromNotes), - EnableNolintBlocks(EnableNolintBlocks), LastErrorRelatesToUserCode(false), - LastErrorPassesLineFilter(false), LastErrorWasIgnored(false) {} + GetFixesFromNotes(GetFixesFromNotes), Type(Type), + NoLintPrefix(NoLintPrefix), EnableNolintBlocks(EnableNolintBlocks), + LastErrorRelatesToUserCode(false), LastErrorPassesLineFilter(false), + LastErrorWasIgnored(false) {} void ClangTidyDiagnosticConsumer::finalizeLastError() { if (!Errors.empty()) { @@ -321,22 +342,37 @@ namespace clang::tidy { -const llvm::StringMap * -getFixIt(const tooling::Diagnostic &Diagnostic, bool GetFixFromNotes) { - if (!Diagnostic.Message.Fix.empty()) - return &Diagnostic.Message.Fix; - if (!GetFixFromNotes) - return nullptr; +std::pair *, FixType> +getFixIt(const ClangTidyError &Error, FixType Type, bool GetFixFromNotes) { const llvm::StringMap *Result = nullptr; - for (const auto &Note : Diagnostic.Notes) { - if (!Note.Fix.empty()) { - if (Result) - // We have 2 different fixes in notes, bail out. + FixType AppliedType = FT_FixIt; + if (Type == FT_FixIt || Type == FT_FixItOrNoLint) { + auto GetFix = + [&Error, + GetFixFromNotes]() -> const llvm::StringMap * { + if (!Error.Message.Fix.empty()) + return &Error.Message.Fix; + if (!GetFixFromNotes) return nullptr; - Result = &Note.Fix; - } + const llvm::StringMap *Result = nullptr; + for (const auto &Note : Error.Notes) { + if (!Note.Fix.empty()) { + if (Result) + // We have 2 different fixes in notes, bail out. + return nullptr; + Result = &Note.Fix; + } + } + return Result; + }; + Result = GetFix(); + AppliedType = FT_FixIt; + } + if ((Type == FT_NoLint || Type == FT_FixItOrNoLint) && !Result) { + Result = &Error.NoLintReplacements; + AppliedType = FT_NoLint; } - return Result; + return std::make_pair(Result, AppliedType); } } // namespace clang::tidy @@ -415,7 +451,7 @@ } else { ClangTidyDiagnosticRenderer Converter( Context.getLangOpts(), &Context.DiagEngine->getDiagnosticOptions(), - Errors.back()); + Errors.back(), NoLintPrefix); SmallString<100> Message; Info.FormatDiagnostic(Message); FullSourceLoc Loc; @@ -639,9 +675,11 @@ std::pair *>> ErrorFixes; for (auto &Error : Errors) { - if (const auto *Fix = getFixIt(Error, GetFixesFromNotes)) + const auto Fix = getFixIt(Error, Type, GetFixesFromNotes); + if (Fix.first) ErrorFixes.emplace_back( - &Error, const_cast *>(Fix)); + &Error, + const_cast *>(Fix.first)); } for (const auto &ErrorAndFix : ErrorFixes) { int Size = 0; 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 @@ -138,6 +138,25 @@ )"), cl::init(false), cl::cat(ClangTidyCategory)); +static cl::opt FixMode("fix-mode", cl::desc(R"( +How to fix warnings: + - 'fixit' (default) applies fix-it if available + - 'nolint' adds a NOLINT comment to suppress the + warning, prefixed with the value of the + --nolint-prefix argument + - 'fixit-or-nolint' applies fix-it if available, + otherwise adds a NOLINT comment like the 'nolint' + option +)"), + cl::init("fixit"), + cl::cat(ClangTidyCategory)); + +static cl::opt NoLintPrefix("nolint-prefix", cl::desc(R"( +Prefix to be added to NOLINT comments. +)"), + cl::init(""), + cl::cat(ClangTidyCategory)); + static cl::opt FormatStyle("format-style", cl::desc(R"( Style for formatting code around applied fixes: - 'none' (default) turns off formatting @@ -473,6 +492,19 @@ return 1; } + FixType Type = FT_FixIt; + if (FixMode.getValue() == "fixit") + Type = FT_FixIt; + else if (FixMode.getValue() == "nolint") + Type = FT_NoLint; + else if (FixMode.getValue() == "fixit-or-nolint") + Type = FT_FixItOrNoLint; + else { + llvm::WithColor::error() + << "invalid value for --fix-type: '" << FixMode.getValue() << "'\n"; + return 1; + } + llvm::IntrusiveRefCntPtr BaseFS( new vfs::OverlayFileSystem(vfs::getRealFileSystem())); @@ -597,9 +629,9 @@ ClangTidyContext Context(std::move(OwningOptionsProvider), AllowEnablingAnalyzerAlphaCheckers); - std::vector Errors = - runClangTidy(Context, OptionsParser->getCompilations(), PathList, BaseFS, - FixNotes, EnableCheckProfile, ProfilePrefix); + std::vector Errors = runClangTidy( + Context, OptionsParser->getCompilations(), PathList, BaseFS, FixNotes, + Type, NoLintPrefix.getValue(), EnableCheckProfile, ProfilePrefix); bool FoundErrors = llvm::any_of(Errors, [](const ClangTidyError &E) { return E.DiagLevel == ClangTidyError::Error; }); @@ -613,7 +645,7 @@ unsigned WErrorCount = 0; - handleErrors(Errors, Context, DisableFixes ? FB_NoFix : Behaviour, + handleErrors(Errors, Context, DisableFixes ? FB_NoFix : Behaviour, Type, WErrorCount, BaseFS); if (!ExportFixes.empty() && !Errors.empty()) { @@ -623,7 +655,7 @@ llvm::errs() << "Error opening output file: " << EC.message() << '\n'; return 1; } - exportReplacements(FilePath.str(), Errors, OS); + exportReplacements(FilePath.str(), Errors, OS, Type); } if (!Quiet) { Index: clang-tools-extra/test/clang-tidy/infrastructure/fix-mode-fixit-or-nolint.cpp =================================================================== --- /dev/null +++ clang-tools-extra/test/clang-tidy/infrastructure/fix-mode-fixit-or-nolint.cpp @@ -0,0 +1,18 @@ +// RUN: grep -Ev "// *[A-Z-]+:" %s > %t.cpp +// RUN: clang-tidy %t.cpp -checks='-*,modernize-use-trailing-return-type,readability-identifier-length' -fix -fix-mode=fixit-or-nolint -export-fixes=%t.yaml -- > %t.msg 2>&1 +// RUN: FileCheck -input-file=%t.cpp %s +// RUN: FileCheck -input-file=%t.msg -check-prefix=CHECK-MESSAGES %s +// RUN: FileCheck -input-file=%t.yaml -check-prefix=CHECK-YAML %s + +int func() { + // CHECK: auto func() -> int { + // CHECK-MESSAGES: note: FIX-IT applied suggested code changes + // CHECK-YAML: ReplacementText: auto + // CHECK-YAML: ReplacementText: ' -> int' + // CHECK: /* NOLINTNEXTLINE(readability-identifier-length) */ + // CHECK-NEXT: int i = 0; + // CHECK-MESSAGES: note: FIX-IT added NOLINT to suppress warning + // CHECK-YAML: ReplacementText: " /* NOLINTNEXTLINE(readability-identifier-length) */\n" + int i = 0; + return i; +} Index: clang-tools-extra/test/clang-tidy/infrastructure/fix-mode-nolint.cpp =================================================================== --- /dev/null +++ clang-tools-extra/test/clang-tidy/infrastructure/fix-mode-nolint.cpp @@ -0,0 +1,18 @@ +// RUN: grep -Ev "// *[A-Z-]+:" %s > %t.cpp +// RUN: clang-tidy %t.cpp -checks='-*,modernize-use-trailing-return-type,readability-identifier-length' -fix -fix-mode=nolint -nolint-prefix='FIXME: ' -export-fixes=%t.yaml -- > %t.msg 2>&1 +// RUN: FileCheck -input-file=%t.cpp %s +// RUN: FileCheck -input-file=%t.msg -check-prefix=CHECK-MESSAGES %s +// RUN: FileCheck -input-file=%t.yaml -check-prefix=CHECK-YAML %s + +// CHECK: /* FIXME: NOLINTNEXTLINE(modernize-use-trailing-return-type) */ +// CHECK-NEXT: int func() { +// CHECK-MESSAGES: note: FIX-IT added NOLINT to suppress warning +// CHECK-YAML: ReplacementText: "/* FIXME: NOLINTNEXTLINE(modernize-use-trailing-return-type) */\n" +int func() { + // CHECK: /* FIXME: NOLINTNEXTLINE(readability-identifier-length) */ + // CHECK-NEXT: int i = 0; + // CHECK-MESSAGES: note: FIX-IT added NOLINT to suppress warning + // CHECK-YAML: ReplacementText: " /* FIXME: NOLINTNEXTLINE(readability-identifier-length) */\n" + int i = 0; + return i; +} Index: clang/include/clang/Basic/DiagnosticFrontendKinds.td =================================================================== --- clang/include/clang/Basic/DiagnosticFrontendKinds.td +++ clang/include/clang/Basic/DiagnosticFrontendKinds.td @@ -180,6 +180,7 @@ "qualifier 'const' is needed for variables in address space '%0'">; def note_fixit_applied : Note<"FIX-IT applied suggested code changes">; +def note_fixit_added_nolint : Note<"FIX-IT added NOLINT to suppress warning">; def note_fixit_in_macro : Note< "FIX-IT unable to apply suggested code changes in a macro">; def note_fixit_failed : Note<