[clang-tidy] Added command line option `fix-notes`

[njames93]

Summary

Added an option to control whether to apply the fixes found in notes attached to clang tidy errors or not.
Diagnostics may contain multiple notes each offering different ways to fix the issue, for that reason the default behaviour should be to not look at fixes found in notes.
Instead offer up all the available fix-its in the output but don't try to apply the first one unless -fix-notes is supplied.

[njames93]

This is very much a work in progress
Another direction I was thinking was only apply the fixes found in notes if there is exactly one fix attached to the notes in a diagnostic, instead of just applying the first one we find

[Eugene.Zelenko]

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

106

It'll be reasonable to use member initialization for TotalFixes, AppliedFixes, WarningsAsErrors.

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

266–267

It'll be reasonable to use member initialization for LastErrorRelatesToUserCode, LastErrorPassesLineFilter, LastErrorWasIgnored.

[njames93]

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

106

It would be, but that's a refactoring change unrelated to this patch

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

266–267

Ditto

[njames93]

Made -fix-notes imply -fix

[njames93]

Fix documentation.

Ping??

[aaron.ballman]

In D84924#2421116, @njames93 wrote:

Fix documentation.

Ping??

Ah, sorry, this fell to the bottom of my review list because the title still says [WIP] and so I thought there was more work being done on it. You may want to remove that bit from the title.

In D84924#2184132, @njames93 wrote:

This is very much a work in progress
Another direction I was thinking was only apply the fixes found in notes if there is exactly one fix attached to the notes in a diagnostic, instead of just applying the first one we find

I was wondering the same thing here myself. If there's exactly one fix, then it's unambiguous as to what behavior you get. One (minor) concern I have about the current approach is with nondeterminism in diagnostic ordering where different runs may pick different fixes for the same code. I don't *think* we have any instances of this in Clang or clang-tidy, but users can add their own plugins (for instance to the clang static analyzer) that may introduce some small risk there. Do you have a reason why you picked one approach over the other?

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

103

I think you should add some comments explaining the difference between Fix and AnyFix because it's not super clear. Alternatively, if these are strongly related, should this be an enumeration rather than two boolean parameters? e.g., enum FixitBehavior { NoFixes, ApplyNonNoteFixes, ApplyAllFixes }; or some such (note, this suggestion may or may not make sense, I'm still reviewing).

clang-tools-extra/test/clang-tidy/checkers/misc-definitions-in-headers.hpp

1

Hmm, should that be --fix-notes? (Note the number of dashes, which differs from the documentation)

clang-tools-extra/test/clang-tidy/infrastructure/alternative-fixes.cpp

1

Same question here as above.

[njames93]

Use enum for handling fix behaviour.
Fix tests using --fix-notes instead of -fix-notes.

[njames93]

In D84924#2446075, @aaron.ballman wrote:

In D84924#2184132, @njames93 wrote:

This is very much a work in progress
Another direction I was thinking was only apply the fixes found in notes if there is exactly one fix attached to the notes in a diagnostic, instead of just applying the first one we find

I was wondering the same thing here myself. If there's exactly one fix, then it's unambiguous as to what behavior you get. One (minor) concern I have about the current approach is with nondeterminism in diagnostic ordering where different runs may pick different fixes for the same code. I don't *think* we have any instances of this in Clang or clang-tidy, but users can add their own plugins (for instance to the clang static analyzer) that may introduce some small risk there. Do you have a reason why you picked one approach over the other?

Part of the reason for this approach is from this bug report https://bugs.llvm.org/show_bug.cgi?id=47971, where its pointed out that clang diags gets fix-its added in notes if the fixes will change behaviour or they aren't sure its going to actually fix the issue.
As clang-tidy also applies fixes reported from clang, it is wise to adhere to a similar level caution.

[aaron.ballman]

In D84924#2465745, @njames93 wrote:

In D84924#2446075, @aaron.ballman wrote:

In D84924#2184132, @njames93 wrote:

This is very much a work in progress
Another direction I was thinking was only apply the fixes found in notes if there is exactly one fix attached to the notes in a diagnostic, instead of just applying the first one we find

I was wondering the same thing here myself. If there's exactly one fix, then it's unambiguous as to what behavior you get. One (minor) concern I have about the current approach is with nondeterminism in diagnostic ordering where different runs may pick different fixes for the same code. I don't *think* we have any instances of this in Clang or clang-tidy, but users can add their own plugins (for instance to the clang static analyzer) that may introduce some small risk there. Do you have a reason why you picked one approach over the other?

Part of the reason for this approach is from this bug report https://bugs.llvm.org/show_bug.cgi?id=47971, where its pointed out that clang diags gets fix-its added in notes if the fixes will change behaviour or they aren't sure its going to actually fix the issue.
As clang-tidy also applies fixes reported from clang, it is wise to adhere to a similar level caution.

Agreed, but I don't think the current behavior in this patch is cautious enough. I like that we have to specify "please apply fixits from notes" explicitly. I think that's the right behavior we want. But if there are multiple fixits attached to the notes for a diagnostic, I don't think we should assume that the first fix-it is the correct one to apply -- I think the user should have to manually pick the variant they want in that case. So I think the behavior should be to apply the fixits from notes if there's only a single fixit to be applied. WDYT?

[njames93]

In D84924#2466564, @aaron.ballman wrote:

Agreed, but I don't think the current behavior in this patch is cautious enough. I like that we have to specify "please apply fixits from notes" explicitly. I think that's the right behavior we want. But if there are multiple fixits attached to the notes for a diagnostic, I don't think we should assume that the first fix-it is the correct one to apply -- I think the user should have to manually pick the variant they want in that case. So I think the behavior should be to apply the fixits from notes if there's only a single fixit to be applied. WDYT?

I can agree with that, One thing I like about using clangd is it will show you all available fixes, letting you pick and choose.

[njames93]

Only apply fixes from notes if there is exactly one fix found in them.

[aaron.ballman]

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

231

Comment seems stale.

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

186 ↗

(On Diff #313272)

Should this change be a separate patch?

clang-tools-extra/clang-tidy/readability/InconsistentDeclarationParameterNameCheck.cpp

222 ↗

(On Diff #313272)

Same for these changes?

clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp

501

Might as well fix this clang-format warning.

clang-tools-extra/docs/clang-tidy/index.rst

176

This sort of reads like it may apply fixes from multiple notes. How about: If a warning has no fix, but a single fix can be found through an associated diagnostic note, apply the fix. Specifying this flag will implicitly enable the '--fix' flag.

(Note, I also quietly switched from using `` to using '' for quoting -- that seems to be the consistent style here.)

[njames93]

Rebased and address comments

[njames93]

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

186 ↗

(On Diff #313272)

It's either modify the check to put the fix in the warning, or modify the test so that fix notes is specified.

clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp

501

I think this is a discrepancy with the clang format in using (truck) and the pre merge. Mine isn't complaining about format

[aaron.ballman]

LGTM!

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

 /// \param StoreCheckProfile If provided, and EnableCheckProfile is true,
 /// the profile will not be output to stderr, but will instead be stored
 /// as a JSON file in the specified directory.
 std::vector<ClangTidyError>
 runClangTidy(clang::tidy::ClangTidyContext &Context,
              const tooling::CompilationDatabase &Compilations,
              ArrayRef<std::string> InputFiles,
              llvm::IntrusiveRefCntPtr<llvm::vfs::OverlayFileSystem> BaseFS,
-             bool EnableCheckProfile = false,
+             bool ApplyAnyFix, bool EnableCheckProfile = false,
              llvm::StringRef StoreCheckProfile = StringRef());
 
+/// Controls what kind of fixes clang-tidy is allowed to apply.
+enum FixBehaviour {
+  /// Don't try to apply any fix.
+  FB_NoFix,
+  /// Only apply fixes added to warnings.
+  FB_Fix,
+  /// Apply fixes found in notes.
+  FB_FixNotes
+};
+
 // FIXME: This interface will need to be significantly extended to be useful.
 // FIXME: Implement confidence levels for displaying/fixing errors.
 //
-/// Displays the found \p Errors to the users. If \p Fix is true, \p
-/// Errors containing fixes are automatically applied and reformatted. If no
-/// clang-format configuration file is found, the given \P FormatStyle is used.
+/// Displays the found \p Errors to the users. If \p Fix is \ref FB_Fix or \ref
+/// FB_FixNotes, \p Errors containing fixes are automatically applied and
+/// reformatted. If no clang-format configuration file is found, the given \P
+/// FormatStyle is used.
 void handleErrors(llvm::ArrayRef<ClangTidyError> Errors,
-                  ClangTidyContext &Context, bool Fix,
+                  ClangTidyContext &Context, FixBehaviour Fix,
                   unsigned &WarningsAsErrorsCount,
                   llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> BaseFS);
 
 /// Serializes replacements into YAML and writes them to the specified
 /// output stream.
 void exportReplacements(StringRef MainFilePath,
                         const std::vector<ClangTidyError> &Errors,
                         raw_ostream &OS);
 
 } // end namespace tidy
 } // end namespace clang
 
 #endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CLANGTIDY_H

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

 
 private:
   ClangTidyContext &Context;
 };
 #endif // CLANG_TIDY_ENABLE_STATIC_ANALYZER
 
 class ErrorReporter {
 public:
-  ErrorReporter(ClangTidyContext &Context, bool ApplyFixes,
+  ErrorReporter(ClangTidyContext &Context, FixBehaviour ApplyFixes,
                 llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> BaseFS)
       : Files(FileSystemOptions(), std::move(BaseFS)),
         DiagOpts(new DiagnosticOptions()),
         DiagPrinter(new TextDiagnosticPrinter(llvm::outs(), &*DiagOpts)),
         Diags(IntrusiveRefCntPtr<DiagnosticIDs>(new DiagnosticIDs), &*DiagOpts,
               DiagPrinter),
         SourceMgr(Diags, Files), Context(Context), ApplyFixes(ApplyFixes),
         TotalFixes(0), AppliedFixes(0), WarningsAsErrors(0) {
     DiagOpts->ShowColors = Context.getOptions().UseColor.getValueOr(
         llvm::sys::Process::StandardOutHasColors());
     DiagPrinter->BeginSourceFile(LangOpts);
     if (DiagOpts->ShowColors && !llvm::sys::Process::StandardOutIsDisplayed()) {
       llvm::sys::Process::UseANSIEscapeCodes(true);
     }
   }
 
SmallVector<std::pair<SourceLocation, bool>, 4> FixLocations;
       if (Error.IsWarningAsError) {
         Name += ",-warnings-as-errors";
         Level = DiagnosticsEngine::Error;
         WarningsAsErrors++;
       }
       auto Diag = Diags.Report(Loc, Diags.getCustomDiagID(Level, "%0 [%1]"))
                   << Message.Message << Name;
       // FIXME: explore options to support interactive fix selection.
-      const llvm::StringMap<Replacements> *ChosenFix = selectFirstFix(Error);
-      if (ApplyFixes && ChosenFix) {
+      const llvm::StringMap<Replacements> *ChosenFix;
+      if (ApplyFixes != FB_NoFix &&
+          (ChosenFix = getFixIt(Error, ApplyFixes == FB_FixNotes))) {
         for (const auto &FileAndReplacements : *ChosenFix) {
           for (const auto &Repl : FileAndReplacements.second) {
             ++TotalFixes;
             bool CanBeApplied = false;
             if (!Repl.isApplicable())
               continue;
             SourceLocation FixLoc;
             SmallString<128> FixAbsoluteFilePath = Repl.getFilePath();
for (auto Fix : FixLocations) {
       Diags.Report(Fix.first, Fix.second ? diag::note_fixit_applied
                                          : diag::note_fixit_failed);
     }
     for (const auto &Note : Error.Notes)
       reportNote(Note);
   }
 
   void finish() {
-    if (ApplyFixes && TotalFixes > 0) {
+    if (TotalFixes > 0) {
       Rewriter Rewrite(SourceMgr, LangOpts);
       for (const auto &FileAndReplacements : FileReplacements) {
         StringRef File = FileAndReplacements.first();
         llvm::ErrorOr<std::unique_ptr<MemoryBuffer>> Buffer =
             SourceMgr.getFileManager().getBufferForFile(File);
         if (!Buffer) {
           llvm::errs() << "Can't get buffer for file " << File << ": "
                        << Buffer.getError().message() << "\n";
private:
   FileManager Files;
   LangOptions LangOpts; // FIXME: use langopts from each original file
   IntrusiveRefCntPtr<DiagnosticOptions> DiagOpts;
   DiagnosticConsumer *DiagPrinter;
   DiagnosticsEngine Diags;
   SourceManager SourceMgr;
   llvm::StringMap<Replacements> FileReplacements;
   ClangTidyContext &Context;
-  bool ApplyFixes;
+  FixBehaviour ApplyFixes;
   unsigned TotalFixes;
   unsigned AppliedFixes;
   unsigned WarningsAsErrors;
 };
 
 class ClangTidyASTConsumer : public MultiplexConsumer {
 public:
   ClangTidyASTConsumer(std::vector<std::unique_ptr<ASTConsumer>> Consumers,
getCheckOptions(const ClangTidyOptions &Options,
   return Factory.getCheckOptions();
 }
 
 std::vector<ClangTidyError>
 runClangTidy(clang::tidy::ClangTidyContext &Context,
              const CompilationDatabase &Compilations,
              ArrayRef<std::string> InputFiles,
              llvm::IntrusiveRefCntPtr<llvm::vfs::OverlayFileSystem> BaseFS,
-             bool EnableCheckProfile, llvm::StringRef StoreCheckProfile) {
+             bool ApplyAnyFix, bool EnableCheckProfile,
+             llvm::StringRef StoreCheckProfile) {
   ClangTool Tool(Compilations, InputFiles,
                  std::make_shared<PCHContainerOperations>(), BaseFS);
 
   // Add extra arguments passed by the clang-tidy command-line.
   ArgumentsAdjuster PerFileExtraArgumentsInserter =
       [&Context](const CommandLineArguments &Args, StringRef Filename) {
         ClangTidyOptions Opts = Context.getOptionsForFile(Filename);
         CommandLineArguments AdjustedArgs = Args;
ArgumentsAdjuster PerFileExtraArgumentsInserter =
         return AdjustedArgs;
       };
 
   Tool.appendArgumentsAdjuster(PerFileExtraArgumentsInserter);
   Tool.appendArgumentsAdjuster(getStripPluginsAdjuster());
   Context.setEnableProfiling(EnableCheckProfile);
   Context.setProfileStoragePrefix(StoreCheckProfile);
 
-  ClangTidyDiagnosticConsumer DiagConsumer(Context);
+  ClangTidyDiagnosticConsumer DiagConsumer(Context, nullptr, true, ApplyAnyFix);
   DiagnosticsEngine DE(new DiagnosticIDs(), new DiagnosticOptions(),
                        &DiagConsumer, /*ShouldOwnClient=*/false);
   Context.setDiagnosticsEngine(&DE);
   Tool.setDiagnosticConsumer(&DiagConsumer);
 
   class ActionFactory : public FrontendActionFactory {
   public:
     ActionFactory(ClangTidyContext &Context,
runClangTidy(clang::tidy::ClangTidyContext &Context,
   };
 
   ActionFactory Factory(Context, std::move(BaseFS));
   Tool.run(&Factory);
   return DiagConsumer.take();
 }
 
 void handleErrors(llvm::ArrayRef<ClangTidyError> Errors,
-                  ClangTidyContext &Context, bool Fix,
+                  ClangTidyContext &Context, FixBehaviour Fix,
                   unsigned &WarningsAsErrorsCount,
                   llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> BaseFS) {
   ErrorReporter Reporter(Context, Fix, std::move(BaseFS));
   llvm::vfs::FileSystem &FileSystem =
       Reporter.getSourceManager().getFileManager().getVirtualFileSystem();
   auto InitialWorkingDir = FileSystem.getCurrentWorkingDirectory();
   if (!InitialWorkingDir)
     llvm::report_fatal_error("Cannot get current working path.");

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

 /// to this function.
 /// If `AllowIO` is false, the function does not attempt to read source files
 /// from disk which are not already mapped into memory; such files are treated
 /// as not containing a suppression comment.
 bool shouldSuppressDiagnostic(DiagnosticsEngine::Level DiagLevel,
                               const Diagnostic &Info, ClangTidyContext &Context,
                               bool AllowIO = true);
 
+/// 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> *
+getFixIt(const tooling::Diagnostic &Diagnostic, bool AnyFix);
+
 /// A diagnostic consumer that turns each \c Diagnostic into a
 /// \c SourceManager-independent \c ClangTidyError.
 //
 // FIXME: If we move away from unit-tests, this can be moved to a private
 // implementation file.
 class ClangTidyDiagnosticConsumer : public DiagnosticConsumer {
 public:
   ClangTidyDiagnosticConsumer(ClangTidyContext &Ctx,
                               DiagnosticsEngine *ExternalDiagEngine = nullptr,
-                              bool RemoveIncompatibleErrors = true);
+                              bool RemoveIncompatibleErrors = true,
+                              bool GetFixesFromNotes = false);
 
   // FIXME: The concept of converting between FixItHints and Replacements is
   // more generic and should be pulled out into a more useful Diagnostics
   // library.
   void HandleDiagnostic(DiagnosticsEngine::Level DiagLevel,
                         const Diagnostic &Info) override;
 
   // Retrieve the diagnostics that were captured.
private:
   void checkFilters(SourceLocation Location, const SourceManager &Sources);
   bool passesLineFilter(StringRef FileName, unsigned LineNumber) const;
 
   void forwardDiagnostic(const Diagnostic &Info);
 
   ClangTidyContext &Context;
   DiagnosticsEngine *ExternalDiagEngine;
   bool RemoveIncompatibleErrors;
+  bool GetFixesFromNotes;
   std::vector<ClangTidyError> Errors;
   std::unique_ptr<llvm::Regex> HeaderFilter;
   bool LastErrorRelatesToUserCode;
   bool LastErrorPassesLineFilter;
   bool LastErrorWasIgnored;
 };
 
 } // end namespace tidy
 } // end namespace clang
 
 #endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CLANGTIDYDIAGNOSTICCONSUMER_H

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

llvm::DenseMap<unsigned, std::string>::const_iterator I =
       CheckNamesByDiagnosticID.find(DiagnosticID);
   if (I != CheckNamesByDiagnosticID.end())
     return I->second;
   return "";
 }
 
 ClangTidyDiagnosticConsumer::ClangTidyDiagnosticConsumer(
     ClangTidyContext &Ctx, DiagnosticsEngine *ExternalDiagEngine,
-    bool RemoveIncompatibleErrors)
+    bool RemoveIncompatibleErrors, bool GetFixesFromNotes)
     : Context(Ctx), ExternalDiagEngine(ExternalDiagEngine),
       RemoveIncompatibleErrors(RemoveIncompatibleErrors),
-      LastErrorRelatesToUserCode(false), LastErrorPassesLineFilter(false),
-      LastErrorWasIgnored(false) {}
+      GetFixesFromNotes(GetFixesFromNotes), LastErrorRelatesToUserCode(false),
+      LastErrorPassesLineFilter(false), LastErrorWasIgnored(false) {}
 
 void ClangTidyDiagnosticConsumer::finalizeLastError() {
   if (!Errors.empty()) {
     ClangTidyError &Error = Errors.back();
     if (Error.DiagnosticName == "clang-tidy-config") {
       // Never ignore these.
     } else if (!Context.isCheckEnabled(Error.DiagnosticName) &&
                Error.DiagLevel != ClangTidyError::Error) {
bool shouldSuppressDiagnostic(DiagnosticsEngine::Level DiagLevel,
   return Info.getLocation().isValid() &&
          DiagLevel != DiagnosticsEngine::Error &&
          DiagLevel != DiagnosticsEngine::Fatal &&
          LineIsMarkedWithNOLINTinMacro(Info.getSourceManager(),
                                        Info.getLocation(), Info.getID(),
                                        Context, AllowIO);
 }
 
+const llvm::StringMap<tooling::Replacements> *
+getFixIt(const tooling::Diagnostic &Diagnostic, bool GetFixFromNotes) {
+  if (!Diagnostic.Message.Fix.empty())
+    return &Diagnostic.Message.Fix;
+  if (!GetFixFromNotes)
+    return nullptr;
+  const llvm::StringMap<tooling::Replacements> *Result = nullptr;
+  for (const auto &Note : Diagnostic.Notes) {
+    if (!Note.Fix.empty()) {
+      if (Result)
+        // We have 2 different fixes in notes, bail out.
+        return nullptr;
+      Result = &Note.Fix;
+    }
+  }
+  return Result;
+}
+
 } // namespace tidy
 } // namespace clang
 
 void ClangTidyDiagnosticConsumer::HandleDiagnostic(
     DiagnosticsEngine::Level DiagLevel, const Diagnostic &Info) {
   if (LastErrorWasIgnored && DiagLevel == DiagnosticsEngine::Note)
     return;
 
void ClangTidyDiagnosticConsumer::removeIncompatibleErrors() {
   removeDuplicatedDiagnosticsOfAliasCheckers();
 
   // Compute error sizes.
   std::vector<int> Sizes;
   std::vector<
       std::pair<ClangTidyError *, llvm::StringMap<tooling::Replacements> *>>
       ErrorFixes;
   for (auto &Error : Errors) {
-    if (const auto *Fix = tooling::selectFirstFix(Error))
+    if (const auto *Fix = getFixIt(Error, GetFixesFromNotes))
       ErrorFixes.emplace_back(
           &Error, const_cast<llvm::StringMap<tooling::Replacements> *>(Fix));
   }
   for (const auto &ErrorAndFix : ErrorFixes) {
     int Size = 0;
     for (const auto &FileAndReplaces : *ErrorAndFix.second) {
       for (const auto &Replace : FileAndReplaces.second)
         Size += Replace.getLength();

clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp

 static cl::opt<bool> FixErrors("fix-errors", cl::desc(R"(
 Apply suggested fixes even if compilation
 errors were found. If compiler errors have
 attached fix-its, clang-tidy will apply them as
 well.
 )"),
                                cl::init(false), cl::cat(ClangTidyCategory));
 
+static cl::opt<bool> FixNotes("fix-notes", cl::desc(R"(
+If a warning has no fix, but a single fix can 
+be found through an associated diagnostic note, 
+apply the fix. 
+Specifying this flag will implicitly enable the 
+'--fix' flag.
+)"),
+                              cl::init(false), cl::cat(ClangTidyCategory));
+
 static cl::opt<std::string> FormatStyle("format-style", cl::desc(R"(
 Style for formatting code around applied fixes:
   - 'none' (default) turns off formatting
   - 'file' (literally 'file', not a placeholder)
     uses .clang-format file in the closest parent
     directory
   - '{ <json> }' specifies options inline, e.g.
     -format-style='{BasedOnStyle: llvm, IndentWidth: 8}'
int clangTidyMain(int argc, const char **argv) {
   llvm::InitializeAllTargetInfos();
   llvm::InitializeAllTargetMCs();
   llvm::InitializeAllAsmParsers();
 
   ClangTidyContext Context(std::move(OwningOptionsProvider),
                            AllowEnablingAnalyzerAlphaCheckers);
   std::vector<ClangTidyError> Errors =
       runClangTidy(Context, OptionsParser->getCompilations(), PathList, BaseFS,
-                   EnableCheckProfile, ProfilePrefix);
+                   FixNotes, EnableCheckProfile, ProfilePrefix);
   bool FoundErrors = llvm::find_if(Errors, [](const ClangTidyError &E) {
                        return E.DiagLevel == ClangTidyError::Error;
                      }) != Errors.end();
 
-  const bool DisableFixes = Fix && FoundErrors && !FixErrors;
+  // --fix-errors and --fix-notes imply --fix.
+  FixBehaviour Behaviour = FixNotes             ? FB_FixNotes
+                           : (Fix || FixErrors) ? FB_Fix
+                                                : FB_NoFix;
+
+  const bool DisableFixes = FoundErrors && !FixErrors;
 
   unsigned WErrorCount = 0;
 
-  // -fix-errors implies -fix.
-  handleErrors(Errors, Context, (FixErrors || Fix) && !DisableFixes, WErrorCount,
-               BaseFS);
+  handleErrors(Errors, Context, DisableFixes ? FB_NoFix : Behaviour,
+               WErrorCount, BaseFS);
 
   if (!ExportFixes.empty() && !Errors.empty()) {
     std::error_code EC;
     llvm::raw_fd_ostream OS(ExportFixes, EC, llvm::sys::fs::OF_None);
     if (EC) {
       llvm::errs() << "Error opening output file: " << EC.message() << '\n';
       return 1;
     }
     exportReplacements(FilePath.str(), Errors, OS);
   }
 
   if (!Quiet) {
     printStats(Context.getStats());
-    if (DisableFixes)
+    if (DisableFixes && Behaviour != FB_NoFix)
       llvm::errs()
           << "Found compiler errors, but -fix-errors was not specified.\n"
              "Fixes have NOT been applied.\n\n";
   }
 
   if (WErrorCount) {
     if (!Quiet) {
       StringRef Plural = WErrorCount == 1 ? "" : "s";

clang-tools-extra/docs/ReleaseNotes.rst

 The improvements are...
 
 Improvements to clang-tidy
 --------------------------
 
 - The `run-clang-tidy.py` helper script is now installed in `bin/` as
   `run-clang-tidy`. It was previously installed in `share/clang/`.
 
+- Added command line option `--fix-notes` to apply fixes found in notes
+  attached to warnings. These are typically cases where we are less confident
+  the fix will have the desired effect.
+
 New checks
 ^^^^^^^^^^
 
 - New :doc:`concurrency-thread-canceltype-asynchronous
   <clang-tidy/checks/concurrency-thread-canceltype-asynchronous>` check.
 
   Finds ``pthread_setcanceltype`` function calls where a thread's cancellation
   type is set to asynchronous.

clang-tools-extra/docs/clang-tidy/index.rst

--fix                          -
                                      Apply suggested fixes. Without -fix-errors
                                      clang-tidy will bail out if any compilation
                                      errors were found.
     --fix-errors                   -
                                      Apply suggested fixes even if compilation
                                      errors were found. If compiler errors have
                                      attached fix-its, clang-tidy will apply them as
                                      well.
+    --fix-notes                    - 
+                                     If a warning has no fix, but a single fix can 
+                                     be found through an associated diagnostic note, 
+                                     apply the fix. 
+                                     Specifying this flag will implicitly enable the 
+                                     '--fix' flag.
     --format-style=<string>        -
                                      Style for formatting code around applied fixes:
                                        - 'none' (default) turns off formatting
                                        - 'file' (literally 'file', not a placeholder)
                                          uses .clang-format file in the closest parent
                                          directory
                                        - '{ <json> }' specifies options inline, e.g.
                                          -format-style='{BasedOnStyle: llvm, IndentWidth: 8}'

clang-tools-extra/test/clang-tidy/checkers/misc-definitions-in-headers.hpp

-// RUN: %check_clang_tidy %s misc-definitions-in-headers %t
+// RUN: %check_clang_tidy %s misc-definitions-in-headers %t -- --fix-notes
 
 int f() {
 // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: function 'f' defined in a header file; function definitions in header files can lead to ODR violations [misc-definitions-in-headers]
 // CHECK-MESSAGES: :[[@LINE-2]]:5: note: make as 'inline'
 // CHECK-FIXES: inline int f() {
   return 1;
 }
 

clang-tools-extra/test/clang-tidy/checkers/misc-unused-using-decls-cxx17.cpp

-// RUN: %check_clang_tidy -std=c++17-or-later %s misc-unused-using-decls %t -- -- -fno-delayed-template-parsing -isystem %S/Inputs/
+// RUN: %check_clang_tidy -std=c++17-or-later %s misc-unused-using-decls %t -- --fix-notes -- -fno-delayed-template-parsing -isystem %S/Inputs/
 
 namespace ns {
 
 template <typename T> class Foo {
 public:
   Foo(T);
 };
 // Deduction guide (CTAD)

clang-tools-extra/test/clang-tidy/checkers/misc-unused-using-decls.cpp

-// RUN: %check_clang_tidy %s misc-unused-using-decls %t -- -- -fno-delayed-template-parsing -isystem %S/Inputs/
-
+// RUN: %check_clang_tidy %s misc-unused-using-decls %t -- --fix-notes -- -fno-delayed-template-parsing -isystem %S/Inputs/
 
 // ----- Definitions -----
 template <typename T> class vector {};
 namespace n {
 class A;
 class B;
 class C;
 class D;

clang-tools-extra/test/clang-tidy/checkers/readability-inconsistent-declaration-parameter-name.cpp

-// RUN: %check_clang_tidy %s readability-inconsistent-declaration-parameter-name %t -- -- -fno-delayed-template-parsing
+// RUN: %check_clang_tidy %s readability-inconsistent-declaration-parameter-name %t -- --fix-notes -- -fno-delayed-template-parsing
 
 void consistentFunction(int a, int b, int c);
 void consistentFunction(int a, int b, int c);
 void consistentFunction(int prefixA, int b, int cSuffix);
 void consistentFunction(int a, int b, int c);
 void consistentFunction(int a, int b, int /*c*/);
 void consistentFunction(int /*c*/, int /*c*/, int /*c*/);
 

clang-tools-extra/test/clang-tidy/infrastructure/alternative-fixes.cpp

-// RUN: %check_clang_tidy %s "llvm-namespace-comment,clang-diagnostic-*" %t
+// RUN: %check_clang_tidy %s "llvm-namespace-comment,clang-diagnostic-*" %t -- --fix-notes
 void foo(int a) {
   if (a = 1) {
     // CHECK-NOTES: [[@LINE-1]]:9: warning: using the result of an assignment as a condition without parentheses [clang-diagnostic-parentheses]
     // CHECK-NOTES: [[@LINE-2]]:9: note: place parentheses around the assignment to silence this warning
     // CHECK-NOTES: [[@LINE-3]]:9: note: use '==' to turn this assignment into an equality comparison
-  // CHECK-FIXES: if ((a = 1)) {
+    // As we have 2 conflicting fixes in notes, don't apply any fix.
+    // CHECK-FIXES: if (a = 1) {
   }
 }

clang-tools-extra/test/clang-tidy/infrastructure/clean-up-code.cpp

-// RUN: %check_clang_tidy %s misc-unused-using-decls %t
-// RUN: %check_clang_tidy %s misc-unused-using-decls %t -- -format-style=none --
-// RUN: %check_clang_tidy %s misc-unused-using-decls %t -- -format-style=llvm --
+// RUN: %check_clang_tidy %s misc-unused-using-decls %t -- --fix-notes
+// RUN: %check_clang_tidy %s misc-unused-using-decls %t -- --fix-notes -format-style=none --
+// RUN: %check_clang_tidy %s misc-unused-using-decls %t -- --fix-notes -format-style=llvm --
 namespace a { class A {}; }
 namespace b {
 using a::A;
 }
 namespace c {}
 // CHECK-MESSAGES: :[[@LINE-3]]:10: warning: using decl 'A' is unused [misc-unused-using-decls]
 // CHECK-FIXES: {{^namespace a { class A {}; }$}}
 // CHECK-FIXES-NOT: namespace
 // CHECK-FIXES: {{^namespace c {}$}}
 // FIXME: cleanupAroundReplacements leaves whitespace. Otherwise we could just
 // check the next line.