Skip to content

Conversation

@chomosuke
Copy link

@chomosuke chomosuke commented Nov 2, 2024

For some other lsp / linters. They will offer a "Ignore this error for this line" code actions for warnings. I find that convenient as I don't have to type the comment myself and copy the name of the diagnostics.

This PR implements this for clangd. For all diagnostics from clang-tidy, this PR adds an extra quick fix to insert a // NOLINTNEXTLINE(diag-name) to suppress the diagnostics.

This is the first time I work on such a large code base so any feedback and guidance are greatly appreciated.

@github-actions
Copy link

github-actions bot commented Nov 2, 2024

Thank you for submitting a Pull Request (PR) to the LLVM Project!

This PR will be automatically labeled and the relevant teams will be notified.

If you wish to, you can add reviewers by using the "Reviewers" section on this page.

If this is not working for you, it is probably because you do not have write permissions for the repository. In which case you can instead tag reviewers by name in a comment by using @ followed by their GitHub username.

If you have received no comments on your PR for a week, you can request a review by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate is once a week. Please remember that you are asking for valuable time from other developers.

If you have further questions, they may be answered by the LLVM GitHub User Guide.

You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums.

@llvmbot
Copy link
Member

llvmbot commented Nov 2, 2024

@llvm/pr-subscribers-clang-tools-extra

Author: Richard Li (chomosuke)

Changes

For some other lsp / linters. They will offer a "Ignore this error for this line" code actions for warnings. I find that convenient as I don't have to type the comment myself and copy the name of the diagnostics.

This is the first time I work on such a large code base so any feedback and guidance are greatly appreciated.


Patch is 32.58 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/114660.diff

13 Files Affected:

  • (modified) clang-tools-extra/clangd/CMakeLists.txt (+1)
  • (modified) clang-tools-extra/clangd/ClangdServer.cpp (+4-3)
  • (modified) clang-tools-extra/clangd/Diagnostics.cpp (+18-8)
  • (modified) clang-tools-extra/clangd/Diagnostics.h (+8-4)
  • (added) clang-tools-extra/clangd/NoLintFixes.cpp (+99)
  • (added) clang-tools-extra/clangd/NoLintFixes.h (+36)
  • (modified) clang-tools-extra/clangd/ParsedAST.cpp (+16-4)
  • (modified) clang-tools-extra/clangd/test/fixits-codeaction-documentchanges.test (+47)
  • (modified) clang-tools-extra/clangd/test/fixits-codeaction.test (+41)
  • (modified) clang-tools-extra/clangd/test/fixits-command-documentchanges.test (+62)
  • (modified) clang-tools-extra/clangd/test/fixits-command.test (+50)
  • (modified) clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp (+13-3)
  • (modified) clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp (+102-15)
diff --git a/clang-tools-extra/clangd/CMakeLists.txt b/clang-tools-extra/clangd/CMakeLists.txt
index d797ddce8c44d1..2a6433a5c3effd 100644
--- 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
index 9b38be04e7ddd7..925a015ae341bc 100644
--- 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"
@@ -62,8 +63,8 @@ namespace clangd {
 namespace {
 
 // Tracks number of times a tweak has been offered.
-static constexpr trace::Metric TweakAvailable(
-    "tweak_available", trace::Metric::Counter, "tweak_id");
+static constexpr trace::Metric
+    TweakAvailable("tweak_available", trace::Metric::Counter, "tweak_id");
 
 // Update the FileIndex with new ASTs and plumb the diagnostics responses.
 struct UpdateIndexCallbacks : public ParsingCallbacks {
@@ -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
index a59d1e7ac84096..e42da2a9135e9b 100644
--- 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) {
@@ -795,8 +806,7 @@ void StoreDiags::HandleDiagnostic(DiagnosticsEngine::Level DiagLevel,
     }
     if (Message.empty()) // either !SyntheticMessage, or we failed to make one.
       Info.FormatDiagnostic(Message);
-    LastDiag->Fixes.push_back(
-        Fix{std::string(Message), std::move(Edits), {}});
+    LastDiag->Fixes.push_back(Fix{std::string(Message), std::move(Edits), {}});
     return true;
   };
 
@@ -822,8 +832,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 +851,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
index d4c0478c63a5cf..1665cc0aa0a6a7 100644
--- 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
new file mode 100644
index 00000000000000..cad828b9e6910a
--- /dev/null
+++ 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
new file mode 100644
index 00000000000000..be67776fcbbf3d
--- /dev/null
+++ 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
index 045d32afbc938a..cfd5b53b3e292d 100644
--- 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,21 @@ 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
index 7a591319a74054..3481377a8d1dfa 100644
--- 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
index 75d0fb012ffc81..6e844fbed5b74a 100644
--- 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": "(",
diff --git a/clang-tools-extra/clangd/test/fixits-command-documentchanges.test b/clang-tools-extra/clangd/test/fixits-command-documentchanges.test
index cd636c4df387ad..4fe127776c825b 100644
--- a/clang-tools-extra/clangd/test/fixits-command-documentchanges.test
+++ b/clang-tools-extra/clangd/test/fixits-command-documentchanges.test
@@ -37,6 +37,37 @@
 # 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": 0
+# CHECK-NEXT:              }
+# CHECK-NEXT:            }
+# CHECK-NEXT:          ]
+# CHECK-NEXT:        }
+# CHECK-NEXT:      ],
+# CHECK-NEXT:      "command": "clangd.applyFix",
+# CHECK-NEXT:      "title": "Apply fix: ignore [clang-diagnostic-parentheses] for this line"
+# CHECK-NEXT:    },
+# CHECK-NEXT:    {
+# CHECK-NEXT:      "arguments": [
+# CHECK-NEXT:        {
+# CHECK-NEXT:          "documentChanges": [
+# CHECK-NEXT:            {
+# CHECK-NEXT:              "edits": [
+# CHECK-NEXT:                {
 # CHECK-NEXT:                  "newText": "(",
 # CHECK-NEXT:                  "range": {
 # CHECK-NEXT:                    "end": {
@@ -119,6 +150,37 @@
 # 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...
[truncated]

@llvmbot
Copy link
Member

llvmbot commented Nov 2, 2024

@llvm/pr-subscribers-clangd

Author: Richard Li (chomosuke)

Changes

For some other lsp / linters. They will offer a "Ignore this error for this line" code actions for warnings. I find that convenient as I don't have to type the comment myself and copy the name of the diagnostics.

This is the first time I work on such a large code base so any feedback and guidance are greatly appreciated.


Patch is 32.58 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/114660.diff

13 Files Affected:

  • (modified) clang-tools-extra/clangd/CMakeLists.txt (+1)
  • (modified) clang-tools-extra/clangd/ClangdServer.cpp (+4-3)
  • (modified) clang-tools-extra/clangd/Diagnostics.cpp (+18-8)
  • (modified) clang-tools-extra/clangd/Diagnostics.h (+8-4)
  • (added) clang-tools-extra/clangd/NoLintFixes.cpp (+99)
  • (added) clang-tools-extra/clangd/NoLintFixes.h (+36)
  • (modified) clang-tools-extra/clangd/ParsedAST.cpp (+16-4)
  • (modified) clang-tools-extra/clangd/test/fixits-codeaction-documentchanges.test (+47)
  • (modified) clang-tools-extra/clangd/test/fixits-codeaction.test (+41)
  • (modified) clang-tools-extra/clangd/test/fixits-command-documentchanges.test (+62)
  • (modified) clang-tools-extra/clangd/test/fixits-command.test (+50)
  • (modified) clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp (+13-3)
  • (modified) clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp (+102-15)
diff --git a/clang-tools-extra/clangd/CMakeLists.txt b/clang-tools-extra/clangd/CMakeLists.txt
index d797ddce8c44d1..2a6433a5c3effd 100644
--- 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
index 9b38be04e7ddd7..925a015ae341bc 100644
--- 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"
@@ -62,8 +63,8 @@ namespace clangd {
 namespace {
 
 // Tracks number of times a tweak has been offered.
-static constexpr trace::Metric TweakAvailable(
-    "tweak_available", trace::Metric::Counter, "tweak_id");
+static constexpr trace::Metric
+    TweakAvailable("tweak_available", trace::Metric::Counter, "tweak_id");
 
 // Update the FileIndex with new ASTs and plumb the diagnostics responses.
 struct UpdateIndexCallbacks : public ParsingCallbacks {
@@ -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
index a59d1e7ac84096..e42da2a9135e9b 100644
--- 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) {
@@ -795,8 +806,7 @@ void StoreDiags::HandleDiagnostic(DiagnosticsEngine::Level DiagLevel,
     }
     if (Message.empty()) // either !SyntheticMessage, or we failed to make one.
       Info.FormatDiagnostic(Message);
-    LastDiag->Fixes.push_back(
-        Fix{std::string(Message), std::move(Edits), {}});
+    LastDiag->Fixes.push_back(Fix{std::string(Message), std::move(Edits), {}});
     return true;
   };
 
@@ -822,8 +832,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 +851,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
index d4c0478c63a5cf..1665cc0aa0a6a7 100644
--- 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
new file mode 100644
index 00000000000000..cad828b9e6910a
--- /dev/null
+++ 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
new file mode 100644
index 00000000000000..be67776fcbbf3d
--- /dev/null
+++ 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
index 045d32afbc938a..cfd5b53b3e292d 100644
--- 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,21 @@ 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
index 7a591319a74054..3481377a8d1dfa 100644
--- 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
index 75d0fb012ffc81..6e844fbed5b74a 100644
--- 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": "(",
diff --git a/clang-tools-extra/clangd/test/fixits-command-documentchanges.test b/clang-tools-extra/clangd/test/fixits-command-documentchanges.test
index cd636c4df387ad..4fe127776c825b 100644
--- a/clang-tools-extra/clangd/test/fixits-command-documentchanges.test
+++ b/clang-tools-extra/clangd/test/fixits-command-documentchanges.test
@@ -37,6 +37,37 @@
 # 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": 0
+# CHECK-NEXT:              }
+# CHECK-NEXT:            }
+# CHECK-NEXT:          ]
+# CHECK-NEXT:        }
+# CHECK-NEXT:      ],
+# CHECK-NEXT:      "command": "clangd.applyFix",
+# CHECK-NEXT:      "title": "Apply fix: ignore [clang-diagnostic-parentheses] for this line"
+# CHECK-NEXT:    },
+# CHECK-NEXT:    {
+# CHECK-NEXT:      "arguments": [
+# CHECK-NEXT:        {
+# CHECK-NEXT:          "documentChanges": [
+# CHECK-NEXT:            {
+# CHECK-NEXT:              "edits": [
+# CHECK-NEXT:                {
 # CHECK-NEXT:                  "newText": "(",
 # CHECK-NEXT:                  "range": {
 # CHECK-NEXT:                    "end": {
@@ -119,6 +150,37 @@
 # 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...
[truncated]

@chomosuke chomosuke force-pushed the clang-tidy-nolint-fix branch 2 times, most recently from a8caf22 to 4c506db Compare November 2, 2024 10:27
@chomosuke chomosuke force-pushed the clang-tidy-nolint-fix branch from 4c506db to f52c645 Compare November 2, 2024 10:29
@chomosuke chomosuke marked this pull request as draft November 2, 2024 10:49
@chomosuke chomosuke changed the title Add quick fix to automatically adds NOLINTNEXTLINE comment [clangd] Add quick fix to automatically adds NOLINTNEXTLINE comment Nov 2, 2024
@chomosuke chomosuke closed this Nov 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants