[clang-tools-extra] b859c39 - [clang-tidy] Add a Standalone diagnostics mode to clang-tidy

Nathan James via cfe-commits cfe-commits at lists.llvm.org
Sat Apr 16 01:53:46 PDT 2022


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"


        


More information about the cfe-commits mailing list