https://github.com/chomosuke created
https://github.com/llvm/llvm-project/pull/114663
None
>From 7b70b905e5a9942f592316b407135975977c93cb Mon Sep 17 00:00:00 2001
From: chomosuke <a13323...@gmail.com>
Date: Fri, 16 Aug 2024 13:31:21 +0000
Subject: [PATCH 01/19] Fixing one error
---
clang-tools-extra/clangd/ClangdServer.cpp | 23 ++++++++++++++++++++++-
1 file changed, 22 insertions(+), 1 deletion(-)
diff --git a/clang-tools-extra/clangd/ClangdServer.cpp
b/clang-tools-extra/clangd/ClangdServer.cpp
index 9b38be04e7ddd7..e08ce12223f6b0 100644
--- a/clang-tools-extra/clangd/ClangdServer.cpp
+++ b/clang-tools-extra/clangd/ClangdServer.cpp
@@ -44,6 +44,7 @@
#include "llvm/ADT/STLExtras.h"
#include "llvm/ADT/StringRef.h"
#include "llvm/Support/Error.h"
+#include "llvm/Support/Format.h"
#include "llvm/Support/Path.h"
#include "llvm/Support/raw_ostream.h"
#include <algorithm>
@@ -672,6 +673,21 @@ tryConvertToRename(const Diag *Diag, const Fix &Fix) {
return std::nullopt;
}
+// Add NOLINT insert as code actions
+std::optional<Fix> tryAddClangTidySuppression(const Diag *Diag) {
+ if (Diag->Source == Diag::ClangTidy) {
+ Fix F;
+ F.Message = llvm::formatv("ignore [{0}] for this line", Diag->Name);
+ TextEdit &E = F.Edits.emplace_back();
+ E.newText = llvm::formatv("// NOLINTNEXTLINE({0})\n", Diag->Name);
+ Position InsertPos = Diag->Range.start;
+ InsertPos.character = 0;
+ E.range = {InsertPos, InsertPos};
+ return F;
+ }
+ return std::nullopt;
+}
+
} // namespace
void ClangdServer::codeAction(const CodeActionInputs &Params,
@@ -701,7 +717,7 @@ void ClangdServer::codeAction(const CodeActionInputs
&Params,
return nullptr;
};
for (const auto &DiagRef : Params.Diagnostics) {
- if (const auto *Diag = FindMatchedDiag(DiagRef))
+ if (const auto *Diag = FindMatchedDiag(DiagRef)) {
for (const auto &Fix : Diag->Fixes) {
if (auto Rename = tryConvertToRename(Diag, Fix)) {
Result.Renames.emplace_back(std::move(*Rename));
@@ -709,6 +725,11 @@ void ClangdServer::codeAction(const CodeActionInputs
&Params,
Result.QuickFixes.push_back({DiagRef, Fix});
}
}
+
+ if (auto Fix = tryAddClangTidySuppression(Diag)) {
+ Result.QuickFixes.push_back({DiagRef, std::move(*Fix)});
+ }
+ }
}
}
>From d5deb1d93e994f44d6b4d932e261a3bd9a42dcc8 Mon Sep 17 00:00:00 2001
From: chomosuke <a13323...@gmail.com>
Date: Sun, 18 Aug 2024 17:15:21 +0000
Subject: [PATCH 02/19] Revert "Fixing one error"
This reverts commit 17989044ad480628a2f7814674dbe7cd985abd17.
---
clang-tools-extra/clangd/ClangdServer.cpp | 23 +----------------------
1 file changed, 1 insertion(+), 22 deletions(-)
diff --git a/clang-tools-extra/clangd/ClangdServer.cpp
b/clang-tools-extra/clangd/ClangdServer.cpp
index e08ce12223f6b0..9b38be04e7ddd7 100644
--- a/clang-tools-extra/clangd/ClangdServer.cpp
+++ b/clang-tools-extra/clangd/ClangdServer.cpp
@@ -44,7 +44,6 @@
#include "llvm/ADT/STLExtras.h"
#include "llvm/ADT/StringRef.h"
#include "llvm/Support/Error.h"
-#include "llvm/Support/Format.h"
#include "llvm/Support/Path.h"
#include "llvm/Support/raw_ostream.h"
#include <algorithm>
@@ -673,21 +672,6 @@ tryConvertToRename(const Diag *Diag, const Fix &Fix) {
return std::nullopt;
}
-// Add NOLINT insert as code actions
-std::optional<Fix> tryAddClangTidySuppression(const Diag *Diag) {
- if (Diag->Source == Diag::ClangTidy) {
- Fix F;
- F.Message = llvm::formatv("ignore [{0}] for this line", Diag->Name);
- TextEdit &E = F.Edits.emplace_back();
- E.newText = llvm::formatv("// NOLINTNEXTLINE({0})\n", Diag->Name);
- Position InsertPos = Diag->Range.start;
- InsertPos.character = 0;
- E.range = {InsertPos, InsertPos};
- return F;
- }
- return std::nullopt;
-}
-
} // namespace
void ClangdServer::codeAction(const CodeActionInputs &Params,
@@ -717,7 +701,7 @@ void ClangdServer::codeAction(const CodeActionInputs
&Params,
return nullptr;
};
for (const auto &DiagRef : Params.Diagnostics) {
- if (const auto *Diag = FindMatchedDiag(DiagRef)) {
+ if (const auto *Diag = FindMatchedDiag(DiagRef))
for (const auto &Fix : Diag->Fixes) {
if (auto Rename = tryConvertToRename(Diag, Fix)) {
Result.Renames.emplace_back(std::move(*Rename));
@@ -725,11 +709,6 @@ void ClangdServer::codeAction(const CodeActionInputs
&Params,
Result.QuickFixes.push_back({DiagRef, Fix});
}
}
-
- if (auto Fix = tryAddClangTidySuppression(Diag)) {
- Result.QuickFixes.push_back({DiagRef, std::move(*Fix)});
- }
- }
}
}
>From 355a32406ae0ce93aed941c4865b040009289c82 Mon Sep 17 00:00:00 2001
From: chomosuke <a13323...@gmail.com>
Date: Mon, 19 Aug 2024 15:17:05 +0000
Subject: [PATCH 03/19] inserted NOLINT with indent
Retrieved CurLine and PrevLine
---
clang-tools-extra/clangd/Diagnostics.cpp | 4 +-
clang-tools-extra/clangd/Diagnostics.h | 4 +-
clang-tools-extra/clangd/ParsedAST.cpp | 57 ++++++++++++++++++++++--
3 files changed, 57 insertions(+), 8 deletions(-)
diff --git a/clang-tools-extra/clangd/Diagnostics.cpp
b/clang-tools-extra/clangd/Diagnostics.cpp
index a59d1e7ac84096..47810248f093e4 100644
--- a/clang-tools-extra/clangd/Diagnostics.cpp
+++ b/clang-tools-extra/clangd/Diagnostics.cpp
@@ -823,7 +823,7 @@ void StoreDiags::HandleDiagnostic(DiagnosticsEngine::Level
DiagLevel,
if (!Info.getFixItHints().empty())
AddFix(true /* try to invent a message instead of repeating the diag */);
if (Fixer) {
- auto ExtraFixes = Fixer(LastDiag->Severity, Info);
+ auto ExtraFixes = Fixer(*LastDiag, Info);
LastDiag->Fixes.insert(LastDiag->Fixes.end(), ExtraFixes.begin(),
ExtraFixes.end());
}
@@ -842,7 +842,7 @@ void StoreDiags::HandleDiagnostic(DiagnosticsEngine::Level
DiagLevel,
// Give include-fixer a chance to replace a note with a fix.
if (Fixer) {
- auto ReplacementFixes = Fixer(LastDiag->Severity, Info);
+ auto ReplacementFixes = Fixer(*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..4fe41fecd8ae5c 100644
--- a/clang-tools-extra/clangd/Diagnostics.h
+++ b/clang-tools-extra/clangd/Diagnostics.h
@@ -148,8 +148,8 @@ class StoreDiags : public DiagnosticConsumer {
/// When passed a main diagnostic, returns fixes to add to it.
/// When passed a note diagnostic, returns fixes to replace it with.
- using DiagFixer = std::function<std::vector<Fix>(DiagnosticsEngine::Level,
- const clang::Diagnostic &)>;
+ using DiagFixer =
+ std::function<std::vector<Fix>(const Diag &, const clang::Diagnostic &)>;
using LevelAdjuster = std::function<DiagnosticsEngine::Level(
DiagnosticsEngine::Level, const clang::Diagnostic &)>;
using DiagCallback =
diff --git a/clang-tools-extra/clangd/ParsedAST.cpp
b/clang-tools-extra/clangd/ParsedAST.cpp
index 045d32afbc938a..4cac2091864c66 100644
--- a/clang-tools-extra/clangd/ParsedAST.cpp
+++ b/clang-tools-extra/clangd/ParsedAST.cpp
@@ -67,6 +67,7 @@
#include "llvm/Support/Error.h"
#include "llvm/Support/MemoryBuffer.h"
#include <cassert>
+#include <cctype>
#include <cstddef>
#include <iterator>
#include <memory>
@@ -401,6 +402,48 @@ filterFastTidyChecks(const tidy::ClangTidyCheckFactories
&All,
} // namespace
+std::vector<Fix>
+clangTidyNoLintFixes(const clang::tidy::ClangTidyContext &CTContext,
+ const clang::Diagnostic &Info, const Diag &Diag) {
+ auto RuleName = CTContext.getCheckName(Info.getID());
+ if (RuleName.empty()) {
+ return {};
+ }
+ if (!Diag.InsideMainFile) {
+ return {};
+ }
+ auto &SrcMgr = Info.getSourceManager();
+ auto &DiagLoc = 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 CodeTilDiag = toSourceCode(
+ SrcMgr, SourceRange(SrcMgr.getLocForStartOfFile(File), DiagLoc));
+
+ auto StartCurLine = CodeTilDiag.find_last_of('\n') + 1;
+ auto CurLine = CodeTilDiag.substr(StartCurLine);
+ elog("CurLine: '{0}'", CurLine);
+ auto Indent = CurLine.take_while([](char C) { return std::isspace(C); });
+
+ if (StartCurLine > 0) {
+ auto StartPrevLine = CodeTilDiag.find_last_of('\n', StartCurLine - 1) + 1;
+ auto PrevLine =
+ CodeTilDiag.substr(StartPrevLine, StartCurLine - StartPrevLine - 1);
+ elog("PrevLine: '{0}'", PrevLine);
+ }
+
+ E.newText = llvm::formatv("{0}// NOLINTNEXTLINE({1})\n", Indent, RuleName);
+
+ auto InsertPos = sourceLocToPosition(SrcMgr, DiagLoc);
+ InsertPos.character = 0;
+ E.range = {InsertPos, InsertPos};
+
+ return {F};
+}
+
std::optional<ParsedAST>
ParsedAST::build(llvm::StringRef Filename, const ParseInputs &Inputs,
std::unique_ptr<clang::CompilerInvocation> CI,
@@ -655,10 +698,16 @@ 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.contributeFixes(
+ [&FixIncludes, &CTContext](const Diag &Diag,
+ const clang::Diagnostic &Info) {
+ auto Fixes = std::vector<Fix>();
+ auto NoLintFixes = clangTidyNoLintFixes(*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;
+ });
Clang->setExternalSemaSource(FixIncludes->unresolvedNameRecorder());
}
}
>From a3db72a4515980c027596df967990b38ac019049 Mon Sep 17 00:00:00 2001
From: chomosuke <a13323...@gmail.com>
Date: Mon, 19 Aug 2024 19:38:05 +0000
Subject: [PATCH 04/19] Fixed converting 'ignore ... for this line' Fix into
rename bug
---
clang-tools-extra/clangd/ClangdServer.cpp | 17 ++++++++++++-----
1 file changed, 12 insertions(+), 5 deletions(-)
diff --git a/clang-tools-extra/clangd/ClangdServer.cpp
b/clang-tools-extra/clangd/ClangdServer.cpp
index 9b38be04e7ddd7..38808c5549608c 100644
--- a/clang-tools-extra/clangd/ClangdServer.cpp
+++ b/clang-tools-extra/clangd/ClangdServer.cpp
@@ -701,14 +701,21 @@ void ClangdServer::codeAction(const CodeActionInputs
&Params,
return nullptr;
};
for (const auto &DiagRef : Params.Diagnostics) {
- if (const auto *Diag = FindMatchedDiag(DiagRef))
- for (const auto &Fix : Diag->Fixes) {
- if (auto Rename = tryConvertToRename(Diag, Fix)) {
+ if (const auto *Diag = FindMatchedDiag(DiagRef)) {
+ auto It = Diag->Fixes.begin();
+ if (It != Diag->Fixes.end()) {
+ if (auto Rename = tryConvertToRename(Diag, *It)) {
+ // Only try to convert the first Fix to rename as subsequent
Fixes
+ // might be "ignore [readability-identifier-naming] for this
+ // line".
Result.Renames.emplace_back(std::move(*Rename));
- } else {
- Result.QuickFixes.push_back({DiagRef, Fix});
+ It++;
}
}
+ for (; It != Diag->Fixes.end(); It++) {
+ Result.QuickFixes.push_back({DiagRef, *It});
+ }
+ }
}
}
>From 7aa312669902143e85cda300b8f0cab5e6c4a9e3 Mon Sep 17 00:00:00 2001
From: chomosuke <a13323...@gmail.com>
Date: Mon, 19 Aug 2024 19:48:25 +0000
Subject: [PATCH 05/19] Adding to exsiting NOLINTNEXTLINE if possible
---
clang-tools-extra/clangd/ParsedAST.cpp | 19 ++++++++++++++-----
1 file changed, 14 insertions(+), 5 deletions(-)
diff --git a/clang-tools-extra/clangd/ParsedAST.cpp
b/clang-tools-extra/clangd/ParsedAST.cpp
index 4cac2091864c66..253f44020011b6 100644
--- a/clang-tools-extra/clangd/ParsedAST.cpp
+++ b/clang-tools-extra/clangd/ParsedAST.cpp
@@ -425,20 +425,29 @@ clangTidyNoLintFixes(const clang::tidy::ClangTidyContext
&CTContext,
auto StartCurLine = CodeTilDiag.find_last_of('\n') + 1;
auto CurLine = CodeTilDiag.substr(StartCurLine);
- elog("CurLine: '{0}'", CurLine);
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);
- elog("PrevLine: '{0}'", PrevLine);
+ auto NLPos = PrevLine.find("NOLINTNEXTLINE(");
+ if (NLPos != StringRef::npos) {
+ ExistingNoLintNextLineInsertPos =
+ std::make_optional(PrevLine.find(")", NLPos));
+ }
}
- E.newText = llvm::formatv("{0}// NOLINTNEXTLINE({1})\n", Indent, RuleName);
-
auto InsertPos = sourceLocToPosition(SrcMgr, DiagLoc);
- InsertPos.character = 0;
+ 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};
>From 81d9847ccbf6df605c799f7243ee93699caeae37 Mon Sep 17 00:00:00 2001
From: chomosuke <a13323...@gmail.com>
Date: Thu, 3 Oct 2024 06:31:02 +0000
Subject: [PATCH 06/19] No NOLINT fix for Error Diagnostic
---
clang-tools-extra/clangd/ParsedAST.cpp | 9 ++++-----
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/clang-tools-extra/clangd/ParsedAST.cpp
b/clang-tools-extra/clangd/ParsedAST.cpp
index 253f44020011b6..98c158ab9d5a38 100644
--- a/clang-tools-extra/clangd/ParsedAST.cpp
+++ b/clang-tools-extra/clangd/ParsedAST.cpp
@@ -405,13 +405,12 @@ filterFastTidyChecks(const tidy::ClangTidyCheckFactories
&All,
std::vector<Fix>
clangTidyNoLintFixes(const clang::tidy::ClangTidyContext &CTContext,
const clang::Diagnostic &Info, const Diag &Diag) {
- auto RuleName = CTContext.getCheckName(Info.getID());
- if (RuleName.empty()) {
- return {};
- }
- if (!Diag.InsideMainFile) {
+ auto RuleName = CTContext.getCheckName(Diag.ID);
+ if (RuleName.empty() || Diag.Severity >= DiagnosticsEngine::Error ||
+ !Diag.InsideMainFile) {
return {};
}
+
auto &SrcMgr = Info.getSourceManager();
auto &DiagLoc = Info.getLocation();
>From 33fa277ce3b080e787bdf98dd1fbb58e32435316 Mon Sep 17 00:00:00 2001
From: chomosuke <a13323...@gmail.com>
Date: Fri, 4 Oct 2024 04:52:00 +0000
Subject: [PATCH 07/19] added comment for return {} branch
---
clang-tools-extra/clangd/ParsedAST.cpp | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/clang-tools-extra/clangd/ParsedAST.cpp
b/clang-tools-extra/clangd/ParsedAST.cpp
index 98c158ab9d5a38..4e713258c58213 100644
--- a/clang-tools-extra/clangd/ParsedAST.cpp
+++ b/clang-tools-extra/clangd/ParsedAST.cpp
@@ -406,7 +406,12 @@ std::vector<Fix>
clangTidyNoLintFixes(const clang::tidy::ClangTidyContext &CTContext,
const clang::Diagnostic &Info, const Diag &Diag) {
auto RuleName = CTContext.getCheckName(Diag.ID);
- if (RuleName.empty() || Diag.Severity >= DiagnosticsEngine::Error ||
+ 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 {};
}
>From c1eb6ebac8ef3206942d2c6ed45bc33b6c435645 Mon Sep 17 00:00:00 2001
From: chomosuke <a13323...@gmail.com>
Date: Fri, 4 Oct 2024 20:44:21 +0000
Subject: [PATCH 08/19] found '(fix available)' insertion code
---
clang-tools-extra/clangd/Diagnostics.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/clang-tools-extra/clangd/Diagnostics.cpp
b/clang-tools-extra/clangd/Diagnostics.cpp
index 47810248f093e4..ee7d36e029fcde 100644
--- a/clang-tools-extra/clangd/Diagnostics.cpp
+++ b/clang-tools-extra/clangd/Diagnostics.cpp
@@ -312,7 +312,7 @@ std::string mainMessage(const Diag &D, const
ClangdDiagnosticOptions &Opts) {
llvm::raw_string_ostream OS(Result);
OS << D.Message;
if (Opts.DisplayFixesCount && !D.Fixes.empty())
- OS << " (" << (D.Fixes.size() > 1 ? "fixes" : "fix") << " available)";
+ OS << " (" << (D.Fixes.size() > 1 ? "fixes" : "fix") << " available)"; //
TODO
// If notes aren't emitted as structured info, add them to the message.
if (!Opts.EmitRelatedLocations)
for (auto &Note : D.Notes) {
>From ea05cf95d8883a37a3a731e071bccf826f69b14b Mon Sep 17 00:00:00 2001
From: chomosuke <a13323...@gmail.com>
Date: Sat, 5 Oct 2024 01:41:49 +0000
Subject: [PATCH 09/19] Don't display (fix available) when there's only NOLINT
fix
---
clang-tools-extra/clangd/CMakeLists.txt | 1 +
clang-tools-extra/clangd/Diagnostics.cpp | 18 +++-
clang-tools-extra/clangd/NoLintFixes.cpp | 102 +++++++++++++++++++++++
clang-tools-extra/clangd/NoLintFixes.h | 35 ++++++++
clang-tools-extra/clangd/ParsedAST.cpp | 57 +------------
5 files changed, 153 insertions(+), 60 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/Diagnostics.cpp
b/clang-tools-extra/clangd/Diagnostics.cpp
index ee7d36e029fcde..f345f7c88542b3 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)"; //
TODO
+
+ // 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 (isClangTidyNoLintFixes(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;
};
diff --git a/clang-tools-extra/clangd/NoLintFixes.cpp
b/clang-tools-extra/clangd/NoLintFixes.cpp
new file mode 100644
index 00000000000000..2ba86b611a8612
--- /dev/null
+++ b/clang-tools-extra/clangd/NoLintFixes.cpp
@@ -0,0 +1,102 @@
+//===--- 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>
+clangTidyNoLintFixes(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 = 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 CodeTilDiag = toSourceCode(
+ SrcMgr, SourceRange(SrcMgr.getLocForStartOfFile(File), 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 MsgRegex = std::regex("ignore \\[.*\\] for this line");
+bool isClangTidyNoLintFixes(const Fix &F) {
+ return std::regex_match(F.Message, MsgRegex);
+}
+
+} // 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..dabe154585e0fb
--- /dev/null
+++ b/clang-tools-extra/clangd/NoLintFixes.h
@@ -0,0 +1,35 @@
+//===--- 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>
+clangTidyNoLintFixes(const clang::tidy::ClangTidyContext &CTContext,
+ const clang::Diagnostic &Info, const Diag &Diag);
+
+/// Check if a fix created by clangTidyNoLintFixes().
+bool isClangTidyNoLintFixes(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 4e713258c58213..eb267c66e9e953 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"
@@ -67,7 +68,6 @@
#include "llvm/Support/Error.h"
#include "llvm/Support/MemoryBuffer.h"
#include <cassert>
-#include <cctype>
#include <cstddef>
#include <iterator>
#include <memory>
@@ -402,61 +402,6 @@ filterFastTidyChecks(const tidy::ClangTidyCheckFactories
&All,
} // namespace
-std::vector<Fix>
-clangTidyNoLintFixes(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 = 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 CodeTilDiag = toSourceCode(
- SrcMgr, SourceRange(SrcMgr.getLocForStartOfFile(File), 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};
-}
-
std::optional<ParsedAST>
ParsedAST::build(llvm::StringRef Filename, const ParseInputs &Inputs,
std::unique_ptr<clang::CompilerInvocation> CI,
>From 9329e96e9685566b43b9194d992c825ca72df9e9 Mon Sep 17 00:00:00 2001
From: chomosuke <a13323...@gmail.com>
Date: Sat, 5 Oct 2024 10:14:30 +0000
Subject: [PATCH 10/19] changed withFix to containsFix in DiagnosticsTest to
ignore NOLINT fix
---
.../clangd/unittests/DiagnosticsTests.cpp | 38 +++++++++++--------
1 file changed, 23 insertions(+), 15 deletions(-)
diff --git a/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
b/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
index 7a47d6ebebf3b2..da5786a4de2931 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);
}
@@ -1290,6 +1295,7 @@ TEST(IncludeFixerTest, IncompleteType) {
{"call_incomplete_argument", "int m(ns::X); int i = m([[*x]]);"},
{"switch_incomplete_class_type", "void a() { [[switch]](*x) {} }"},
{"delete_incomplete_class_type", "void f() { [[delete]] *x; }"},
+ // TODO: Add to test case
{"-Wdelete-incomplete", "void f() { [[delete]] x; }"},
{"dereference_incomplete_type",
R"cpp(void f() { asm("" : "=r"([[*]]x)::); })cpp"},
@@ -1297,11 +1303,12 @@ 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")))))
+ // TODO: maybe only do containsFix on -Wdelete-incomplete
+ 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 +1669,9 @@ 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")))));
+ // TODO: Add to test case
+ containsFix(Fix(Test.range("insert"), "#include <stdio.h>\n",
+ "Include <stdio.h> for symbol printf")))));
TU.ExtraArgs = {"-xc", "-std=c89"};
EXPECT_THAT(
@@ -1671,8 +1679,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 +1706,15 @@ 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(DiagsInHeaders, DiagInsideHeader) {
@@ -1930,10 +1938,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) {
>From 53a0df47c8b8917227a1e15d9f3f589e5f8a2e28 Mon Sep 17 00:00:00 2001
From: chomosuke <a13323...@gmail.com>
Date: Sat, 5 Oct 2024 12:02:13 +0000
Subject: [PATCH 11/19] Filtered NOLINT fix in ClangTidyRename test
---
clang-tools-extra/clangd/Diagnostics.cpp | 2 +-
clang-tools-extra/clangd/NoLintFixes.cpp | 8 ++++----
clang-tools-extra/clangd/NoLintFixes.h | 9 +++++----
clang-tools-extra/clangd/ParsedAST.cpp | 2 +-
.../clangd/unittests/ClangdLSPServerTests.cpp | 16 +++++++++++++---
5 files changed, 24 insertions(+), 13 deletions(-)
diff --git a/clang-tools-extra/clangd/Diagnostics.cpp
b/clang-tools-extra/clangd/Diagnostics.cpp
index f345f7c88542b3..c1a4fc5c9ca7b1 100644
--- a/clang-tools-extra/clangd/Diagnostics.cpp
+++ b/clang-tools-extra/clangd/Diagnostics.cpp
@@ -317,7 +317,7 @@ std::string mainMessage(const Diag &D, const
ClangdDiagnosticOptions &Opts) {
// the fixes is just to suppress could be misleading.
int RealFixCount = D.Fixes.size();
for (auto const &Fix : D.Fixes) {
- if (isClangTidyNoLintFixes(Fix)) {
+ if (isNoLintFixes(Fix)) {
RealFixCount--;
}
}
diff --git a/clang-tools-extra/clangd/NoLintFixes.cpp
b/clang-tools-extra/clangd/NoLintFixes.cpp
index 2ba86b611a8612..80e2976c8bea30 100644
--- a/clang-tools-extra/clangd/NoLintFixes.cpp
+++ b/clang-tools-extra/clangd/NoLintFixes.cpp
@@ -39,7 +39,7 @@ namespace clang {
namespace clangd {
std::vector<Fix>
-clangTidyNoLintFixes(const clang::tidy::ClangTidyContext &CTContext,
+noLintFixes(const clang::tidy::ClangTidyContext &CTContext,
const clang::Diagnostic &Info, const Diag &Diag) {
auto RuleName = CTContext.getCheckName(Diag.ID);
if (
@@ -93,9 +93,9 @@ clangTidyNoLintFixes(const clang::tidy::ClangTidyContext
&CTContext,
return {F};
}
-const auto MsgRegex = std::regex("ignore \\[.*\\] for this line");
-bool isClangTidyNoLintFixes(const Fix &F) {
- return std::regex_match(F.Message, MsgRegex);
+const auto Regex = std::regex(NoLintFixMsgRegexStr);
+bool isNoLintFixes(const Fix &F) {
+ return std::regex_match(F.Message, Regex);
}
} // namespace clangd
diff --git a/clang-tools-extra/clangd/NoLintFixes.h
b/clang-tools-extra/clangd/NoLintFixes.h
index dabe154585e0fb..be67776fcbbf3d 100644
--- a/clang-tools-extra/clangd/NoLintFixes.h
+++ b/clang-tools-extra/clangd/NoLintFixes.h
@@ -22,12 +22,13 @@ namespace clangd {
/// Suggesting to insert "\\ NOLINTNEXTLINE(...)" to suppress clang-tidy
/// diagnostics.
-std::vector<Fix>
-clangTidyNoLintFixes(const clang::tidy::ClangTidyContext &CTContext,
- const clang::Diagnostic &Info, const Diag &Diag);
+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 isClangTidyNoLintFixes(const Fix &F);
+bool isNoLintFixes(const Fix &F);
} // namespace clangd
} // namespace clang
diff --git a/clang-tools-extra/clangd/ParsedAST.cpp
b/clang-tools-extra/clangd/ParsedAST.cpp
index eb267c66e9e953..7e46cb62c18857 100644
--- a/clang-tools-extra/clangd/ParsedAST.cpp
+++ b/clang-tools-extra/clangd/ParsedAST.cpp
@@ -660,7 +660,7 @@ ParsedAST::build(llvm::StringRef Filename, const
ParseInputs &Inputs,
[&FixIncludes, &CTContext](const Diag &Diag,
const clang::Diagnostic &Info) {
auto Fixes = std::vector<Fix>();
- auto NoLintFixes = clangTidyNoLintFixes(*CTContext, Info, Diag);
+ 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());
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);
>From 7d2860b4d0793ca29bd8614a68d881a63c349516 Mon Sep 17 00:00:00 2001
From: chomosuke <a13323...@gmail.com>
Date: Mon, 7 Oct 2024 03:51:46 +0000
Subject: [PATCH 12/19] moved assert for Include-fixer to correct place
---
clang-tools-extra/clangd/Diagnostics.cpp | 2 --
clang-tools-extra/clangd/ParsedAST.cpp | 26 +++++++++++++++---------
2 files changed, 16 insertions(+), 12 deletions(-)
diff --git a/clang-tools-extra/clangd/Diagnostics.cpp
b/clang-tools-extra/clangd/Diagnostics.cpp
index c1a4fc5c9ca7b1..da40afbc3b22a5 100644
--- a/clang-tools-extra/clangd/Diagnostics.cpp
+++ b/clang-tools-extra/clangd/Diagnostics.cpp
@@ -854,8 +854,6 @@ void StoreDiags::HandleDiagnostic(DiagnosticsEngine::Level
DiagLevel,
if (Fixer) {
auto ReplacementFixes = Fixer(*LastDiag, Info);
if (!ReplacementFixes.empty()) {
- assert(Info.getNumFixItHints() == 0 &&
- "Include-fixer replaced a note with clang fix-its attached!");
LastDiag->Fixes.insert(LastDiag->Fixes.end(), ReplacementFixes.begin(),
ReplacementFixes.end());
return;
diff --git a/clang-tools-extra/clangd/ParsedAST.cpp
b/clang-tools-extra/clangd/ParsedAST.cpp
index 7e46cb62c18857..949e54bb71ace5 100644
--- a/clang-tools-extra/clangd/ParsedAST.cpp
+++ b/clang-tools-extra/clangd/ParsedAST.cpp
@@ -656,16 +656,22 @@ ParsedAST::build(llvm::StringRef Filename, const
ParseInputs &Inputs,
: Symbol::Include;
FixIncludes.emplace(Filename, Inserter, *Inputs.Index,
/*IndexRequestLimit=*/5, Directive);
- ASTDiags.contributeFixes(
- [&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.contributeFixes([&FixIncludes,
+ &CTContext](const Diag &Diag,
+ const clang::Diagnostic &Info) {
+ auto Fixes = std::vector<Fix>();
+ auto IncludeFixes = FixIncludes->fix(Diag.Severity, Info);
+
+ // Ensures that if clang later introduces its own fix-it for includes
it
+ // will get on our radar.
+ assert((IncludeFixes.empty() || Info.getNumFixItHints() == 0) &&
+ "Include-fixer replaced a note with clang fix-its attached!");
+
+ Fixes.insert(Fixes.end(), IncludeFixes.begin(), IncludeFixes.end());
+ auto NoLintFixes = noLintFixes(*CTContext, Info, Diag);
+ Fixes.insert(Fixes.end(), NoLintFixes.begin(), NoLintFixes.end());
+ return Fixes;
+ });
Clang->setExternalSemaSource(FixIncludes->unresolvedNameRecorder());
}
}
>From a0c6fe177c4c1e25aadd4c98eed99f8bdbd69499 Mon Sep 17 00:00:00 2001
From: chomosuke <a13323...@gmail.com>
Date: Mon, 7 Oct 2024 03:59:59 +0000
Subject: [PATCH 13/19] better deal with converting fixes to rename while
avoiding NoLint fixes
---
clang-tools-extra/clangd/ClangdServer.cpp | 24 +++++++++--------------
clang-tools-extra/clangd/ParsedAST.cpp | 5 +++--
2 files changed, 12 insertions(+), 17 deletions(-)
diff --git a/clang-tools-extra/clangd/ClangdServer.cpp
b/clang-tools-extra/clangd/ClangdServer.cpp
index 38808c5549608c..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;
@@ -701,21 +702,14 @@ void ClangdServer::codeAction(const CodeActionInputs
&Params,
return nullptr;
};
for (const auto &DiagRef : Params.Diagnostics) {
- if (const auto *Diag = FindMatchedDiag(DiagRef)) {
- auto It = Diag->Fixes.begin();
- if (It != Diag->Fixes.end()) {
- if (auto Rename = tryConvertToRename(Diag, *It)) {
- // Only try to convert the first Fix to rename as subsequent
Fixes
- // might be "ignore [readability-identifier-naming] for this
- // line".
+ if (const auto *Diag = FindMatchedDiag(DiagRef))
+ for (const auto &Fix : Diag->Fixes) {
+ if (auto Rename = tryConvertToRename(Diag, Fix)) {
Result.Renames.emplace_back(std::move(*Rename));
- It++;
+ } else {
+ Result.QuickFixes.push_back({DiagRef, Fix});
}
}
- for (; It != Diag->Fixes.end(); It++) {
- Result.QuickFixes.push_back({DiagRef, *It});
- }
- }
}
}
diff --git a/clang-tools-extra/clangd/ParsedAST.cpp
b/clang-tools-extra/clangd/ParsedAST.cpp
index 949e54bb71ace5..095c3b02e3613a 100644
--- a/clang-tools-extra/clangd/ParsedAST.cpp
+++ b/clang-tools-extra/clangd/ParsedAST.cpp
@@ -660,16 +660,17 @@ ParsedAST::build(llvm::StringRef Filename, const
ParseInputs &Inputs,
&CTContext](const Diag &Diag,
const clang::Diagnostic &Info) {
auto Fixes = std::vector<Fix>();
- auto IncludeFixes = FixIncludes->fix(Diag.Severity, Info);
+ auto IncludeFixes = FixIncludes->fix(Diag.Severity, Info);
// Ensures that if clang later introduces its own fix-it for includes
it
// will get on our radar.
assert((IncludeFixes.empty() || Info.getNumFixItHints() == 0) &&
"Include-fixer replaced a note with clang fix-its attached!");
-
Fixes.insert(Fixes.end(), IncludeFixes.begin(), IncludeFixes.end());
+
auto NoLintFixes = noLintFixes(*CTContext, Info, Diag);
Fixes.insert(Fixes.end(), NoLintFixes.begin(), NoLintFixes.end());
+
return Fixes;
});
Clang->setExternalSemaSource(FixIncludes->unresolvedNameRecorder());
>From e17bf1bc45aee8222ba6415a89593864a1091a54 Mon Sep 17 00:00:00 2001
From: chomosuke <a13323...@gmail.com>
Date: Wed, 9 Oct 2024 00:14:04 +0000
Subject: [PATCH 14/19] Revert "moved assert for Include-fixer to correct
place"
This reverts commit 039ecf375d0d71524b1c3a904f93e57a10807b41.
---
clang-tools-extra/clangd/Diagnostics.cpp | 2 ++
clang-tools-extra/clangd/ParsedAST.cpp | 27 +++++++++---------------
2 files changed, 12 insertions(+), 17 deletions(-)
diff --git a/clang-tools-extra/clangd/Diagnostics.cpp
b/clang-tools-extra/clangd/Diagnostics.cpp
index da40afbc3b22a5..c1a4fc5c9ca7b1 100644
--- a/clang-tools-extra/clangd/Diagnostics.cpp
+++ b/clang-tools-extra/clangd/Diagnostics.cpp
@@ -854,6 +854,8 @@ void StoreDiags::HandleDiagnostic(DiagnosticsEngine::Level
DiagLevel,
if (Fixer) {
auto ReplacementFixes = Fixer(*LastDiag, Info);
if (!ReplacementFixes.empty()) {
+ assert(Info.getNumFixItHints() == 0 &&
+ "Include-fixer replaced a note with clang fix-its attached!");
LastDiag->Fixes.insert(LastDiag->Fixes.end(), ReplacementFixes.begin(),
ReplacementFixes.end());
return;
diff --git a/clang-tools-extra/clangd/ParsedAST.cpp
b/clang-tools-extra/clangd/ParsedAST.cpp
index 095c3b02e3613a..7e46cb62c18857 100644
--- a/clang-tools-extra/clangd/ParsedAST.cpp
+++ b/clang-tools-extra/clangd/ParsedAST.cpp
@@ -656,23 +656,16 @@ ParsedAST::build(llvm::StringRef Filename, const
ParseInputs &Inputs,
: Symbol::Include;
FixIncludes.emplace(Filename, Inserter, *Inputs.Index,
/*IndexRequestLimit=*/5, Directive);
- ASTDiags.contributeFixes([&FixIncludes,
- &CTContext](const Diag &Diag,
- const clang::Diagnostic &Info) {
- auto Fixes = std::vector<Fix>();
-
- auto IncludeFixes = FixIncludes->fix(Diag.Severity, Info);
- // Ensures that if clang later introduces its own fix-it for includes
it
- // will get on our radar.
- assert((IncludeFixes.empty() || Info.getNumFixItHints() == 0) &&
- "Include-fixer replaced a note with clang fix-its attached!");
- Fixes.insert(Fixes.end(), IncludeFixes.begin(), IncludeFixes.end());
-
- auto NoLintFixes = noLintFixes(*CTContext, Info, Diag);
- Fixes.insert(Fixes.end(), NoLintFixes.begin(), NoLintFixes.end());
-
- return Fixes;
- });
+ ASTDiags.contributeFixes(
+ [&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;
+ });
Clang->setExternalSemaSource(FixIncludes->unresolvedNameRecorder());
}
}
>From e9bc7fce3012dee639843976ca1ba04e1e127c1b Mon Sep 17 00:00:00 2001
From: chomosuke <a13323...@gmail.com>
Date: Wed, 9 Oct 2024 01:07:54 +0000
Subject: [PATCH 15/19] fixed all tests
---
clang-tools-extra/clangd/Diagnostics.cpp | 8 +--
clang-tools-extra/clangd/Diagnostics.h | 10 ++-
clang-tools-extra/clangd/ParsedAST.cpp | 7 ++-
.../fixits-codeaction-documentchanges.test | 47 ++++++++++++++
.../clangd/test/fixits-codeaction.test | 41 ++++++++++++
.../test/fixits-command-documentchanges.test | 62 +++++++++++++++++++
.../clangd/test/fixits-command.test | 50 +++++++++++++++
7 files changed, 217 insertions(+), 8 deletions(-)
diff --git a/clang-tools-extra/clangd/Diagnostics.cpp
b/clang-tools-extra/clangd/Diagnostics.cpp
index c1a4fc5c9ca7b1..e42da2a9135e9b 100644
--- a/clang-tools-extra/clangd/Diagnostics.cpp
+++ b/clang-tools-extra/clangd/Diagnostics.cpp
@@ -832,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, Info);
+ if (MainFixer) {
+ auto ExtraFixes = MainFixer(*LastDiag, Info);
LastDiag->Fixes.insert(LastDiag->Fixes.end(), ExtraFixes.begin(),
ExtraFixes.end());
}
@@ -851,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, 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 4fe41fecd8ae5c..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 =
+ 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/ParsedAST.cpp
b/clang-tools-extra/clangd/ParsedAST.cpp
index 7e46cb62c18857..cfd5b53b3e292d 100644
--- a/clang-tools-extra/clangd/ParsedAST.cpp
+++ b/clang-tools-extra/clangd/ParsedAST.cpp
@@ -656,7 +656,7 @@ ParsedAST::build(llvm::StringRef Filename, const
ParseInputs &Inputs,
: Symbol::Include;
FixIncludes.emplace(Filename, Inserter, *Inputs.Index,
/*IndexRequestLimit=*/5, Directive);
- ASTDiags.contributeFixes(
+ ASTDiags.contributeMainDiagFixes(
[&FixIncludes, &CTContext](const Diag &Diag,
const clang::Diagnostic &Info) {
auto Fixes = std::vector<Fix>();
@@ -666,6 +666,11 @@ ParsedAST::build(llvm::StringRef Filename, const
ParseInputs &Inputs,
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": "(",
>From e94aa8ab723a277063b7a1e3e927bc8aa61fdc20 Mon Sep 17 00:00:00 2001
From: chomosuke <a13323...@gmail.com>
Date: Thu, 24 Oct 2024 17:03:57 +0000
Subject: [PATCH 16/19] fixed DiagLoc failes when it's within a macro
---
clang-tools-extra/clangd/NoLintFixes.cpp | 15 ++++++---------
1 file changed, 6 insertions(+), 9 deletions(-)
diff --git a/clang-tools-extra/clangd/NoLintFixes.cpp
b/clang-tools-extra/clangd/NoLintFixes.cpp
index 80e2976c8bea30..cad828b9e6910a 100644
--- a/clang-tools-extra/clangd/NoLintFixes.cpp
+++ b/clang-tools-extra/clangd/NoLintFixes.cpp
@@ -38,9 +38,8 @@
namespace clang {
namespace clangd {
-std::vector<Fix>
-noLintFixes(const clang::tidy::ClangTidyContext &CTContext,
- const clang::Diagnostic &Info, const Diag &Diag) {
+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
@@ -53,15 +52,15 @@ noLintFixes(const clang::tidy::ClangTidyContext &CTContext,
}
auto &SrcMgr = Info.getSourceManager();
- auto &DiagLoc = Info.getLocation();
+ 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 CodeTilDiag = toSourceCode(
- SrcMgr, SourceRange(SrcMgr.getLocForStartOfFile(File), 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);
@@ -94,9 +93,7 @@ noLintFixes(const clang::tidy::ClangTidyContext &CTContext,
}
const auto Regex = std::regex(NoLintFixMsgRegexStr);
-bool isNoLintFixes(const Fix &F) {
- return std::regex_match(F.Message, Regex);
-}
+bool isNoLintFixes(const Fix &F) { return std::regex_match(F.Message, Regex); }
} // namespace clangd
} // namespace clang
>From f5ecb7cb734c9e3619a867c6edfea46f3b12569e Mon Sep 17 00:00:00 2001
From: chomosuke <a13323...@gmail.com>
Date: Mon, 28 Oct 2024 05:52:29 +0000
Subject: [PATCH 17/19] TODO marked on ClangTidyDiagnosticConsumer
---
clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp | 1 +
1 file changed, 1 insertion(+)
diff --git a/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
b/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
index 4c75b422701148..edc41776d65cdf 100644
--- a/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
+++ b/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
@@ -132,6 +132,7 @@ class ClangTidyDiagnosticRenderer : public
DiagnosticRenderer {
assert(false && "Fix conflicts with existing fix!");
}
}
+ // TODO: Maybe add extra fixit here
}
void emitIncludeLocation(FullSourceLoc Loc, PresumedLoc PLoc) override {}
>From 96d305c95fa48872c9bdef8897876eeea9de00c9 Mon Sep 17 00:00:00 2001
From: chomosuke <a13323...@gmail.com>
Date: Sat, 2 Nov 2024 09:04:00 +0000
Subject: [PATCH 18/19] added 3 tests
---
.../clangd/unittests/DiagnosticsTests.cpp | 82 +++++++++++++++++++
1 file changed, 82 insertions(+)
diff --git a/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
b/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
index da5786a4de2931..e9bf45407ca02e 100644
--- a/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
+++ b/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
@@ -1717,6 +1717,88 @@ TEST(IncludeFixerTest, CImplicitFunctionDecl) {
"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) {
Annotations Main(R"cpp(
#include [["a.h"]]
>From 9b5618e8f0aeca7b001091afccaf99a63dfd4d29 Mon Sep 17 00:00:00 2001
From: chomosuke <a13323...@gmail.com>
Date: Sat, 2 Nov 2024 11:02:19 +0000
Subject: [PATCH 19/19] prep for PR
---
.../clang-tidy/ClangTidyDiagnosticConsumer.cpp | 1 -
clang-tools-extra/clangd/ClangdServer.cpp | 4 ++--
clang-tools-extra/clangd/Diagnostics.cpp | 3 ++-
clang-tools-extra/clangd/ParsedAST.cpp | 3 +--
.../clangd/unittests/ClangdLSPServerTests.cpp | 4 ++--
clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp | 7 ++-----
6 files changed, 9 insertions(+), 13 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
b/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
index edc41776d65cdf..4c75b422701148 100644
--- a/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
+++ b/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
@@ -132,7 +132,6 @@ class ClangTidyDiagnosticRenderer : public
DiagnosticRenderer {
assert(false && "Fix conflicts with existing fix!");
}
}
- // TODO: Maybe add extra fixit here
}
void emitIncludeLocation(FullSourceLoc Loc, PresumedLoc PLoc) override {}
diff --git a/clang-tools-extra/clangd/ClangdServer.cpp
b/clang-tools-extra/clangd/ClangdServer.cpp
index 925a015ae341bc..7f73b61b12c63f 100644
--- a/clang-tools-extra/clangd/ClangdServer.cpp
+++ b/clang-tools-extra/clangd/ClangdServer.cpp
@@ -63,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 {
diff --git a/clang-tools-extra/clangd/Diagnostics.cpp
b/clang-tools-extra/clangd/Diagnostics.cpp
index e42da2a9135e9b..6948e12995bed1 100644
--- a/clang-tools-extra/clangd/Diagnostics.cpp
+++ b/clang-tools-extra/clangd/Diagnostics.cpp
@@ -806,7 +806,8 @@ 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;
};
diff --git a/clang-tools-extra/clangd/ParsedAST.cpp
b/clang-tools-extra/clangd/ParsedAST.cpp
index cfd5b53b3e292d..044b6581333fce 100644
--- a/clang-tools-extra/clangd/ParsedAST.cpp
+++ b/clang-tools-extra/clangd/ParsedAST.cpp
@@ -667,8 +667,7 @@ ParsedAST::build(llvm::StringRef Filename, const
ParseInputs &Inputs,
return Fixes;
});
ASTDiags.contributeNoteDiagFixes(
- [&FixIncludes](const Diag &Diag,
- const clang::Diagnostic &Info) {
+ [&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/unittests/ClangdLSPServerTests.cpp
b/clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp
index 3593fd15d022ed..aa82ff22a76159 100644
--- a/clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp
+++ b/clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp
@@ -239,8 +239,8 @@ TEST_F(LSPTest, ClangTidyRename) {
const auto NoLintRegex = std::regex("Apply fix: " + NoLintFixMsgRegexStr);
const auto &RenameCommand = *std::find_if(
Fixes.begin(), Fixes.end(), [&](decltype(Fixes)::value_type &Fix) {
-
- return
!std::regex_match((*Fix.getAsObject())["title"].getAsString()->data(),
NoLintRegex);
+ return !std::regex_match(
+ (*Fix.getAsObject())["title"].getAsString()->data(), NoLintRegex);
});
ASSERT_EQ((*RenameCommand.getAsObject()).get("title")->getAsString(),
diff --git a/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
b/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
index e9bf45407ca02e..f1cb12d7f5626d 100644
--- a/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
+++ b/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
@@ -1295,7 +1295,6 @@ TEST(IncludeFixerTest, IncompleteType) {
{"call_incomplete_argument", "int m(ns::X); int i = m([[*x]]);"},
{"switch_incomplete_class_type", "void a() { [[switch]](*x) {} }"},
{"delete_incomplete_class_type", "void f() { [[delete]] *x; }"},
- // TODO: Add to test case
{"-Wdelete-incomplete", "void f() { [[delete]] x; }"},
{"dereference_incomplete_type",
R"cpp(void f() { asm("" : "=r"([[*]]x)::); })cpp"},
@@ -1303,7 +1302,6 @@ TEST(IncludeFixerTest, IncompleteType) {
for (auto Case : Tests) {
Annotations Main(Case.second);
TU.Code = Main.code().str() + "\n // error-ok";
- // TODO: maybe only do containsFix on -Wdelete-incomplete
EXPECT_THAT(TU.build().getDiagnostics(),
ElementsAre(AllOf(
diagName(Case.first), hasRange(Main.range()),
@@ -1669,7 +1667,6 @@ TEST(IncludeFixerTest, HeaderNamedInDiag) {
"with type 'int (const char *, ...)'; ISO C99 "
"and later do not support implicit function "
"declarations"),
- // TODO: Add to test case
containsFix(Fix(Test.range("insert"), "#include <stdio.h>\n",
"Include <stdio.h> for symbol printf")))));
@@ -2020,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
