Author: Nathan James Date: 2022-04-16T09:53:35+01:00 New Revision: b859c39c40a79ff74033f67d807a18130b9afe30
URL: https://github.com/llvm/llvm-project/commit/b859c39c40a79ff74033f67d807a18130b9afe30 DIFF: https://github.com/llvm/llvm-project/commit/b859c39c40a79ff74033f67d807a18130b9afe30.diff LOG: [clang-tidy] Add a Standalone diagnostics mode to clang-tidy Adds a flag to `ClangTidyContext` that is used to indicate to checks that fixes will only be applied one at a time. This is to indicate to checks that each fix emitted should not depend on any other fixes emitted across the translation unit. I've currently implemented the `IncludeInserter`, `LoopConvertCheck` and `PreferMemberInitializerCheck` to use these support these modes. Reasoning behind this is in use cases like `clangd` it's only possible to apply one fix at a time. For include inserter checks, the include is only added once for the first diagnostic that requires it, this will result in subsequent fixes not having the included needed. A similar issue is seen in the `PreferMemberInitializerCheck` where the `:` will only be added for the first member that needs fixing. Fixes emitted in `StandaloneDiagsMode` will likely result in malformed code if they are applied all together, conversely fixes currently emitted may result in malformed code if they are applied one at a time. For this reason invoking `clang-tidy` from the binary will always with `StandaloneDiagsMode` disabled, However using it as a library its possible to select the mode you wish to use, `clangd` always selects `StandaloneDiagsMode`. This is an example of the current behaviour failing ```lang=c++ struct Foo { int A, B; Foo(int D, int E) { A = D; B = E; // Fix Here } }; ``` Incorrectly transformed to: ```lang=c++ struct Foo { int A, B; Foo(int D, int E), B(E) { A = D; // Fix Here } }; ``` In `StandaloneDiagsMode`, it gets transformed to: ```lang=c++ struct Foo { int A, B; Foo(int D, int E) : B(E) { A = D; // Fix Here } }; ``` Reviewed By: sammccall Differential Revision: https://reviews.llvm.org/D97121 Added: Modified: 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/abseil/StringFindStartswithCheck.cpp clang-tools-extra/clang-tidy/bugprone/ImplicitWideningOfMultiplicationResultCheck.cpp clang-tools-extra/clang-tidy/cppcoreguidelines/InitVariablesCheck.cpp clang-tools-extra/clang-tidy/cppcoreguidelines/PreferMemberInitializerCheck.cpp clang-tools-extra/clang-tidy/cppcoreguidelines/ProBoundsConstantArrayIndexCheck.cpp clang-tools-extra/clang-tidy/misc/UniqueptrResetReleaseCheck.cpp clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp clang-tools-extra/clang-tidy/modernize/PassByValueCheck.cpp clang-tools-extra/clang-tidy/modernize/ReplaceAutoPtrCheck.cpp clang-tools-extra/clang-tidy/modernize/ReplaceRandomShuffleCheck.cpp clang-tools-extra/clang-tidy/performance/TypePromotionInMathFnCheck.cpp clang-tools-extra/clang-tidy/performance/UnnecessaryValueParamCheck.cpp clang-tools-extra/clang-tidy/utils/IncludeInserter.cpp clang-tools-extra/clang-tidy/utils/IncludeInserter.h clang-tools-extra/clang-tidy/utils/TransformerClangTidyCheck.cpp clang-tools-extra/clangd/ParsedAST.cpp clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp clang-tools-extra/docs/ReleaseNotes.rst clang-tools-extra/unittests/clang-tidy/IncludeInserterTest.cpp Removed: ################################################################################ diff --git a/clang-tools-extra/clang-tidy/ClangTidyCheck.h b/clang-tools-extra/clang-tidy/ClangTidyCheck.h index 9b41e5836de73..33f84a15b8441 100644 --- a/clang-tools-extra/clang-tidy/ClangTidyCheck.h +++ b/clang-tools-extra/clang-tidy/ClangTidyCheck.h @@ -417,6 +417,11 @@ class ClangTidyCheck : public ast_matchers::MatchFinder::MatchCallback { StringRef getCurrentMainFile() const { return Context->getCurrentFile(); } /// Returns the language options from the context. const LangOptions &getLangOpts() const { return Context->getLangOpts(); } + /// Returns true when the check is run in a use case when only 1 fix will be + /// applied at a time. + bool areDiagsSelfContained() const { + return Context->areDiagsSelfContained(); + } }; /// Read a named option from the ``Context`` and parse it as a bool. diff --git a/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp b/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp index 04721a2d3a020..d455473673b09 100644 --- a/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp +++ b/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp @@ -162,7 +162,8 @@ ClangTidyContext::ClangTidyContext( bool AllowEnablingAnalyzerAlphaCheckers) : DiagEngine(nullptr), OptionsProvider(std::move(OptionsProvider)), Profile(false), - AllowEnablingAnalyzerAlphaCheckers(AllowEnablingAnalyzerAlphaCheckers) { + AllowEnablingAnalyzerAlphaCheckers(AllowEnablingAnalyzerAlphaCheckers), + SelfContainedDiags(false) { // Before the first translation unit we can get errors related to command-line // parsing, use empty string for the file name in this case. setCurrentFile(""); diff --git a/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h b/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h index 1b1866476c38b..d9424234fcf8d 100644 --- a/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h +++ b/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h @@ -187,6 +187,10 @@ class ClangTidyContext { return AllowEnablingAnalyzerAlphaCheckers; } + void setSelfContainedDiags(bool Value) { SelfContainedDiags = Value; } + + bool areDiagsSelfContained() const { return SelfContainedDiags; } + using DiagLevelAndFormatString = std::pair<DiagnosticIDs::Level, std::string>; DiagLevelAndFormatString getDiagLevelAndFormatString(unsigned DiagnosticID, SourceLocation Loc) { @@ -223,6 +227,8 @@ class ClangTidyContext { bool AllowEnablingAnalyzerAlphaCheckers; + bool SelfContainedDiags; + NoLintDirectiveHandler NoLintHandler; }; diff --git a/clang-tools-extra/clang-tidy/abseil/StringFindStartswithCheck.cpp b/clang-tools-extra/clang-tidy/abseil/StringFindStartswithCheck.cpp index e834c8a124590..4ac76d0242ec1 100644 --- a/clang-tools-extra/clang-tidy/abseil/StringFindStartswithCheck.cpp +++ b/clang-tools-extra/clang-tidy/abseil/StringFindStartswithCheck.cpp @@ -27,7 +27,8 @@ StringFindStartswithCheck::StringFindStartswithCheck(StringRef Name, StringLikeClasses(utils::options::parseStringList( Options.get("StringLikeClasses", "::std::basic_string"))), IncludeInserter(Options.getLocalOrGlobal("IncludeStyle", - utils::IncludeSorter::IS_LLVM)), + utils::IncludeSorter::IS_LLVM), + areDiagsSelfContained()), AbseilStringsMatchHeader( Options.get("AbseilStringsMatchHeader", "absl/strings/match.h")) {} diff --git a/clang-tools-extra/clang-tidy/bugprone/ImplicitWideningOfMultiplicationResultCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/ImplicitWideningOfMultiplicationResultCheck.cpp index 29b2bdfc66c02..fbf3438889efd 100644 --- a/clang-tools-extra/clang-tidy/bugprone/ImplicitWideningOfMultiplicationResultCheck.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/ImplicitWideningOfMultiplicationResultCheck.cpp @@ -43,8 +43,8 @@ ImplicitWideningOfMultiplicationResultCheck:: Options.get("UseCXXStaticCastsInCppSources", true)), UseCXXHeadersInCppSources(Options.get("UseCXXHeadersInCppSources", true)), IncludeInserter(Options.getLocalOrGlobal("IncludeStyle", - utils::IncludeSorter::IS_LLVM)) { -} + utils::IncludeSorter::IS_LLVM), + areDiagsSelfContained()) {} void ImplicitWideningOfMultiplicationResultCheck::registerPPCallbacks( const SourceManager &SM, Preprocessor *PP, Preprocessor *ModuleExpanderPP) { diff --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/InitVariablesCheck.cpp b/clang-tools-extra/clang-tidy/cppcoreguidelines/InitVariablesCheck.cpp index cded53487ae8f..efe694426728e 100644 --- a/clang-tools-extra/clang-tidy/cppcoreguidelines/InitVariablesCheck.cpp +++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/InitVariablesCheck.cpp @@ -27,7 +27,8 @@ InitVariablesCheck::InitVariablesCheck(StringRef Name, ClangTidyContext *Context) : ClangTidyCheck(Name, Context), IncludeInserter(Options.getLocalOrGlobal("IncludeStyle", - utils::IncludeSorter::IS_LLVM)), + utils::IncludeSorter::IS_LLVM), + areDiagsSelfContained()), MathHeader(Options.get("MathHeader", "<math.h>")) {} void InitVariablesCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) { diff --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/PreferMemberInitializerCheck.cpp b/clang-tools-extra/clang-tidy/cppcoreguidelines/PreferMemberInitializerCheck.cpp index 4ff362476a0c3..16a2a69988546 100644 --- a/clang-tools-extra/clang-tidy/cppcoreguidelines/PreferMemberInitializerCheck.cpp +++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/PreferMemberInitializerCheck.cpp @@ -306,10 +306,10 @@ void PreferMemberInitializerCheck::check( NewInit, AddComma ? "), " : ")"}); Diag << FixItHint::CreateInsertion(InsertPos, Insertion, FirstToCtorInits); + FirstToCtorInits = areDiagsSelfContained(); } Diag << FixItHint::CreateRemoval( CharSourceRange::getCharRange(S->getBeginLoc(), SemiColonEnd)); - FirstToCtorInits = false; } } } diff --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/ProBoundsConstantArrayIndexCheck.cpp b/clang-tools-extra/clang-tidy/cppcoreguidelines/ProBoundsConstantArrayIndexCheck.cpp index d7bc56511dfb5..8c3457d9c5580 100644 --- a/clang-tools-extra/clang-tidy/cppcoreguidelines/ProBoundsConstantArrayIndexCheck.cpp +++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/ProBoundsConstantArrayIndexCheck.cpp @@ -22,7 +22,8 @@ ProBoundsConstantArrayIndexCheck::ProBoundsConstantArrayIndexCheck( StringRef Name, ClangTidyContext *Context) : ClangTidyCheck(Name, Context), GslHeader(Options.get("GslHeader", "")), Inserter(Options.getLocalOrGlobal("IncludeStyle", - utils::IncludeSorter::IS_LLVM)) {} + utils::IncludeSorter::IS_LLVM), + areDiagsSelfContained()) {} void ProBoundsConstantArrayIndexCheck::storeOptions( ClangTidyOptions::OptionMap &Opts) { diff --git a/clang-tools-extra/clang-tidy/misc/UniqueptrResetReleaseCheck.cpp b/clang-tools-extra/clang-tidy/misc/UniqueptrResetReleaseCheck.cpp index a063fd9c99066..7ffbaa44af272 100644 --- a/clang-tools-extra/clang-tidy/misc/UniqueptrResetReleaseCheck.cpp +++ b/clang-tools-extra/clang-tidy/misc/UniqueptrResetReleaseCheck.cpp @@ -20,7 +20,8 @@ UniqueptrResetReleaseCheck::UniqueptrResetReleaseCheck( StringRef Name, ClangTidyContext *Context) : ClangTidyCheck(Name, Context), Inserter(Options.getLocalOrGlobal("IncludeStyle", - utils::IncludeSorter::IS_LLVM)) {} + utils::IncludeSorter::IS_LLVM), + areDiagsSelfContained()) {} void UniqueptrResetReleaseCheck::storeOptions( ClangTidyOptions::OptionMap &Opts) { diff --git a/clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp b/clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp index b0b882be1a6c7..accc95d126f83 100644 --- a/clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp +++ b/clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp @@ -463,7 +463,8 @@ LoopConvertCheck::LoopConvertCheck(StringRef Name, ClangTidyContext *Context) MinConfidence(Options.get("MinConfidence", Confidence::CL_Reasonable)), NamingStyle(Options.get("NamingStyle", VariableNamer::NS_CamelCase)), Inserter(Options.getLocalOrGlobal("IncludeStyle", - utils::IncludeSorter::IS_LLVM)), + utils::IncludeSorter::IS_LLVM), + areDiagsSelfContained()), UseCxx20IfAvailable(Options.get("UseCxx20ReverseRanges", true)), ReverseFunction(Options.get("MakeReverseRangeFunction", "")), ReverseHeader(Options.get("MakeReverseRangeHeader", "")) { @@ -800,9 +801,12 @@ bool LoopConvertCheck::isConvertible(ASTContext *Context, const ast_matchers::BoundNodes &Nodes, const ForStmt *Loop, LoopFixerKind FixerKind) { - // If we already modified the range of this for loop, don't do any further - // updates on this iteration. - if (TUInfo->getReplacedVars().count(Loop)) + // In self contained diagnosics mode we don't want dependancies on other + // loops, otherwise, If we already modified the range of this for loop, don't + // do any further updates on this iteration. + if (areDiagsSelfContained()) + TUInfo = std::make_unique<TUTrackingInfo>(); + else if (TUInfo->getReplacedVars().count(Loop)) return false; // Check that we have exactly one index variable and at most one end variable. diff --git a/clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp b/clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp index 54a5c5ae7e9a8..7992fa0c990af 100644 --- a/clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp +++ b/clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp @@ -44,7 +44,8 @@ MakeSmartPtrCheck::MakeSmartPtrCheck(StringRef Name, ClangTidyContext *Context, StringRef MakeSmartPtrFunctionName) : ClangTidyCheck(Name, Context), Inserter(Options.getLocalOrGlobal("IncludeStyle", - utils::IncludeSorter::IS_LLVM)), + utils::IncludeSorter::IS_LLVM), + areDiagsSelfContained()), MakeSmartPtrFunctionHeader( Options.get("MakeSmartPtrFunctionHeader", "<memory>")), MakeSmartPtrFunctionName( diff --git a/clang-tools-extra/clang-tidy/modernize/PassByValueCheck.cpp b/clang-tools-extra/clang-tidy/modernize/PassByValueCheck.cpp index 2e1f2143947e9..cbfe528eaa7bb 100644 --- a/clang-tools-extra/clang-tidy/modernize/PassByValueCheck.cpp +++ b/clang-tools-extra/clang-tidy/modernize/PassByValueCheck.cpp @@ -190,7 +190,8 @@ collectParamDecls(const CXXConstructorDecl *Ctor, PassByValueCheck::PassByValueCheck(StringRef Name, ClangTidyContext *Context) : ClangTidyCheck(Name, Context), Inserter(Options.getLocalOrGlobal("IncludeStyle", - utils::IncludeSorter::IS_LLVM)), + utils::IncludeSorter::IS_LLVM), + areDiagsSelfContained()), ValuesOnly(Options.get("ValuesOnly", false)) {} void PassByValueCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) { diff --git a/clang-tools-extra/clang-tidy/modernize/ReplaceAutoPtrCheck.cpp b/clang-tools-extra/clang-tidy/modernize/ReplaceAutoPtrCheck.cpp index 746d2cfed0241..84c17effb9bc4 100644 --- a/clang-tools-extra/clang-tidy/modernize/ReplaceAutoPtrCheck.cpp +++ b/clang-tools-extra/clang-tidy/modernize/ReplaceAutoPtrCheck.cpp @@ -41,7 +41,8 @@ ReplaceAutoPtrCheck::ReplaceAutoPtrCheck(StringRef Name, ClangTidyContext *Context) : ClangTidyCheck(Name, Context), Inserter(Options.getLocalOrGlobal("IncludeStyle", - utils::IncludeSorter::IS_LLVM)) {} + utils::IncludeSorter::IS_LLVM), + areDiagsSelfContained()) {} void ReplaceAutoPtrCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) { Options.store(Opts, "IncludeStyle", Inserter.getStyle()); diff --git a/clang-tools-extra/clang-tidy/modernize/ReplaceRandomShuffleCheck.cpp b/clang-tools-extra/clang-tidy/modernize/ReplaceRandomShuffleCheck.cpp index 3cb934e4a0f4c..2f17065b480dd 100644 --- a/clang-tools-extra/clang-tidy/modernize/ReplaceRandomShuffleCheck.cpp +++ b/clang-tools-extra/clang-tidy/modernize/ReplaceRandomShuffleCheck.cpp @@ -24,8 +24,8 @@ ReplaceRandomShuffleCheck::ReplaceRandomShuffleCheck(StringRef Name, ClangTidyContext *Context) : ClangTidyCheck(Name, Context), IncludeInserter(Options.getLocalOrGlobal("IncludeStyle", - utils::IncludeSorter::IS_LLVM)) { -} + utils::IncludeSorter::IS_LLVM), + areDiagsSelfContained()) {} void ReplaceRandomShuffleCheck::registerMatchers(MatchFinder *Finder) { const auto Begin = hasArgument(0, expr()); diff --git a/clang-tools-extra/clang-tidy/performance/TypePromotionInMathFnCheck.cpp b/clang-tools-extra/clang-tidy/performance/TypePromotionInMathFnCheck.cpp index 8f91200c28e4d..4f7e6fe6f2981 100644 --- a/clang-tools-extra/clang-tidy/performance/TypePromotionInMathFnCheck.cpp +++ b/clang-tools-extra/clang-tidy/performance/TypePromotionInMathFnCheck.cpp @@ -32,8 +32,8 @@ TypePromotionInMathFnCheck::TypePromotionInMathFnCheck( StringRef Name, ClangTidyContext *Context) : ClangTidyCheck(Name, Context), IncludeInserter(Options.getLocalOrGlobal("IncludeStyle", - utils::IncludeSorter::IS_LLVM)) { -} + utils::IncludeSorter::IS_LLVM), + areDiagsSelfContained()) {} void TypePromotionInMathFnCheck::registerPPCallbacks( const SourceManager &SM, Preprocessor *PP, Preprocessor *ModuleExpanderPP) { diff --git a/clang-tools-extra/clang-tidy/performance/UnnecessaryValueParamCheck.cpp b/clang-tools-extra/clang-tidy/performance/UnnecessaryValueParamCheck.cpp index 2a9f9b7051775..ba2b89c291214 100644 --- a/clang-tools-extra/clang-tidy/performance/UnnecessaryValueParamCheck.cpp +++ b/clang-tools-extra/clang-tidy/performance/UnnecessaryValueParamCheck.cpp @@ -69,7 +69,8 @@ UnnecessaryValueParamCheck::UnnecessaryValueParamCheck( StringRef Name, ClangTidyContext *Context) : ClangTidyCheck(Name, Context), Inserter(Options.getLocalOrGlobal("IncludeStyle", - utils::IncludeSorter::IS_LLVM)), + utils::IncludeSorter::IS_LLVM), + areDiagsSelfContained()), AllowedTypes( utils::options::parseStringList(Options.get("AllowedTypes", ""))) {} diff --git a/clang-tools-extra/clang-tidy/utils/IncludeInserter.cpp b/clang-tools-extra/clang-tidy/utils/IncludeInserter.cpp index b8bd50d9bc3f1..144ebdf5dbbfe 100644 --- a/clang-tools-extra/clang-tidy/utils/IncludeInserter.cpp +++ b/clang-tools-extra/clang-tidy/utils/IncludeInserter.cpp @@ -36,8 +36,9 @@ class IncludeInserterCallback : public PPCallbacks { IncludeInserter *Inserter; }; -IncludeInserter::IncludeInserter(IncludeSorter::IncludeStyle Style) - : Style(Style) {} +IncludeInserter::IncludeInserter(IncludeSorter::IncludeStyle Style, + bool SelfContainedDiags) + : Style(Style), SelfContainedDiags(SelfContainedDiags) {} void IncludeInserter::registerPreprocessor(Preprocessor *PP) { assert(PP && "PP shouldn't be null"); @@ -73,7 +74,9 @@ IncludeInserter::createIncludeInsertion(FileID FileID, llvm::StringRef Header) { return llvm::None; // We assume the same Header will never be included both angled and not // angled. - if (!InsertedHeaders[FileID].insert(Header).second) + // In self contained diags mode we don't track what headers we have already + // inserted. + if (!SelfContainedDiags && !InsertedHeaders[FileID].insert(Header).second) return llvm::None; return getOrCreate(FileID).createIncludeInsertion(Header, IsAngled); diff --git a/clang-tools-extra/clang-tidy/utils/IncludeInserter.h b/clang-tools-extra/clang-tidy/utils/IncludeInserter.h index 74903b2d166db..c5649177b2286 100644 --- a/clang-tools-extra/clang-tidy/utils/IncludeInserter.h +++ b/clang-tools-extra/clang-tidy/utils/IncludeInserter.h @@ -59,7 +59,8 @@ class IncludeInserter { /// using \code /// Options.getLocalOrGlobal("IncludeStyle", <DefaultStyle>) /// \endcode - explicit IncludeInserter(IncludeSorter::IncludeStyle Style); + explicit IncludeInserter(IncludeSorter::IncludeStyle Style, + bool SelfContainedDiags); /// Registers this with the Preprocessor \p PP, must be called before this /// class is used. @@ -93,6 +94,7 @@ class IncludeInserter { llvm::DenseMap<FileID, llvm::StringSet<>> InsertedHeaders; const SourceManager *SourceMgr{nullptr}; const IncludeSorter::IncludeStyle Style; + const bool SelfContainedDiags; friend class IncludeInserterCallback; }; diff --git a/clang-tools-extra/clang-tidy/utils/TransformerClangTidyCheck.cpp b/clang-tools-extra/clang-tidy/utils/TransformerClangTidyCheck.cpp index c18838fb0f992..b8b2c253aa492 100644 --- a/clang-tools-extra/clang-tidy/utils/TransformerClangTidyCheck.cpp +++ b/clang-tools-extra/clang-tidy/utils/TransformerClangTidyCheck.cpp @@ -30,8 +30,8 @@ static void verifyRule(const RewriteRuleWith<std::string> &Rule) { TransformerClangTidyCheck::TransformerClangTidyCheck(StringRef Name, ClangTidyContext *Context) : ClangTidyCheck(Name, Context), - Inserter( - Options.getLocalOrGlobal("IncludeStyle", IncludeSorter::IS_LLVM)) {} + Inserter(Options.getLocalOrGlobal("IncludeStyle", IncludeSorter::IS_LLVM), + areDiagsSelfContained()) {} // This constructor cannot dispatch to the simpler one (below), because, in // order to get meaningful results from `getLangOpts` and `Options`, we need the diff --git a/clang-tools-extra/clangd/ParsedAST.cpp b/clang-tools-extra/clangd/ParsedAST.cpp index e40657d7a0c34..f6ccfed3e0e5c 100644 --- a/clang-tools-extra/clangd/ParsedAST.cpp +++ b/clang-tools-extra/clangd/ParsedAST.cpp @@ -412,6 +412,7 @@ ParsedAST::build(llvm::StringRef Filename, const ParseInputs &Inputs, CTContext->setDiagnosticsEngine(&Clang->getDiagnostics()); CTContext->setASTContext(&Clang->getASTContext()); CTContext->setCurrentFile(Filename); + CTContext->setSelfContainedDiags(true); CTChecks = CTFactories.createChecks(CTContext.getPointer()); llvm::erase_if(CTChecks, [&](const auto &Check) { return !Check->isLanguageVersionSupported(CTContext->getLangOpts()); diff --git a/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp b/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp index 5a5a53679facc..4aea1f06087c7 100644 --- a/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp +++ b/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp @@ -678,6 +678,67 @@ TEST(DiagnosticTest, ElseAfterReturnRange) { Diag(Main.range(), "do not use 'else' after 'return'")))); } +TEST(DiagnosticTest, ClangTidySelfContainedDiags) { + Annotations Main(R"cpp($MathHeader[[]] + struct Foo{ + int A, B; + Foo()$Fix[[]] { + $A[[A = 1;]] + $B[[B = 1;]] + } + }; + void InitVariables() { + float $C[[C]]$CFix[[]]; + double $D[[D]]$DFix[[]]; + } + )cpp"); + TestTU TU = TestTU::withCode(Main.code()); + TU.ClangTidyProvider = + addTidyChecks("cppcoreguidelines-prefer-member-initializer," + "cppcoreguidelines-init-variables"); + clangd::Fix ExpectedAFix; + ExpectedAFix.Message = + "'A' should be initialized in a member initializer of the constructor"; + ExpectedAFix.Edits.push_back(TextEdit{Main.range("Fix"), " : A(1)"}); + ExpectedAFix.Edits.push_back(TextEdit{Main.range("A"), ""}); + + // When invoking clang-tidy normally, this code would produce `, B(1)` as the + // fix the `B` member, as it would think its already included the ` : ` from + // the previous `A` fix. + clangd::Fix ExpectedBFix; + ExpectedBFix.Message = + "'B' should be initialized in a member initializer of the constructor"; + ExpectedBFix.Edits.push_back(TextEdit{Main.range("Fix"), " : B(1)"}); + ExpectedBFix.Edits.push_back(TextEdit{Main.range("B"), ""}); + + clangd::Fix ExpectedCFix; + ExpectedCFix.Message = "variable 'C' is not initialized"; + ExpectedCFix.Edits.push_back(TextEdit{Main.range("CFix"), " = NAN"}); + ExpectedCFix.Edits.push_back( + TextEdit{Main.range("MathHeader"), "#include <math.h>\n\n"}); + + // Again in clang-tidy only the include directive would be emitted for the + // first warning. However we need the include attaching for both warnings. + clangd::Fix ExpectedDFix; + ExpectedDFix.Message = "variable 'D' is not initialized"; + ExpectedDFix.Edits.push_back(TextEdit{Main.range("DFix"), " = NAN"}); + ExpectedDFix.Edits.push_back( + TextEdit{Main.range("MathHeader"), "#include <math.h>\n\n"}); + EXPECT_THAT( + *TU.build().getDiagnostics(), + UnorderedElementsAre( + AllOf(Diag(Main.range("A"), "'A' should be initialized in a member " + "initializer of the constructor"), + withFix(equalToFix(ExpectedAFix))), + AllOf(Diag(Main.range("B"), "'B' should be initialized in a member " + "initializer of the constructor"), + withFix(equalToFix(ExpectedBFix))), + AllOf(Diag(Main.range("C"), "variable 'C' is not initialized"), + withFix(equalToFix(ExpectedCFix))), + AllOf(Diag(Main.range("D"), "variable 'D' is not initialized"), + withFix(equalToFix(ExpectedDFix))))); +} + TEST(DiagnosticsTest, Preprocessor) { // This looks like a preamble, but there's an #else in the middle! // Check that: diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index 5392ab5758f59..9472797eca86e 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -53,6 +53,7 @@ Inlay hints Diagnostics ^^^^^^^^^^^ +- Improved Fix-its of some clang-tidy checks when applied with clangd. Semantic Highlighting ^^^^^^^^^^^^^^^^^^^^^ diff --git a/clang-tools-extra/unittests/clang-tidy/IncludeInserterTest.cpp b/clang-tools-extra/unittests/clang-tidy/IncludeInserterTest.cpp index 1499197e9486d..a736c557a0a09 100644 --- a/clang-tools-extra/unittests/clang-tidy/IncludeInserterTest.cpp +++ b/clang-tools-extra/unittests/clang-tidy/IncludeInserterTest.cpp @@ -30,8 +30,10 @@ class IncludeInserterCheckBase : public ClangTidyCheck { public: IncludeInserterCheckBase(StringRef CheckName, ClangTidyContext *Context, utils::IncludeSorter::IncludeStyle Style = - utils::IncludeSorter::IS_Google) - : ClangTidyCheck(CheckName, Context), Inserter(Style) {} + utils::IncludeSorter::IS_Google, + bool SelfContainedDiags = false) + : ClangTidyCheck(CheckName, Context), + Inserter(Style, SelfContainedDiags) {} void registerPPCallbacks(const SourceManager &SM, Preprocessor *PP, Preprocessor *ModuleExpanderPP) override { @@ -85,6 +87,19 @@ class MultipleHeaderInserterCheck : public IncludeInserterCheckBase { } }; +class MultipleHeaderSingleInserterCheck : public IncludeInserterCheckBase { +public: + MultipleHeaderSingleInserterCheck(StringRef CheckName, + ClangTidyContext *Context) + : IncludeInserterCheckBase(CheckName, Context, + utils::IncludeSorter::IS_Google, + /*SelfContainedDiags=*/true) {} + + std::vector<StringRef> headersToInclude() const override { + return {"path/to/header.h", "path/to/header2.h", "path/to/header.h"}; + } +}; + class CSystemIncludeInserterCheck : public IncludeInserterCheckBase { public: CSystemIncludeInserterCheck(StringRef CheckName, ClangTidyContext *Context) @@ -246,6 +261,41 @@ void foo() { PreCode, "clang_tidy/tests/insert_includes_test_input2.cc")); } +TEST(IncludeInserterTest, InsertMultipleIncludesNoDeduplicate) { + const char *PreCode = R"( +#include "clang_tidy/tests/insert_includes_test_header.h" + +#include <list> +#include <map> + +#include "path/to/a/header.h" + +void foo() { + int a = 0; +})"; + // FIXME: ClangFormat bug - https://bugs.llvm.org/show_bug.cgi?id=49298 + // clang-format off + const char *PostCode = R"( +#include "clang_tidy/tests/insert_includes_test_header.h" + +#include <list> +#include <map> + +#include "path/to/a/header.h" +#include "path/to/header.h" +#include "path/to/header2.h" +#include "path/to/header.h" + +void foo() { + int a = 0; +})"; + // clang-format on + + EXPECT_EQ(PostCode, + runCheckOnCode<MultipleHeaderSingleInserterCheck>( + PreCode, "clang_tidy/tests/insert_includes_test_input2.cc")); +} + TEST(IncludeInserterTest, InsertBeforeFirstNonSystemInclude) { const char *PreCode = R"( #include "clang_tidy/tests/insert_includes_test_header.h" _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits