[clang-tidy] Add a Standalone diagnostics mode to clang-tidy

[njames93]

Summary

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

struct Foo {
  int A, B;
  Foo(int D, int E) {
    A = D;
    B = E; // Fix Here
  }
};

Incorrectly transformed to:

struct Foo {
  int A, B;
  Foo(int D, int E), B(E) {
    A = D;
     // Fix Here
  }
};

In StandaloneDiagsMode, it gets transformed to:

struct Foo {
  int A, B;
  Foo(int D, int E) : B(E) {
    A = D;
     // Fix Here
  }
};

[njames93]

Fix test

[njames93]

Disable formatting causing test to fail and document it.

[njames93]

Remove unintended change.

[steveire]

I just tried to apply this, but I think it conflicts with your other change to PreferMemberInitializerCheck.cpp. Please rebase it.

[steveire]

clang-tools-extra/clang-tidy/ClangTidyCheck.h

420

Very minor, but I think this should be run.

clang-tools-extra/clang-tidy/utils/IncludeInserter.cpp

81

The comment and the code seem to me to be opposites. The code says "if SingleFixMode is true and we have already inserted the header, don't insert it again." The comment says that "SingleFixMode does not prevent inserting it again" -- The comment seems to be the opposite of the code?

[njames93]

clang-tools-extra/clang-tidy/utils/IncludeInserter.cpp

81

That't not what the code is saying, the magic is clear in the else branch. If SingleFixMode is enabled we are only querying the set, If its disabled then we are updating the set.
The real shortfall is the map is called InsertedHeaders which is not a good reflection of what it actually contains. It actually contains all the headers included in each file. Which in Multi fix mode gets updated with each include insertion.

[steveire]

clang-tools-extra/clang-tidy/utils/IncludeInserter.cpp

81

Ah, the "and we have already inserted the header, don't insert it again" is not true because we never do an insert in SingleFixMode.

Perhaps update the comment to say that the InsertedHeaders is never added to in SingleFixMode, so it only contains the headers that are already there? If something like that is true.

clang-tools-extra/unittests/clang-tidy/IncludeInserterTest.cpp

95

I think the true here points to this being a boolean trap. Consider using an enum for the parameter instead.

287

I still find it really confusing that the "single inserter" mode results in multiple of the same header being added. Perhaps the names should be along the lines of "duplicating" and "deduplicating" instead of "single" and "multi"?

[njames93]

Add test cases to clangd showing the improved behaviour of fix-its.
Rebased.
Update some comments.

[njames93]

Add include insertion test for clangd showing it attaching the include for multiple warnings.

[steveire]

clang-tools-extra/clang-tidy/ClangTidyCheck.h

422

I find the naming of this as "single fix" confusing.

Something along the lines of "self contained diagnostics" or so I think would be better.

[njames93]

Rename mode to self contained diags.
Added support for modernize-loop-convert. Currently transforming a loop will prevent any nested loops being transformed if they are affected by the first transformation. This is counter productive in self contained diags mode.

[steveire]

clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h

190

I don't think this Value should be = true. It makes call sites confusing.

There is precedent though according to

git grep "void set.*bool.*=.*)"

so consider this a nit.

229

The order of members here is starting to look suspicious, size-wise. Not something for this change though.

clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp

803–804

Can you update this comment to not refer to "single fix" mode?

clang-tools-extra/clang-tidy/utils/IncludeInserter.h

63

Should the "single fix" naming here also be changed?

clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp

681

More "single fix" naming here.

clang-tools-extra/unittests/clang-tidy/IncludeInserterTest.cpp

34

"Single fix" naming here too.

[njames93]

clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h

229

Not something I'm prepared to worry about in this patch.

clang-tools-extra/unittests/clang-tidy/IncludeInserterTest.cpp

95

I don't see a point in defining an enum for this when its only a test case, though a parameter comment should be added.

287

The name of the test is InsertMultipleIncludesNoDeduplicate, Is that not sufficient?

[njames93]

Update.

[njames93]

Turns out we don't need to worry about InsertedHeaders at all in IncludeInserter.

[steveire]

clang-tools-extra/unittests/clang-tidy/IncludeInserterTest.cpp

287

Yes, the comment you're responding to here is quite old and predates the renaming.

[njames93]

Rebase and fix up new checks added and changes to tests.

[njames93]

Rebase

[njames93]

Rebase

[aaron.ballman]

I'd like to hear from @sammccall as well, but I think the changes here LGTM. Can you please add a release note for the fix?

[sammccall]

Thank you!

(I'm sorry, somehow I had misunderstood what this patch was about, and wasn't paying attention)

[njames93]

In D97121#3448467, @aaron.ballman wrote:

I'd like to hear from @sammccall as well, but I think the changes here LGTM. Can you please add a release note for the fix?

Is this a clang-tidy, clangd or both release notes.
The changes aren't visible in the clang-tidy binary, but are when using it as a library.
However these changes do have noticable effects in clangd with some clang-tidy checks running.

[aaron.ballman]

In D97121#3452435, @njames93 wrote:

In D97121#3448467, @aaron.ballman wrote:

I'd like to hear from @sammccall as well, but I think the changes here LGTM. Can you please add a release note for the fix?

Is this a clang-tidy, clangd or both release notes.
The changes aren't visible in the clang-tidy binary, but are when using it as a library.
However these changes do have noticable effects in clangd with some clang-tidy checks running.

I'd probably put the note under clangd as that's going to be most relevant for users, WDYT?

[njames93]

Added clangd release notes.

clang-tools-extra/clang-tidy/ClangTidyCheck.h

private:
   ClangTidyContext *Context;
 
 protected:
   OptionsView Options;
   /// Returns the main file name of the current translation unit.
   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.
 ///
 /// Reads the option with the check-local name \p LocalName from the
 /// ``CheckOptions``. If the corresponding key is not present, return
 /// ``None``.
 ///

clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h

public:
   }
 
   /// If the experimental alpha checkers from the static analyzer can be
   /// enabled.
   bool canEnableAnalyzerAlphaCheckers() const {
     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) {
     return DiagLevelAndFormatString(
         static_cast<DiagnosticIDs::Level>(
             DiagEngine->getDiagnosticLevel(DiagnosticID, Loc)),
         std::string(
             DiagEngine->getDiagnosticIDs()->getDescription(DiagnosticID)));
private:
   std::string CurrentBuildDirectory;
 
   llvm::DenseMap<unsigned, std::string> CheckNamesByDiagnosticID;
 
   bool Profile;
   std::string ProfilePrefix;
 
   bool AllowEnablingAnalyzerAlphaCheckers;
 
+  bool SelfContainedDiags;
+
   NoLintDirectiveHandler NoLintHandler;
 };
 
 /// Gets the Fix attached to \p Diagnostic.
 /// 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<tooling::Replacements> *

clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp

ClangTidyError::ClangTidyError(StringRef CheckName,
     : tooling::Diagnostic(CheckName, DiagLevel, BuildDirectory),
       IsWarningAsError(IsWarningAsError) {}
 
 ClangTidyContext::ClangTidyContext(
     std::unique_ptr<ClangTidyOptionsProvider> OptionsProvider,
     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("");
 }
 
 ClangTidyContext::~ClangTidyContext() = default;
 
 DiagnosticBuilder ClangTidyContext::diag(

clang-tools-extra/clang-tidy/abseil/StringFindStartswithCheck.cpp

 namespace abseil {
 
 StringFindStartswithCheck::StringFindStartswithCheck(StringRef Name,
                                                      ClangTidyContext *Context)
     : ClangTidyCheck(Name, Context),
       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")) {}
 
 void StringFindStartswithCheck::registerMatchers(MatchFinder *Finder) {
   auto ZeroLiteral = integerLiteral(equals(0));
   auto StringClassMatcher = cxxRecordDecl(hasAnyName(SmallVector<StringRef, 4>(
       StringLikeClasses.begin(), StringLikeClasses.end())));
   auto StringType = hasUnqualifiedDesugaredType(

clang-tools-extra/clang-tidy/bugprone/ImplicitWideningOfMultiplicationResultCheck.cpp

 ImplicitWideningOfMultiplicationResultCheck::
     ImplicitWideningOfMultiplicationResultCheck(StringRef Name,
                                                 ClangTidyContext *Context)
     : ClangTidyCheck(Name, Context),
       UseCXXStaticCastsInCppSources(
           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) {
   IncludeInserter.registerPreprocessor(PP);
 }
 
 void ImplicitWideningOfMultiplicationResultCheck::storeOptions(
     ClangTidyOptions::OptionMap &Opts) {

clang-tools-extra/clang-tidy/cppcoreguidelines/InitVariablesCheck.cpp

 namespace {
 AST_MATCHER(VarDecl, isLocalVarDecl) { return Node.isLocalVarDecl(); }
 } // namespace
 
 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) {
   Options.store(Opts, "IncludeStyle", IncludeInserter.getStyle());
   Options.store(Opts, "MathHeader", MathHeader);
 }
 
 void InitVariablesCheck::registerMatchers(MatchFinder *Finder) {

clang-tools-extra/clang-tidy/cppcoreguidelines/PreferMemberInitializerCheck.cpp

if (Field) {
             Diag << FixItHint::CreateInsertion(InsertPos, NewInit);
           else
             Diag << FixItHint::CreateReplacement(ReplaceRange, NewInit);
         } else {
           SmallString<128> Insertion({InsertPrefix, Field->getName(), "(",
                                       NewInit, AddComma ? "), " : ")"});
           Diag << FixItHint::CreateInsertion(InsertPos, Insertion,
                                              FirstToCtorInits);
+          FirstToCtorInits = areDiagsSelfContained();
         }
         Diag << FixItHint::CreateRemoval(
             CharSourceRange::getCharRange(S->getBeginLoc(), SemiColonEnd));
-        FirstToCtorInits = false;
       }
     }
   }
 }
 
 } // namespace cppcoreguidelines
 } // namespace tidy
 } // namespace clang

clang-tools-extra/clang-tidy/cppcoreguidelines/ProBoundsConstantArrayIndexCheck.cpp

 namespace clang {
 namespace tidy {
 namespace cppcoreguidelines {
 
 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) {
   Options.store(Opts, "GslHeader", GslHeader);
   Options.store(Opts, "IncludeStyle", Inserter.getStyle());
 }
 
 void ProBoundsConstantArrayIndexCheck::registerPPCallbacks(

clang-tools-extra/clang-tidy/misc/UniqueptrResetReleaseCheck.cpp

 namespace clang {
 namespace tidy {
 namespace misc {
 
 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) {
   Options.store(Opts, "IncludeStyle", Inserter.getStyle());
 }
 
 void UniqueptrResetReleaseCheck::registerPPCallbacks(
     const SourceManager &SM, Preprocessor *PP, Preprocessor *ModuleExpanderPP) {

clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp

: ContainerNeedsDereference(false), DerefByConstRef(false),
       DerefByValue(false), NeedsReverseCall(false) {}
 
 LoopConvertCheck::LoopConvertCheck(StringRef Name, ClangTidyContext *Context)
     : ClangTidyCheck(Name, Context), TUInfo(new TUTrackingInfo),
       MaxCopySize(Options.get("MaxCopySize", 16ULL)),
       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", "")) {
 
   if (ReverseFunction.empty() && !ReverseHeader.empty()) {
     configurationDiag(
         "modernize-loop-convert: 'MakeReverseRangeHeader' is set but "
         "'MakeReverseRangeFunction' is not, disabling reverse loop "
else
     getArrayLoopQualifiers(Context, Nodes, ContainerExpr, Usages, Descriptor);
 }
 
 /// Check some of the conditions that must be met for the loop to be
 /// convertible.
 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.
   const auto *InitVar = Nodes.getNodeAs<VarDecl>(InitVarName);
 
   // FIXME: Try to put most of this logic inside a matcher.
   if (FixerKind == LFK_Iterator || FixerKind == LFK_ReverseIterator) {
     QualType InitVarType = InitVar->getType();

clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp

 } // namespace
 
 const char MakeSmartPtrCheck::PointerType[] = "pointerType";
 
 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(
           Options.get("MakeSmartPtrFunction", MakeSmartPtrFunctionName)),
       IgnoreMacros(Options.getLocalOrGlobal("IgnoreMacros", true)),
       IgnoreDefaultInitialization(
           Options.get("IgnoreDefaultInitialization", true)) {}
 

clang-tools-extra/clang-tidy/modernize/PassByValueCheck.cpp

collectParamDecls(const CXXConstructorDecl *Ctor,
   for (const FunctionDecl *Redecl : Ctor->redecls())
     Results.push_back(Redecl->getParamDecl(ParamIdx));
   return Results;
 }
 
 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) {
   Options.store(Opts, "IncludeStyle", Inserter.getStyle());
   Options.store(Opts, "ValuesOnly", ValuesOnly);
 }
 
 void PassByValueCheck::registerMatchers(MatchFinder *Finder) {

clang-tools-extra/clang-tidy/modernize/ReplaceAutoPtrCheck.cpp

 AST_MATCHER(Expr, isLValue) { return Node.getValueKind() == VK_LValue; }
 
 } // namespace
 
 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());
 }
 
 void ReplaceAutoPtrCheck::registerMatchers(MatchFinder *Finder) {
   auto AutoPtrDecl = recordDecl(hasName("auto_ptr"), isInStdNamespace());
   auto AutoPtrType = qualType(hasDeclaration(AutoPtrDecl));

clang-tools-extra/clang-tidy/modernize/ReplaceRandomShuffleCheck.cpp

 namespace clang {
 namespace tidy {
 namespace modernize {
 
 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());
   const auto End = hasArgument(1, expr());
   const auto RandomFunc = hasArgument(2, expr().bind("randomFunc"));
   Finder->addMatcher(
       traverse(
           TK_AsIs,

clang-tools-extra/clang-tidy/performance/TypePromotionInMathFnCheck.cpp

AST_MATCHER_P(Type, isBuiltinType, BuiltinType::Kind, Kind) {
   return false;
 }
 } // anonymous namespace
 
 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) {
   IncludeInserter.registerPreprocessor(PP);
 }
 
 void TypePromotionInMathFnCheck::storeOptions(
     ClangTidyOptions::OptionMap &Opts) {

clang-tools-extra/clang-tidy/performance/UnnecessaryValueParamCheck.cpp

 }
 
 } // namespace
 
 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", ""))) {}
 
 void UnnecessaryValueParamCheck::registerMatchers(MatchFinder *Finder) {
   const auto ExpensiveValueParamDecl = parmVarDecl(
       hasType(qualType(
           hasCanonicalType(matchers::isExpensiveToCopy()),
           unless(anyOf(hasCanonicalType(referenceType()),

clang-tools-extra/clang-tidy/utils/IncludeInserter.h

 /// \endcode
 class IncludeInserter {
 public:
   /// Initializes the IncludeInserter using the IncludeStyle \p Style.
   /// In most cases the \p Style will be retrieved from the ClangTidyOptions
   /// 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.
   void registerPreprocessor(Preprocessor *PP);
 
   /// Creates a \p Header inclusion directive fixit in the File \p FileID.
   /// When \p Header is enclosed in angle brackets, uses angle brackets in the
   /// inclusion directive, otherwise uses quotes.
void addInclude(StringRef FileName, bool IsAngled,
                   SourceLocation HashLocation, SourceLocation EndLocation);
 
   IncludeSorter &getOrCreate(FileID FileID);
 
   llvm::DenseMap<FileID, std::unique_ptr<IncludeSorter>> IncludeSorterByFile;
   llvm::DenseMap<FileID, llvm::StringSet<>> InsertedHeaders;
   const SourceManager *SourceMgr{nullptr};
   const IncludeSorter::IncludeStyle Style;
+  const bool SelfContainedDiags;
   friend class IncludeInserterCallback;
 };
 
 } // namespace utils
 } // namespace tidy
 } // namespace clang
 #endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_INCLUDEINSERTER_H

clang-tools-extra/clang-tidy/utils/IncludeInserter.cpp

void InclusionDirective(SourceLocation HashLocation,
     Inserter->addInclude(FileNameRef, IsAngled, HashLocation,
                          IncludeToken.getEndLoc());
   }
 
 private:
   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");
   SourceMgr = &PP->getSourceManager();
 
   // If this gets registered multiple times, clear the maps
   if (!IncludeSorterByFile.empty())
     IncludeSorterByFile.clear();

 
 llvm::Optional<FixItHint>
 IncludeInserter::createIncludeInsertion(FileID FileID, llvm::StringRef Header) {
   bool IsAngled = Header.consume_front("<");
   if (IsAngled != Header.consume_back(">"))
     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);
 }
 
 llvm::Optional<FixItHint>
 IncludeInserter::createMainFileIncludeInsertion(StringRef Header) {
   assert(SourceMgr && "SourceMgr shouldn't be null; did you remember to call "
                       "registerPreprocessor()?");
   return createIncludeInsertion(SourceMgr->getMainFileID(), Header);

clang-tools-extra/clang-tidy/utils/TransformerClangTidyCheck.cpp

static void verifyRule(const RewriteRuleWith<std::string> &Rule) {
   assert(llvm::all_of(Rule.Metadata, hasGenerator) &&
          "clang-tidy checks must have an explanation by default;"
          " explicitly provide an empty explanation if none is desired");
 }
 
 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
 // `ClangTidyCheck()` constructor to have been called. If we were to dispatch,
 // we would be accessing `getLangOpts` and `Options` before the underlying
 // `ClangTidyCheck` instance was properly initialized.
 TransformerClangTidyCheck::TransformerClangTidyCheck(
     std::function<Optional<RewriteRuleWith<std::string>>(const LangOptions &,

clang-tools-extra/clangd/ParsedAST.cpp

if (PreserveDiags) {
     tidy::ClangTidyCheckFactories CTFactories;
     for (const auto &E : tidy::ClangTidyModuleRegistry::entries())
       E.instantiate()->addCheckFactories(CTFactories);
     CTContext.emplace(std::make_unique<tidy::DefaultOptionsProvider>(
         tidy::ClangTidyGlobalOptions(), ClangTidyOpts));
     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());
     });
     Preprocessor *PP = &Clang->getPreprocessor();
     for (const auto &Check : CTChecks) {
       Check->registerPPCallbacks(Clang->getSourceManager(), PP, PP);
       Check->registerMatchers(&CTFinder);

clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp

TEST(DiagnosticTest, ElseAfterReturnRange) {
   )cpp");
   TestTU TU = TestTU::withCode(Main.code());
   TU.ClangTidyProvider = addTidyChecks("llvm-else-after-return");
   EXPECT_THAT(*TU.build().getDiagnostics(),
               ifTidyChecks(ElementsAre(
                   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:
   //  - the #else doesn't generate diagnostics (we had this bug)
   //  - we get diagnostics from the taken branch
   //  - we get no diagnostics from the not taken branch
   Annotations Test(R"cpp(
     #ifndef FOO

clang-tools-extra/docs/ReleaseNotes.rst

 Improvements to clangd
 ----------------------
 
 Inlay hints
 ^^^^^^^^^^^
 
 Diagnostics
 ^^^^^^^^^^^
+- Improved Fix-its of some clang-tidy checks when applied with clangd.
 
 Semantic Highlighting
 ^^^^^^^^^^^^^^^^^^^^^
 
 Compile flags
 ^^^^^^^^^^^^^
 
 Hover

clang-tools-extra/unittests/clang-tidy/IncludeInserterTest.cpp

 namespace clang {
 namespace tidy {
 namespace {
 
 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 {
     Inserter.registerPreprocessor(PP);
   }
 
   void registerMatchers(ast_matchers::MatchFinder *Finder) override {
     Finder->addMatcher(ast_matchers::declStmt().bind("stmt"), this);
public:
   MultipleHeaderInserterCheck(StringRef CheckName, ClangTidyContext *Context)
       : IncludeInserterCheckBase(CheckName, Context) {}
 
   std::vector<StringRef> headersToInclude() const override {
     return {"path/to/header.h", "path/to/header2.h", "path/to/header.h"};
   }
 };
 
+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)
       : IncludeInserterCheckBase(CheckName, Context) {}
 
   std::vector<StringRef> headersToInclude() const override {
     return {"<stdlib.h>"};
   }
void foo() {
   int a = 0;
 })";
 
   EXPECT_EQ(PostCode,
             runCheckOnCode<MultipleHeaderInserterCheck>(
                 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"
 
 #include <list>
 #include <map>
 
 #include "path/to/z/header.h"