https://github.com/chomosuke updated
https://github.com/llvm/llvm-project/pull/114660
>From 3cc27f1a8880e2c31ce19d86e3bb74ce0b82fbec Mon Sep 17 00:00:00 2001
From: chomosuke <a13323...@gmail.com>
Date: Sat, 2 Nov 2024 09:18:38 +0000
Subject: [PATCH] Add quick fix to automatically adds NOLINTNEXTLINE comment
above clang-tidy warnings
---
clang-tools-extra/clangd/CMakeLists.txt | 1 +
clang-tools-extra/clangd/ClangdServer.cpp | 7 +-
clang-tools-extra/clangd/Diagnostics.cpp | 26 ++--
clang-tools-extra/clangd/Diagnostics.h | 12 +-
clang-tools-extra/clangd/NoLintFixes.cpp | 99 +++++++++++++++
clang-tools-extra/clangd/NoLintFixes.h | 36 ++++++
clang-tools-extra/clangd/ParsedAST.cpp | 20 ++-
.../fixits-codeaction-documentchanges.test | 47 +++++++
.../clangd/test/fixits-codeaction.test | 41 ++++++
.../test/fixits-command-documentchanges.test | 62 ++++++++++
.../clangd/test/fixits-command.test | 50 ++++++++
.../clangd/unittests/ClangdLSPServerTests.cpp | 16 ++-
.../clangd/unittests/DiagnosticsTests.cpp | 117 +++++++++++++++---
13 files changed, 497 insertions(+), 37 deletions(-)
create mode 100644 clang-tools-extra/clangd/NoLintFixes.cpp
create mode 100644 clang-tools-extra/clangd/NoLintFixes.h
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,
+# 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": {
diff --git a/clang-tools-extra/clangd/test/fixits-command.test
b/clang-tools-extra/clangd/test/fixits-command.test
index 62b5a6152d2cf3..8cc6daee48d971 100644
--- a/clang-tools-extra/clangd/test/fixits-command.test
+++ b/clang-tools-extra/clangd/test/fixits-command.test
@@ -34,6 +34,31 @@
# CHECK-NEXT: "arguments": [
# CHECK-NEXT: {
# 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: ],
+# 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: "changes": {
# CHECK-NEXT: "file://{{.*}}/foo.c": [
# CHECK-NEXT: {
# CHECK-NEXT: "newText": "(",
@@ -104,6 +129,31 @@
# CHECK-NEXT: "arguments": [
# CHECK-NEXT: {
# 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: ],
+# 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: "changes": {
# CHECK-NEXT: "file://{{.*}}/foo.c": [
# CHECK-NEXT: {
# CHECK-NEXT: "newText": "(",
diff --git a/clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp
b/clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp
index 49a94045ea4878..3593fd15d022ed 100644
--- a/clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp
+++ b/clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp
@@ -15,6 +15,7 @@
#include "FeatureModule.h"
#include "LSPBinder.h"
#include "LSPClient.h"
+#include "NoLintFixes.h"
#include "TestFS.h"
#include "support/Function.h"
#include "support/Logger.h"
@@ -32,6 +33,7 @@
#include "llvm/Testing/Support/SupportHelpers.h"
#include "gmock/gmock.h"
#include "gtest/gtest.h"
+#include <algorithm>
#include <cassert>
#include <condition_variable>
#include <cstddef>
@@ -39,6 +41,7 @@
#include <memory>
#include <mutex>
#include <optional>
+#include <regex>
#include <thread>
#include <utility>
@@ -222,7 +225,7 @@ TEST_F(LSPTest, ClangTidyRename) {
ASSERT_TRUE(Diags && !Diags->empty());
auto RenameDiag = Diags->front();
- auto RenameCommand =
+ auto Fixes =
(*Client
.call("textDocument/codeAction",
llvm::json::Object{
@@ -232,9 +235,16 @@ TEST_F(LSPTest, ClangTidyRename) {
{"diagnostics", llvm::json::Array{RenameDiag}}}},
{"range", Source.range()}})
.takeValue()
- .getAsArray())[0];
+ .getAsArray());
+ const auto NoLintRegex = std::regex("Apply fix: " + NoLintFixMsgRegexStr);
+ const auto &RenameCommand = *std::find_if(
+ Fixes.begin(), Fixes.end(), [&](decltype(Fixes)::value_type &Fix) {
- ASSERT_EQ((*RenameCommand.getAsObject())["title"], "change 'foo' to 'Foo'");
+ return
!std::regex_match((*Fix.getAsObject())["title"].getAsString()->data(),
NoLintRegex);
+ });
+
+ ASSERT_EQ((*RenameCommand.getAsObject()).get("title")->getAsString(),
+ "change 'foo' to 'Foo'");
Client.expectServerCall("workspace/applyEdit");
Client.call("workspace/executeCommand", RenameCommand);
diff --git a/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
b/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
index 7a47d6ebebf3b2..a092425fb08a66 100644
--- a/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
+++ b/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
@@ -69,6 +69,11 @@ ::testing::Matcher<const Diag &>
withFix(::testing::Matcher<Fix> FixMatcher1,
return Field(&Diag::Fixes, UnorderedElementsAre(FixMatcher1, FixMatcher2));
}
+::testing::Matcher<const Diag &>
+containsFix(::testing::Matcher<Fix> FixMatcher) {
+ return Field(&Diag::Fixes, Contains(FixMatcher));
+}
+
::testing::Matcher<const Diag &> withID(unsigned ID) {
return Field(&Diag::ID, ID);
}
@@ -1297,11 +1302,11 @@ TEST(IncludeFixerTest, IncompleteType) {
for (auto Case : Tests) {
Annotations Main(Case.second);
TU.Code = Main.code().str() + "\n // error-ok";
- EXPECT_THAT(
- TU.build().getDiagnostics(),
- ElementsAre(AllOf(diagName(Case.first), hasRange(Main.range()),
- withFix(Fix(Range{}, "#include \"x.h\"\n",
- "Include \"x.h\" for symbol ns::X")))))
+ EXPECT_THAT(TU.build().getDiagnostics(),
+ ElementsAre(AllOf(
+ diagName(Case.first), hasRange(Main.range()),
+ containsFix(Fix(Range{}, "#include \"x.h\"\n",
+ "Include \"x.h\" for symbol ns::X")))))
<< Case.second;
}
}
@@ -1662,8 +1667,8 @@ TEST(IncludeFixerTest, HeaderNamedInDiag) {
"with type 'int (const char *, ...)'; ISO C99 "
"and later do not support implicit function "
"declarations"),
- withFix(Fix(Test.range("insert"), "#include <stdio.h>\n",
- "Include <stdio.h> for symbol printf")))));
+ containsFix(Fix(Test.range("insert"), "#include <stdio.h>\n",
+ "Include <stdio.h> for symbol printf")))));
TU.ExtraArgs = {"-xc", "-std=c89"};
EXPECT_THAT(
@@ -1671,8 +1676,8 @@ TEST(IncludeFixerTest, HeaderNamedInDiag) {
ElementsAre(AllOf(
Diag(Test.range(), "implicitly declaring library function 'printf' "
"with type 'int (const char *, ...)'"),
- withFix(Fix(Test.range("insert"), "#include <stdio.h>\n",
- "Include <stdio.h> for symbol printf")))));
+ containsFix(Fix(Test.range("insert"), "#include <stdio.h>\n",
+ "Include <stdio.h> for symbol printf")))));
}
TEST(IncludeFixerTest, CImplicitFunctionDecl) {
@@ -1698,15 +1703,97 @@ TEST(IncludeFixerTest, CImplicitFunctionDecl) {
Diag(Test.range(),
"call to undeclared function 'foo'; ISO C99 and later do not "
"support implicit function declarations"),
- withFix(Fix(Range{}, "#include \"foo.h\"\n",
- "Include \"foo.h\" for symbol foo")))));
+ containsFix(Fix(Range{}, "#include \"foo.h\"\n",
+ "Include \"foo.h\" for symbol foo")))));
TU.ExtraArgs = {"-std=c89", "-Wall"};
EXPECT_THAT(TU.build().getDiagnostics(),
ElementsAre(AllOf(
Diag(Test.range(), "implicit declaration of function 'foo'"),
- withFix(Fix(Range{}, "#include \"foo.h\"\n",
- "Include \"foo.h\" for symbol foo")))));
+ containsFix(Fix(Range{}, "#include \"foo.h\"\n",
+ "Include \"foo.h\" for symbol foo")))));
+}
+
+TEST(NolintFixesTest, DiagInMacro) {
+ Annotations Test(R"cpp(
+ #define SQUARE(X) (X)*(X)
+ int main() {
+ int y = 4;
+$insert[[]] return SQUARE([[++]]y);
+ }
+ )cpp");
+
+ auto TU = TestTU::withCode(Test.code());
+ TU.ClangTidyProvider = addTidyChecks("bugprone-macro-repeated-side-effects");
+ auto Index = buildIndexWithSymbol({});
+ TU.ExternalIndex = Index.get();
+ EXPECT_THAT(
+ TU.build().getDiagnostics(),
+ ElementsAre(
+ AllOf(Diag(Test.range(), "side effects in the 1st macro argument 'X'
"
+ "are repeated in macro expansion"),
+ containsFix(Fix(
+ Test.range("insert"),
+ " // "
+ "NOLINTNEXTLINE(bugprone-macro-repeated-side-effects)\n",
+ "ignore [bugprone-macro-repeated-side-effects] for this "
+ "line"))),
+ AllOf(Diag(Test.range(), "multiple unsequenced modifications to
'y'"),
+ containsFix(Fix(
+ Test.range("insert"),
+ " // NOLINTNEXTLINE(clang-diagnostic-unsequenced)\n",
+ "ignore [clang-diagnostic-unsequenced] for this "
+ "line")))));
+}
+
+TEST(NoLintFixesTest, ExistingNoLint) {
+ Annotations Test(R"cpp(
+ int main() {
+ // NOLINTNEXTLINE(readability-isolate-declaration$insert[[]])
+ int [[y]], z = 4;
+ }
+ )cpp");
+ auto TU = TestTU::withCode(Test.code());
+ TU.ClangTidyProvider = addTidyChecks("readability-isolate-declaration,"
+ "cppcoreguidelines-init-variables");
+ auto Index = buildIndexWithSymbol({});
+ TU.ExternalIndex = Index.get();
+
+ EXPECT_THAT(
+ TU.build().getDiagnostics(),
+ ElementsAre(AllOf(
+ Diag(Test.range(), "variable 'y' is not initialized"),
+ containsFix(Fix(Test.range("insert"),
+ ", cppcoreguidelines-init-variables",
+ "ignore [cppcoreguidelines-init-variables] for this "
+ "line")))));
+}
+
+TEST(NoLintFixesTest, RenameNoLint) {
+ Annotations Test(R"cpp(
+ int main() {
+$insert[[]] int [[Y]] = 4;
+ }
+ )cpp");
+ auto TU = TestTU::withCode(Test.code());
+ TU.ClangTidyProvider = [](tidy::ClangTidyOptions &ClangTidyOpts,
+ llvm::StringRef) {
+ ClangTidyOpts.Checks = {"-*,readability-identifier-naming"};
+ ClangTidyOpts.CheckOptions["readability-identifier-naming.VariableCase"] =
+ "lower_case";
+ };
+ auto Index = buildIndexWithSymbol({});
+ TU.ExternalIndex = Index.get();
+
+ EXPECT_THAT(
+ TU.build().getDiagnostics(),
+ ElementsAre(
+ AllOf(Diag(Test.range(), "invalid case style for variable 'Y'"),
+ containsFix(Fix(
+ Test.range("insert"),
+ " // NOLINTNEXTLINE(readability-identifier-naming)\n",
+ "ignore [readability-identifier-naming] for this "
+ "line")))));
}
TEST(DiagsInHeaders, DiagInsideHeader) {
@@ -1930,10 +2017,10 @@ TEST(ParsedASTTest, ModuleSawDiag) {
TestTU TU;
auto AST = TU.build();
- #if 0
+#if 0
EXPECT_THAT(AST.getDiagnostics(),
testing::Contains(Diag(Code.range(), KDiagMsg.str())));
- #endif
+#endif
}
TEST(Preamble, EndsOnNonEmptyLine) {
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
