llvm · chomosuke · Nov 2, 2024
diff --git a/clang-tools-extra/clangd/CMakeLists.txt b/clang-tools-extra/clangd/CMakeLists.txt
@@ -98,6 +98,7 @@ add_clang_library(clangDaemon STATIC
   InlayHints.cpp
   JSONTransport.cpp
   ModulesBuilder.cpp
+  NoLintFixes.cpp
   PathMapping.cpp
   Protocol.cpp
   Quality.cpp

diff --git a/clang-tools-extra/clangd/ClangdServer.cpp b/clang-tools-extra/clangd/ClangdServer.cpp
@@ -15,6 +15,7 @@
 #include "Format.h"
 #include "HeaderSourceSwitch.h"
 #include "InlayHints.h"
+#include "NoLintFixes.h"
 #include "ParsedAST.h"
 #include "Preamble.h"
 #include "Protocol.h"
@@ -661,7 +662,7 @@ tryConvertToRename(const Diag *Diag, const Fix &Fix) {
   bool IsClangTidyRename = Diag->Source == Diag::ClangTidy &&
                            Diag->Name == "readability-identifier-naming" &&
                            !Fix.Edits.empty();
-  if (IsClangTidyRename && Diag->InsideMainFile) {
+  if (IsClangTidyRename && !isNoLintFixes(Fix) && Diag->InsideMainFile) {
     ClangdServer::CodeActionResult::Rename R;
     R.NewName = Fix.Edits.front().newText;
     R.FixMessage = Fix.Message;

diff --git a/clang-tools-extra/clangd/Diagnostics.cpp b/clang-tools-extra/clangd/Diagnostics.cpp
@@ -9,6 +9,7 @@
 #include "Diagnostics.h"
 #include "../clang-tidy/ClangTidyDiagnosticConsumer.h"
 #include "Compiler.h"
+#include "NoLintFixes.h"
 #include "Protocol.h"
 #include "SourceCode.h"
 #include "support/Logger.h"
@@ -311,8 +312,18 @@ std::string mainMessage(const Diag &D, const ClangdDiagnosticOptions &Opts) {
   std::string Result;
   llvm::raw_string_ostream OS(Result);
   OS << D.Message;
-  if (Opts.DisplayFixesCount && !D.Fixes.empty())
-    OS << " (" << (D.Fixes.size() > 1 ? "fixes" : "fix") << " available)";
+
+  // NOLINT fixes are somewhat not real fixes and to say "(fix available)" when
+  // the fixes is just to suppress could be misleading.
+  int RealFixCount = D.Fixes.size();
+  for (auto const &Fix : D.Fixes) {
+    if (isNoLintFixes(Fix)) {
+      RealFixCount--;
+    }
+  }
+
+  if (Opts.DisplayFixesCount && RealFixCount > 0)
+    OS << " (" << (RealFixCount > 1 ? "fixes" : "fix") << " available)";
   // If notes aren't emitted as structured info, add them to the message.
   if (!Opts.EmitRelatedLocations)
     for (auto &Note : D.Notes) {
@@ -822,8 +833,8 @@ void StoreDiags::HandleDiagnostic(DiagnosticsEngine::Level DiagLevel,
     LastDiagOriginallyError = OriginallyError;
     if (!Info.getFixItHints().empty())
       AddFix(true /* try to invent a message instead of repeating the diag */);
-    if (Fixer) {
-      auto ExtraFixes = Fixer(LastDiag->Severity, Info);
+    if (MainFixer) {
+      auto ExtraFixes = MainFixer(*LastDiag, Info);
       LastDiag->Fixes.insert(LastDiag->Fixes.end(), ExtraFixes.begin(),
                              ExtraFixes.end());
     }
@@ -841,8 +852,8 @@ void StoreDiags::HandleDiagnostic(DiagnosticsEngine::Level DiagLevel,
       return;
 
     // Give include-fixer a chance to replace a note with a fix.
-    if (Fixer) {
-      auto ReplacementFixes = Fixer(LastDiag->Severity, Info);
+    if (NoteFixer) {
+      auto ReplacementFixes = NoteFixer(*LastDiag, Info);
       if (!ReplacementFixes.empty()) {
         assert(Info.getNumFixItHints() == 0 &&
                "Include-fixer replaced a note with clang fix-its attached!");

diff --git a/clang-tools-extra/clangd/Diagnostics.h b/clang-tools-extra/clangd/Diagnostics.h
@@ -147,15 +147,18 @@ class StoreDiags : public DiagnosticConsumer {
                         const clang::Diagnostic &Info) override;
 
   /// When passed a main diagnostic, returns fixes to add to it.
+  using MainDiagFixer =
+      std::function<std::vector<Fix>(const Diag &, const clang::Diagnostic &)>;
   /// When passed a note diagnostic, returns fixes to replace it with.
-  using DiagFixer = std::function<std::vector<Fix>(DiagnosticsEngine::Level,
-                                                   const clang::Diagnostic &)>;
+  using NoteDiagFixer =
+      std::function<std::vector<Fix>(const Diag &, const clang::Diagnostic &)>;
   using LevelAdjuster = std::function<DiagnosticsEngine::Level(
       DiagnosticsEngine::Level, const clang::Diagnostic &)>;
   using DiagCallback =
       std::function<void(const clang::Diagnostic &, clangd::Diag &)>;
   /// If set, possibly adds fixes for diagnostics using \p Fixer.
-  void contributeFixes(DiagFixer Fixer) { this->Fixer = Fixer; }
+  void contributeMainDiagFixes(MainDiagFixer Fixer) { this->MainFixer = Fixer; }
+  void contributeNoteDiagFixes(NoteDiagFixer Fixer) { this->NoteFixer = Fixer; }
   /// If set, this allows the client of this class to adjust the level of
   /// diagnostics, such as promoting warnings to errors, or ignoring
   /// diagnostics.
@@ -167,7 +170,8 @@ class StoreDiags : public DiagnosticConsumer {
 private:
   void flushLastDiag();
 
-  DiagFixer Fixer = nullptr;
+  MainDiagFixer MainFixer = nullptr;
+  NoteDiagFixer NoteFixer = nullptr;
   LevelAdjuster Adjuster = nullptr;
   DiagCallback DiagCB = nullptr;
   std::vector<Diag> Output;

diff --git a/clang-tools-extra/clangd/NoLintFixes.cpp b/clang-tools-extra/clangd/NoLintFixes.cpp
@@ -0,0 +1,99 @@
+//===--- NoLintFixes.cpp -------------------------------------------*-
+// C++-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "NoLintFixes.h"
+#include "../clang-tidy/ClangTidyCheck.h"
+#include "../clang-tidy/ClangTidyDiagnosticConsumer.h"
+#include "../clang-tidy/ClangTidyModule.h"
+#include "AST.h"
+#include "Diagnostics.h"
+#include "FeatureModule.h"
+#include "SourceCode.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/Basic/Diagnostic.h"
+#include "clang/Basic/DiagnosticIDs.h"
+#include "clang/Basic/LLVM.h"
+#include "clang/Basic/SourceLocation.h"
+#include "clang/Basic/SourceManager.h"
+#include "clang/Frontend/CompilerInstance.h"
+#include "clang/Frontend/CompilerInvocation.h"
+#include "clang/Lex/Lexer.h"
+#include "clang/Serialization/ASTWriter.h"
+#include "llvm/ADT/STLFunctionalExtras.h"
+#include "llvm/ADT/SmallVector.h"
+#include "llvm/ADT/StringRef.h"
+#include <cassert>
+#include <cctype>
+#include <optional>
+#include <regex>
+#include <string>
+#include <vector>
+
+namespace clang {
+namespace clangd {
+
+std::vector<Fix> noLintFixes(const clang::tidy::ClangTidyContext &CTContext,
+                             const clang::Diagnostic &Info, const Diag &Diag) {
+  auto RuleName = CTContext.getCheckName(Diag.ID);
+  if (
+      // If this isn't a clang-tidy diag
+      RuleName.empty() ||
+      // NOLINT does not work on Serverity Error or above
+      Diag.Severity >= DiagnosticsEngine::Error ||
+      // No point adding extra fixes if the Diag is for a different file
+      !Diag.InsideMainFile) {
+    return {};
+  }
+
+  auto &SrcMgr = Info.getSourceManager();
+  auto DiagLoc = SrcMgr.getSpellingLoc(Info.getLocation());
+
+  auto F = Fix{};
+  F.Message = llvm::formatv("ignore [{0}] for this line", RuleName);
+  auto &E = F.Edits.emplace_back();
+
+  auto File = SrcMgr.getFileID(DiagLoc);
+  auto FileLoc = SrcMgr.getLocForStartOfFile(File);
+  auto CodeTilDiag = toSourceCode(SrcMgr, SourceRange(FileLoc, DiagLoc));
+
+  auto StartCurLine = CodeTilDiag.find_last_of('\n') + 1;
+  auto CurLine = CodeTilDiag.substr(StartCurLine);
+  auto Indent = CurLine.take_while([](char C) { return std::isspace(C); });
+
+  auto ExistingNoLintNextLineInsertPos = std::optional<int>();
+  if (StartCurLine > 0) {
+    auto StartPrevLine = CodeTilDiag.find_last_of('\n', StartCurLine - 1) + 1;
+    auto PrevLine =
+        CodeTilDiag.substr(StartPrevLine, StartCurLine - StartPrevLine - 1);
+    auto NLPos = PrevLine.find("NOLINTNEXTLINE(");
+    if (NLPos != StringRef::npos) {
+      ExistingNoLintNextLineInsertPos =
+          std::make_optional(PrevLine.find(")", NLPos));
+    }
+  }
+
+  auto InsertPos = sourceLocToPosition(SrcMgr, DiagLoc);
+  if (ExistingNoLintNextLineInsertPos) {
+    E.newText = llvm::formatv(", {0}", RuleName);
+    InsertPos.line -= 1;
+    InsertPos.character = *ExistingNoLintNextLineInsertPos;
+  } else {
+    E.newText = llvm::formatv("{0}// NOLINTNEXTLINE({1})\n", Indent, RuleName);
+    InsertPos.character = 0;
+  }
+  E.range = {InsertPos, InsertPos};
+
+  return {F};
+}
+
+const auto Regex = std::regex(NoLintFixMsgRegexStr);
+bool isNoLintFixes(const Fix &F) { return std::regex_match(F.Message, Regex); }
+
+} // namespace clangd
+} // namespace clang
diff --git a/clang-tools-extra/clangd/NoLintFixes.h b/clang-tools-extra/clangd/NoLintFixes.h
@@ -0,0 +1,36 @@
+//===--- NoLintFixes.h ------------------------------------------*- C++-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_NOLINTFIXES_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_NOLINTFIXES_H
+
+#include "../clang-tidy/ClangTidyDiagnosticConsumer.h"
+#include "../clang-tidy/ClangTidyModule.h"
+#include "Diagnostics.h"
+#include "FeatureModule.h"
+#include "clang/Basic/Diagnostic.h"
+#include <cassert>
+#include <vector>
+
+namespace clang {
+namespace clangd {
+
+/// Suggesting to insert "\\ NOLINTNEXTLINE(...)" to suppress clang-tidy
+/// diagnostics.
+std::vector<Fix> noLintFixes(const clang::tidy::ClangTidyContext &CTContext,
+                             const clang::Diagnostic &Info, const Diag &Diag);
+
+const auto NoLintFixMsgRegexStr = std::string("ignore \\[.*\\] for this line");
+
+/// Check if a fix created by clangTidyNoLintFixes().
+bool isNoLintFixes(const Fix &F);
+
+} // namespace clangd
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANGD_NOLINTFIXES_H
diff --git a/clang-tools-extra/clangd/ParsedAST.cpp b/clang-tools-extra/clangd/ParsedAST.cpp
@@ -23,6 +23,7 @@
 #include "HeuristicResolver.h"
 #include "IncludeCleaner.h"
 #include "IncludeFixer.h"
+#include "NoLintFixes.h"
 #include "Preamble.h"
 #include "SourceCode.h"
 #include "TidyProvider.h"
@@ -655,10 +656,20 @@ ParsedAST::build(llvm::StringRef Filename, const ParseInputs &Inputs,
               : Symbol::Include;
       FixIncludes.emplace(Filename, Inserter, *Inputs.Index,
                           /*IndexRequestLimit=*/5, Directive);
-      ASTDiags.contributeFixes([&FixIncludes](DiagnosticsEngine::Level DiagLevl,
-                                              const clang::Diagnostic &Info) {
-        return FixIncludes->fix(DiagLevl, Info);
-      });
+      ASTDiags.contributeMainDiagFixes(
+          [&FixIncludes, &CTContext](const Diag &Diag,
+                                     const clang::Diagnostic &Info) {
+            auto Fixes = std::vector<Fix>();
+            auto NoLintFixes = noLintFixes(*CTContext, Info, Diag);
+            Fixes.insert(Fixes.end(), NoLintFixes.begin(), NoLintFixes.end());
+            auto IncludeFixes = FixIncludes->fix(Diag.Severity, Info);
+            Fixes.insert(Fixes.end(), IncludeFixes.begin(), IncludeFixes.end());
+            return Fixes;
+          });
+      ASTDiags.contributeNoteDiagFixes(
+          [&FixIncludes](const Diag &Diag, const clang::Diagnostic &Info) {
+            return FixIncludes->fix(Diag.Severity, Info);
+          });
       Clang->setExternalSemaSource(FixIncludes->unresolvedNameRecorder());
     }
   }

diff --git a/clang-tools-extra/clangd/test/fixits-codeaction-documentchanges.test b/clang-tools-extra/clangd/test/fixits-codeaction-documentchanges.test
@@ -54,6 +54,53 @@
 # CHECK-NEXT:          {
 # CHECK-NEXT:            "edits": [
 # CHECK-NEXT:              {
+# CHECK-NEXT:                "newText": "// NOLINTNEXTLINE(clang-diagnostic-parentheses)\n",
+# CHECK-NEXT:                "range": {
+# CHECK-NEXT:                  "end": {
+# CHECK-NEXT:                    "character": 0,
+# CHECK-NEXT:                    "line": 0
+# CHECK-NEXT:                  },
+# CHECK-NEXT:                  "start": {
+# CHECK-NEXT:                    "character": 0,
+# CHECK-NEXT:                    "line": 0
+# CHECK-NEXT:                  }
+# CHECK-NEXT:                }
+# CHECK-NEXT:              }
+# CHECK-NEXT:            ],
+# CHECK-NEXT:            "textDocument": {
+# CHECK-NEXT:              "uri": "file:///clangd-test/foo.c",
+# CHECK-NEXT:              "version": 1
+# CHECK-NEXT:            }
+# CHECK-NEXT:          }
+# CHECK-NEXT:        ]
+# CHECK-NEXT:      },
+# CHECK-NEXT:      "kind": "quickfix",
+# CHECK-NEXT:      "title": "ignore [clang-diagnostic-parentheses] for this line"
+# CHECK-NEXT:    },
+# CHECK-NEXT:    {
+# CHECK-NEXT:      "diagnostics": [
+# CHECK-NEXT:        {
+# CHECK-NEXT:          "code": "-Wparentheses",
+# CHECK-NEXT:          "message": "Using the result of an assignment as a condition without parentheses (fixes available)",
+# CHECK-NEXT:          "range": {
+# CHECK-NEXT:            "end": {
+# CHECK-NEXT:              "character": 37,
+# CHECK-NEXT:              "line": 0
+# CHECK-NEXT:            },
+# CHECK-NEXT:            "start": {
+# CHECK-NEXT:              "character": 32,
+# CHECK-NEXT:              "line": 0
+# CHECK-NEXT:            }
+# CHECK-NEXT:          },
+# CHECK-NEXT:          "severity": 2,
+# CHECK-NEXT:          "source": "clang"
+# CHECK-NEXT:        }
+# CHECK-NEXT:      ],
+# CHECK-NEXT:      "edit": {
+# CHECK-NEXT:        "documentChanges": [
+# CHECK-NEXT:          {
+# CHECK-NEXT:            "edits": [
+# CHECK-NEXT:              {
 # CHECK-NEXT:                "newText": "(",
 # CHECK-NEXT:                "range": {
 # CHECK-NEXT:                  "end": {

diff --git a/clang-tools-extra/clangd/test/fixits-codeaction.test b/clang-tools-extra/clangd/test/fixits-codeaction.test
@@ -51,6 +51,47 @@
 # CHECK-NEXT:      ],
 # CHECK-NEXT:      "edit": {
 # CHECK-NEXT:        "changes": {
+# CHECK-NEXT:          "file:///clangd-test/foo.c": [
+# CHECK-NEXT:            {
+# CHECK-NEXT:              "newText": "// NOLINTNEXTLINE(clang-diagnostic-parentheses)\n",
+# CHECK-NEXT:              "range": {
+# CHECK-NEXT:                "end": {
+# CHECK-NEXT:                  "character": 0,
+# CHECK-NEXT:                  "line": 0
+# CHECK-NEXT:                },
+# CHECK-NEXT:                "start": {
+# CHECK-NEXT:                  "character": 0,
+# CHECK-NEXT:                  "line": 0
+# CHECK-NEXT:                }
+# CHECK-NEXT:              }
+# CHECK-NEXT:            }
+# CHECK-NEXT:          ]
+# CHECK-NEXT:        }
+# CHECK-NEXT:      },
+# CHECK-NEXT:      "kind": "quickfix",
+# CHECK-NEXT:      "title": "ignore [clang-diagnostic-parentheses] for this line"
+# CHECK-NEXT:    },
+# CHECK-NEXT:    {
+# CHECK-NEXT:      "diagnostics": [
+# CHECK-NEXT:        {
+# CHECK-NEXT:          "code": "-Wparentheses",
+# CHECK-NEXT:          "message": "Using the result of an assignment as a condition without parentheses (fixes available)",
+# CHECK-NEXT:          "range": {
+# CHECK-NEXT:            "end": {
+# CHECK-NEXT:              "character": 37,
+# CHECK-NEXT:              "line": 0
+# CHECK-NEXT:            },
+# CHECK-NEXT:            "start": {
+# CHECK-NEXT:              "character": 32,
+# CHECK-NEXT:              "line": 0
+# CHECK-NEXT:            }
+# CHECK-NEXT:          },
+# CHECK-NEXT:          "severity": 2,
+# CHECK-NEXT:          "source": "clang"
+# CHECK-NEXT:        }
+# CHECK-NEXT:      ],
+# CHECK-NEXT:      "edit": {
+# CHECK-NEXT:        "changes": {
 # CHECK-NEXT:          "file://{{.*}}/foo.c": [
 # CHECK-NEXT:            {
 # CHECK-NEXT:              "newText": "(",