[clang-tools-extra] Insert `// NOLINTNEXTLINE(...)` for clang-tidy diagnostics (PR #111640)
https://github.com/chomosuke created https://github.com/llvm/llvm-project/pull/111640 In some other language servers, there will be code actions that inserts warning suppression comment for the user. This PR implements that for clangd for clang-tidy diagnostics.. >From 0c18e956b9ef4c44bb2f896dd66bfec56f7a2d45 Mon Sep 17 00:00:00 2001 From: chomosuke Date: Fri, 16 Aug 2024 13:31:21 + Subject: [PATCH 01/15] 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 @@ -672,6 +673,21 @@ tryConvertToRename(const Diag *Diag, const Fix &Fix) { return std::nullopt; } +// Add NOLINT insert as code actions +std::optional 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 209e5edbeb3ed2013ac2bfb291ee8fe823b9131d Mon Sep 17 00:00:00 2001 From: chomosuke Date: Sun, 18 Aug 2024 17:15:21 + Subject: [PATCH 02/15] 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 @@ -673,21 +672,6 @@ tryConvertToRename(const Diag *Diag, const Fix &Fix) { return std::nullopt; } -// Add NOLINT insert as code actions -std::optional 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 19265f96f86e2851a63c8dba745eaf7d3663051c Mon Sep 17 00:00:00 2001 From: chomosuke Date: Mon, 19 A
[clang-tools-extra] Insert `// NOLINTNEXTLINE(...)` for clang-tidy diagnostics (PR #111640)
https://github.com/chomosuke updated https://github.com/llvm/llvm-project/pull/111640 >From e1c2a46487c42c17dc0bbfab56cde194c15e14b3 Mon Sep 17 00:00:00 2001 From: chomosuke Date: Fri, 16 Aug 2024 13:31:21 + Subject: [PATCH 01/16] 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 @@ -672,6 +673,21 @@ tryConvertToRename(const Diag *Diag, const Fix &Fix) { return std::nullopt; } +// Add NOLINT insert as code actions +std::optional 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 433040437830935d7a0822984cd5de53213b8af3 Mon Sep 17 00:00:00 2001 From: chomosuke Date: Sun, 18 Aug 2024 17:15:21 + Subject: [PATCH 02/16] 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 @@ -673,21 +672,6 @@ tryConvertToRename(const Diag *Diag, const Fix &Fix) { return std::nullopt; } -// Add NOLINT insert as code actions -std::optional 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 7b64a8fafd93af944f72b987d7abf4e167f03205 Mon Sep 17 00:00:00 2001 From: chomosuke Date: Mon, 19 Aug 2024 15:17:05 + Subject: [PATCH 03/16] inserted NOLINT with indent Retrieved CurLine and PrevLine --- clang-tools-extra/clangd/Diagnostics.cpp | 4 +- clang-tools-extra/cl
[clang-tools-extra] [clangd] Add quick fix to automatically adds NOLINTNEXTLINE comment (PR #114661)
chomosuke wrote: Thank you for the prompt review!! I am currently working on something else now but I'll address this PR asap once I'm done with that :>. https://github.com/llvm/llvm-project/pull/114661 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Add quick fix to automatically adds NOLINTNEXTLINE comment (PR #114661)
https://github.com/chomosuke converted_to_draft https://github.com/llvm/llvm-project/pull/114661 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Add quick fix to automatically adds NOLINTNEXTLINE comment (PR #114661)
https://github.com/chomosuke updated https://github.com/llvm/llvm-project/pull/114661 >From f52c645a8b0a3664c2520e70b46e248c984037ae Mon Sep 17 00:00:00 2001 From: chomosuke Date: Sat, 2 Nov 2024 09:18:38 + 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 | 3 +- clang-tools-extra/clangd/Diagnostics.cpp | 23 +++- 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| 19 ++- .../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 | 18 ++- .../clangd/unittests/DiagnosticsTests.cpp | 113 -- 13 files changed, 492 insertions(+), 32 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..7f73b61b12c63f 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" @@ -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..6948e12995bed1 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) { @@ -822,8 +833,8 @@ void StoreDiags::HandleDiagnostic(DiagnosticsEngine::Level DiagLevel, LastDiagOriginallyError = OriginallyError; if (!Info.getFixItHints().empty()) AddFix(true /* try to invent a message instead of repeating the diag */); -if (Fixer) { - auto ExtraFixes = Fixer(LastDiag->Severity, Info); +if (MainFixer) { + auto ExtraFixes = MainFixer(*LastDiag, Info); LastDiag->Fixes.insert(LastDiag->Fixes.end(), ExtraFixes.begin(), ExtraFixes.end()); } @@ -841,8 +852,8 @@ void StoreDiags::HandleDiagnostic(DiagnosticsEngine::Level DiagLevel, return; // Give include-fixer a chance to replace a note with a fix. -if (Fixer) { - auto ReplacementFixes = Fixer(LastDiag->Severity, Info); +if (NoteFixer) { + auto ReplacementFixes = NoteFixer(*LastDiag, Info); if (!ReplacementFixes.empty()) { assert(Info.getNumFixItHints() == 0 && "Include-fixer replaced a note with clang fix
[clang-tools-extra] Add quick fix to automatically adds NOLINTNEXTLINE comment (PR #114660)
https://github.com/chomosuke updated https://github.com/llvm/llvm-project/pull/114660 >From a8caf223a55b16fc4e0aaa483c4af62306137179 Mon Sep 17 00:00:00 2001 From: chomosuke Date: Sat, 2 Nov 2024 09:18:38 + 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.g
[clang-tools-extra] Add quick fix to automatically adds NOLINTNEXTLINE comment (PR #114660)
https://github.com/chomosuke updated https://github.com/llvm/llvm-project/pull/114660 >From 4c506db1c879a648ba3f0dc3851c47f597f9ad96 Mon Sep 17 00:00:00 2001 From: chomosuke Date: Sat, 2 Nov 2024 09:18:38 + 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 | 3 +- clang-tools-extra/clangd/Diagnostics.cpp | 23 +++- 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 | 113 -- 13 files changed, 492 insertions(+), 31 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..7f73b61b12c63f 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" @@ -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..6948e12995bed1 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) { @@ -822,8 +833,8 @@ void StoreDiags::HandleDiagnostic(DiagnosticsEngine::Level DiagLevel, LastDiagOriginallyError = OriginallyError; if (!Info.getFixItHints().empty()) AddFix(true /* try to invent a message instead of repeating the diag */); -if (Fixer) { - auto ExtraFixes = Fixer(LastDiag->Severity, Info); +if (MainFixer) { + auto ExtraFixes = MainFixer(*LastDiag, Info); LastDiag->Fixes.insert(LastDiag->Fixes.end(), ExtraFixes.begin(), ExtraFixes.end()); } @@ -841,8 +852,8 @@ void StoreDiags::HandleDiagnostic(DiagnosticsEngine::Level DiagLevel, return; // Give include-fixer a chance to replace a note with a fix. -if (Fixer) { - auto ReplacementFixes = Fixer(LastDiag->Severity, Info); +if (NoteFixer) { + auto ReplacementFixes = NoteFixer(*LastDiag, Info); if (!ReplacementFixes.empty()) { assert(Info.getNumFixItHints() == 0 && "Include-fixer replaced a note with clang fi
[clang-tools-extra] Add quick fix to automatically adds NOLINTNEXTLINE comment (PR #114660)
https://github.com/chomosuke updated https://github.com/llvm/llvm-project/pull/114660 >From f52c645a8b0a3664c2520e70b46e248c984037ae Mon Sep 17 00:00:00 2001 From: chomosuke Date: Sat, 2 Nov 2024 09:18:38 + 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 | 3 +- clang-tools-extra/clangd/Diagnostics.cpp | 23 +++- 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| 19 ++- .../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 | 18 ++- .../clangd/unittests/DiagnosticsTests.cpp | 113 -- 13 files changed, 492 insertions(+), 32 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..7f73b61b12c63f 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" @@ -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..6948e12995bed1 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) { @@ -822,8 +833,8 @@ void StoreDiags::HandleDiagnostic(DiagnosticsEngine::Level DiagLevel, LastDiagOriginallyError = OriginallyError; if (!Info.getFixItHints().empty()) AddFix(true /* try to invent a message instead of repeating the diag */); -if (Fixer) { - auto ExtraFixes = Fixer(LastDiag->Severity, Info); +if (MainFixer) { + auto ExtraFixes = MainFixer(*LastDiag, Info); LastDiag->Fixes.insert(LastDiag->Fixes.end(), ExtraFixes.begin(), ExtraFixes.end()); } @@ -841,8 +852,8 @@ void StoreDiags::HandleDiagnostic(DiagnosticsEngine::Level DiagLevel, return; // Give include-fixer a chance to replace a note with a fix. -if (Fixer) { - auto ReplacementFixes = Fixer(LastDiag->Severity, Info); +if (NoteFixer) { + auto ReplacementFixes = NoteFixer(*LastDiag, Info); if (!ReplacementFixes.empty()) { assert(Info.getNumFixItHints() == 0 && "Include-fixer replaced a note with clang fix
[clang-tools-extra] Add quick fix to automatically adds NOLINTNEXTLINE comment (PR #114660)
https://github.com/chomosuke created https://github.com/llvm/llvm-project/pull/114660 For some other lsp / linters. They will offer a "Ignore this error for this line" code actions for warnings. I find that convenient as I don't have to type the comment myself and copy the name of the diagnostics. This is the first time I work on such a large code base so any feedback and guidance are greatly appreciated. >From 1902b40e074d3ea461d9088a7d37a47b0e12d41e Mon Sep 17 00:00:00 2001 From: chomosuke Date: Sat, 2 Nov 2024 09:18:38 + 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 | 120 +++--- 13 files changed, 500 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_
[clang-tools-extra] Add quick fix to automatically adds NOLINTNEXTLINE comment (PR #114660)
https://github.com/chomosuke updated https://github.com/llvm/llvm-project/pull/114660 >From 3cc27f1a8880e2c31ce19d86e3bb74ce0b82fbec Mon Sep 17 00:00:00 2001 From: chomosuke Date: Sat, 2 Nov 2024 09:18:38 + 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.g
[clang-tools-extra] Add quick fix to automatically adds NOLINTNEXTLINE comment (PR #114660)
https://github.com/chomosuke updated https://github.com/llvm/llvm-project/pull/114660 >From 3cc27f1a8880e2c31ce19d86e3bb74ce0b82fbec Mon Sep 17 00:00:00 2001 From: chomosuke Date: Sat, 2 Nov 2024 09:18:38 + 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.g
[clang-tools-extra] [clangd] Add quick fix to automatically adds NOLINTNEXTLINE comment (PR #114660)
https://github.com/chomosuke edited https://github.com/llvm/llvm-project/pull/114660 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Add quick fix to automatically adds NOLINTNEXTLINE comment (PR #114660)
https://github.com/chomosuke closed https://github.com/llvm/llvm-project/pull/114660 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Add quick fix to automatically adds NOLINTNEXTLINE comment (PR #114661)
https://github.com/chomosuke created https://github.com/llvm/llvm-project/pull/114661 For some other lsp / linters. They will offer a "Ignore this error for this line" code actions for warnings. I find that convenient as I don't have to type the comment myself and copy the name of the diagnostics. This PR implements this for clangd. For all diagnostics from clang-tidy, this PR adds an extra quick fix to insert a // NOLINTNEXTLINE(diag-name) to suppress the diagnostics. This is the first time I work on such a large code base so any feedback and guidance are greatly appreciated. >From f52c645a8b0a3664c2520e70b46e248c984037ae Mon Sep 17 00:00:00 2001 From: chomosuke Date: Sat, 2 Nov 2024 09:18:38 + 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 | 3 +- clang-tools-extra/clangd/Diagnostics.cpp | 23 +++- 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| 19 ++- .../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 | 18 ++- .../clangd/unittests/DiagnosticsTests.cpp | 113 -- 13 files changed, 492 insertions(+), 32 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..7f73b61b12c63f 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" @@ -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..6948e12995bed1 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) { @@ -822,8 +833,8 @@ void StoreDiags::HandleDiagnostic(DiagnosticsEngine::Level DiagLevel, LastDiagOriginallyError = OriginallyError; if (!Info.getFixItHints().empty()) AddFix(true /* try to invent a message instead of repeating the diag */); -if (Fixer) { - auto ExtraFixes = Fixer(LastDiag->Severity, Info); +if (MainFixer) { + auto ExtraFixes = MainFixer(*LastDiag, Info); LastDiag->Fixes.insert(LastDiag->Fixes.end(), ExtraFixes.begin(), ExtraF
[clang-tools-extra] [clangd] Add quick fix to automatically adds NOLINTNEXTLINE comment (PR #114661)
https://github.com/chomosuke updated https://github.com/llvm/llvm-project/pull/114661 >From 7b70b905e5a9942f592316b407135975977c93cb Mon Sep 17 00:00:00 2001 From: chomosuke Date: Fri, 16 Aug 2024 13:31:21 + 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 @@ -672,6 +673,21 @@ tryConvertToRename(const Diag *Diag, const Fix &Fix) { return std::nullopt; } +// Add NOLINT insert as code actions +std::optional 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 Date: Sun, 18 Aug 2024 17:15:21 + 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 @@ -673,21 +672,6 @@ tryConvertToRename(const Diag *Diag, const Fix &Fix) { return std::nullopt; } -// Add NOLINT insert as code actions -std::optional 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 Date: Mon, 19 Aug 2024 15:17:05 + Subject: [PATCH 03/19] inserted NOLINT with indent Retrieved CurLine and PrevLine --- clang-tools-extra/clangd/Diagnostics.cpp | 4 +- clang-tools-extra/cl
[clang-tools-extra] [clangd] Add quick fix to automatically adds NOLINTNEXTLINE comment (PR #114660)
https://github.com/chomosuke edited https://github.com/llvm/llvm-project/pull/114660 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] Nolint fix (PR #114663)
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 Date: Fri, 16 Aug 2024 13:31:21 + 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 @@ -672,6 +673,21 @@ tryConvertToRename(const Diag *Diag, const Fix &Fix) { return std::nullopt; } +// Add NOLINT insert as code actions +std::optional 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 Date: Sun, 18 Aug 2024 17:15:21 + 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 @@ -673,21 +672,6 @@ tryConvertToRename(const Diag *Diag, const Fix &Fix) { return std::nullopt; } -// Add NOLINT insert as code actions -std::optional 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 Date: Mon, 19 Aug 2024 15:17:05 + Subject: [PATCH 03/19] inserted NOLINT with indent Retrieved CurLine and PrevLine --- clang-tools-extra/clangd/Diagnostics.cpp | 4 +- clang-tools-ex
[clang-tools-extra] Nolint fix (PR #114663)
https://github.com/chomosuke edited https://github.com/llvm/llvm-project/pull/114663 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] Nolint fix (PR #114663)
https://github.com/chomosuke updated https://github.com/llvm/llvm-project/pull/114663 >From 7b70b905e5a9942f592316b407135975977c93cb Mon Sep 17 00:00:00 2001 From: chomosuke Date: Fri, 16 Aug 2024 13:31:21 + Subject: [PATCH 01/20] 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 @@ -672,6 +673,21 @@ tryConvertToRename(const Diag *Diag, const Fix &Fix) { return std::nullopt; } +// Add NOLINT insert as code actions +std::optional 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 Date: Sun, 18 Aug 2024 17:15:21 + Subject: [PATCH 02/20] 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 @@ -673,21 +672,6 @@ tryConvertToRename(const Diag *Diag, const Fix &Fix) { return std::nullopt; } -// Add NOLINT insert as code actions -std::optional 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 Date: Mon, 19 Aug 2024 15:17:05 + Subject: [PATCH 03/20] inserted NOLINT with indent Retrieved CurLine and PrevLine --- clang-tools-extra/clangd/Diagnostics.cpp | 4 +- clang-tools-extra/cl
[clang-tools-extra] Nolint fix (PR #114663)
https://github.com/chomosuke closed https://github.com/llvm/llvm-project/pull/114663 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Add quick fix to automatically adds NOLINTNEXTLINE comment (PR #114661)
https://github.com/chomosuke updated https://github.com/llvm/llvm-project/pull/114661 >From 97a830bdf1b3eb1eb352561ef38f95e8baa4e0b9 Mon Sep 17 00:00:00 2001 From: chomosuke Date: Sat, 2 Nov 2024 09:18:38 + 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 | 3 +- clang-tools-extra/clangd/Diagnostics.cpp | 23 +++- 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| 19 ++- .../fixits-codeaction-documentchanges.test| 47 .../clangd/test/fixits-codeaction.test| 82 + .../test/fixits-command-documentchanges.test | 62 ++ .../clangd/test/fixits-command.test | 100 .../clangd/unittests/ClangdLSPServerTests.cpp | 18 ++- .../clangd/unittests/DiagnosticsTests.cpp | 113 -- 13 files changed, 583 insertions(+), 32 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..7f73b61b12c63f 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" @@ -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..6948e12995bed1 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) { @@ -822,8 +833,8 @@ void StoreDiags::HandleDiagnostic(DiagnosticsEngine::Level DiagLevel, LastDiagOriginallyError = OriginallyError; if (!Info.getFixItHints().empty()) AddFix(true /* try to invent a message instead of repeating the diag */); -if (Fixer) { - auto ExtraFixes = Fixer(LastDiag->Severity, Info); +if (MainFixer) { + auto ExtraFixes = MainFixer(*LastDiag, Info); LastDiag->Fixes.insert(LastDiag->Fixes.end(), ExtraFixes.begin(), ExtraFixes.end()); } @@ -841,8 +852,8 @@ void StoreDiags::HandleDiagnostic(DiagnosticsEngine::Level DiagLevel, return; // Give include-fixer a chance to replace a note with a fix. -if (Fixer) { - auto ReplacementFixes = Fixer(LastDiag->Severity, Info); +if (NoteFixer) { + auto ReplacementFixes = NoteFixer(*LastDiag, Info); if (!ReplacementFixes.empty()) { assert(Info.getNumFixItHints() == 0 && "Include-fixer replaced a note
[clang-tools-extra] [clangd] Add quick fix to automatically adds NOLINTNEXTLINE comment (PR #114661)
https://github.com/chomosuke updated https://github.com/llvm/llvm-project/pull/114661 >From 581004de1006ce232ad5dbbed9f8af8eb2f312ae Mon Sep 17 00:00:00 2001 From: chomosuke Date: Sat, 2 Nov 2024 09:18:38 + 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 | 3 +- clang-tools-extra/clangd/Diagnostics.cpp | 23 +++- 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| 19 ++- .../fixits-codeaction-documentchanges.test| 47 .../clangd/test/fixits-codeaction.test| 82 + .../test/fixits-command-documentchanges.test | 62 ++ .../clangd/test/fixits-command.test | 100 .../clangd/unittests/ClangdLSPServerTests.cpp | 18 ++- .../clangd/unittests/DiagnosticsTests.cpp | 113 -- 13 files changed, 583 insertions(+), 32 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..7f73b61b12c63f 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" @@ -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..6948e12995bed1 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) { @@ -822,8 +833,8 @@ void StoreDiags::HandleDiagnostic(DiagnosticsEngine::Level DiagLevel, LastDiagOriginallyError = OriginallyError; if (!Info.getFixItHints().empty()) AddFix(true /* try to invent a message instead of repeating the diag */); -if (Fixer) { - auto ExtraFixes = Fixer(LastDiag->Severity, Info); +if (MainFixer) { + auto ExtraFixes = MainFixer(*LastDiag, Info); LastDiag->Fixes.insert(LastDiag->Fixes.end(), ExtraFixes.begin(), ExtraFixes.end()); } @@ -841,8 +852,8 @@ void StoreDiags::HandleDiagnostic(DiagnosticsEngine::Level DiagLevel, return; // Give include-fixer a chance to replace a note with a fix. -if (Fixer) { - auto ReplacementFixes = Fixer(LastDiag->Severity, Info); +if (NoteFixer) { + auto ReplacementFixes = NoteFixer(*LastDiag, Info); if (!ReplacementFixes.empty()) { assert(Info.getNumFixItHints() == 0 && "Include-fixer replaced a note
[clang-tools-extra] [clangd] Add quick fix to automatically adds NOLINTNEXTLINE comment (PR #114661)
https://github.com/chomosuke updated https://github.com/llvm/llvm-project/pull/114661 >From 311c752356617d768f0bdc8599e765ae42272329 Mon Sep 17 00:00:00 2001 From: chomosuke Date: Sat, 2 Nov 2024 09:18:38 + 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 | 3 +- clang-tools-extra/clangd/Diagnostics.cpp | 23 +++- 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| 19 ++- .../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 | 18 ++- .../clangd/unittests/DiagnosticsTests.cpp | 113 -- 13 files changed, 492 insertions(+), 32 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..7f73b61b12c63f 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" @@ -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..6948e12995bed1 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) { @@ -822,8 +833,8 @@ void StoreDiags::HandleDiagnostic(DiagnosticsEngine::Level DiagLevel, LastDiagOriginallyError = OriginallyError; if (!Info.getFixItHints().empty()) AddFix(true /* try to invent a message instead of repeating the diag */); -if (Fixer) { - auto ExtraFixes = Fixer(LastDiag->Severity, Info); +if (MainFixer) { + auto ExtraFixes = MainFixer(*LastDiag, Info); LastDiag->Fixes.insert(LastDiag->Fixes.end(), ExtraFixes.begin(), ExtraFixes.end()); } @@ -841,8 +852,8 @@ void StoreDiags::HandleDiagnostic(DiagnosticsEngine::Level DiagLevel, return; // Give include-fixer a chance to replace a note with a fix. -if (Fixer) { - auto ReplacementFixes = Fixer(LastDiag->Severity, Info); +if (NoteFixer) { + auto ReplacementFixes = NoteFixer(*LastDiag, Info); if (!ReplacementFixes.empty()) { assert(Info.getNumFixItHints() == 0 && "Include-fixer replaced a note with clang fix
[clang-tools-extra] [clangd] Add quick fix to automatically adds NOLINTNEXTLINE comment (PR #114661)
https://github.com/chomosuke updated https://github.com/llvm/llvm-project/pull/114661 >From 35cbe235a9b1b40b23b0910eb1bfa42c488ab54e Mon Sep 17 00:00:00 2001 From: chomosuke Date: Sat, 2 Nov 2024 09:18:38 + 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 | 3 +- clang-tools-extra/clangd/Diagnostics.cpp | 23 +++- 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| 19 ++- .../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 | 18 ++- .../clangd/unittests/DiagnosticsTests.cpp | 113 -- 13 files changed, 492 insertions(+), 32 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..7f73b61b12c63f 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" @@ -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..6948e12995bed1 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) { @@ -822,8 +833,8 @@ void StoreDiags::HandleDiagnostic(DiagnosticsEngine::Level DiagLevel, LastDiagOriginallyError = OriginallyError; if (!Info.getFixItHints().empty()) AddFix(true /* try to invent a message instead of repeating the diag */); -if (Fixer) { - auto ExtraFixes = Fixer(LastDiag->Severity, Info); +if (MainFixer) { + auto ExtraFixes = MainFixer(*LastDiag, Info); LastDiag->Fixes.insert(LastDiag->Fixes.end(), ExtraFixes.begin(), ExtraFixes.end()); } @@ -841,8 +852,8 @@ void StoreDiags::HandleDiagnostic(DiagnosticsEngine::Level DiagLevel, return; // Give include-fixer a chance to replace a note with a fix. -if (Fixer) { - auto ReplacementFixes = Fixer(LastDiag->Severity, Info); +if (NoteFixer) { + auto ReplacementFixes = NoteFixer(*LastDiag, Info); if (!ReplacementFixes.empty()) { assert(Info.getNumFixItHints() == 0 && "Include-fixer replaced a note with clang fix
[clang-tools-extra] [clangd] Add quick fix to automatically adds NOLINTNEXTLINE comment (PR #114661)
https://github.com/chomosuke updated https://github.com/llvm/llvm-project/pull/114661 >From 311c752356617d768f0bdc8599e765ae42272329 Mon Sep 17 00:00:00 2001 From: chomosuke Date: Sat, 2 Nov 2024 09:18:38 + 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 | 3 +- clang-tools-extra/clangd/Diagnostics.cpp | 23 +++- 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| 19 ++- .../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 | 18 ++- .../clangd/unittests/DiagnosticsTests.cpp | 113 -- 13 files changed, 492 insertions(+), 32 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..7f73b61b12c63f 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" @@ -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..6948e12995bed1 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) { @@ -822,8 +833,8 @@ void StoreDiags::HandleDiagnostic(DiagnosticsEngine::Level DiagLevel, LastDiagOriginallyError = OriginallyError; if (!Info.getFixItHints().empty()) AddFix(true /* try to invent a message instead of repeating the diag */); -if (Fixer) { - auto ExtraFixes = Fixer(LastDiag->Severity, Info); +if (MainFixer) { + auto ExtraFixes = MainFixer(*LastDiag, Info); LastDiag->Fixes.insert(LastDiag->Fixes.end(), ExtraFixes.begin(), ExtraFixes.end()); } @@ -841,8 +852,8 @@ void StoreDiags::HandleDiagnostic(DiagnosticsEngine::Level DiagLevel, return; // Give include-fixer a chance to replace a note with a fix. -if (Fixer) { - auto ReplacementFixes = Fixer(LastDiag->Severity, Info); +if (NoteFixer) { + auto ReplacementFixes = NoteFixer(*LastDiag, Info); if (!ReplacementFixes.empty()) { assert(Info.getNumFixItHints() == 0 && "Include-fixer replaced a note with clang fix
[clang-tools-extra] [clangd] Add quick fix to automatically adds NOLINTNEXTLINE comment (PR #114661)
https://github.com/chomosuke ready_for_review https://github.com/llvm/llvm-project/pull/114661 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] Add quick fix to automatically adds NOLINTNEXTLINE comment (PR #114660)
https://github.com/chomosuke converted_to_draft https://github.com/llvm/llvm-project/pull/114660 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] Insert `// NOLINTNEXTLINE(...)` for clang-tidy diagnostics (PR #111640)
https://github.com/chomosuke closed https://github.com/llvm/llvm-project/pull/111640 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] Insert `// NOLINTNEXTLINE(...)` for clang-tidy diagnostics (PR #111640)
https://github.com/chomosuke updated https://github.com/llvm/llvm-project/pull/111640 >From e1c2a46487c42c17dc0bbfab56cde194c15e14b3 Mon Sep 17 00:00:00 2001 From: chomosuke Date: Fri, 16 Aug 2024 13:31:21 + Subject: [PATCH 01/17] 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 @@ -672,6 +673,21 @@ tryConvertToRename(const Diag *Diag, const Fix &Fix) { return std::nullopt; } +// Add NOLINT insert as code actions +std::optional 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 433040437830935d7a0822984cd5de53213b8af3 Mon Sep 17 00:00:00 2001 From: chomosuke Date: Sun, 18 Aug 2024 17:15:21 + Subject: [PATCH 02/17] 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 @@ -673,21 +672,6 @@ tryConvertToRename(const Diag *Diag, const Fix &Fix) { return std::nullopt; } -// Add NOLINT insert as code actions -std::optional 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 7b64a8fafd93af944f72b987d7abf4e167f03205 Mon Sep 17 00:00:00 2001 From: chomosuke Date: Mon, 19 Aug 2024 15:17:05 + Subject: [PATCH 03/17] inserted NOLINT with indent Retrieved CurLine and PrevLine --- clang-tools-extra/clangd/Diagnostics.cpp | 4 +- clang-tools-extra/cl
[clang-tools-extra] Insert `// NOLINTNEXTLINE(...)` for clang-tidy diagnostics (PR #111640)
https://github.com/chomosuke updated https://github.com/llvm/llvm-project/pull/111640 >From e1c2a46487c42c17dc0bbfab56cde194c15e14b3 Mon Sep 17 00:00:00 2001 From: chomosuke Date: Fri, 16 Aug 2024 13:31:21 + Subject: [PATCH 01/15] 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 @@ -672,6 +673,21 @@ tryConvertToRename(const Diag *Diag, const Fix &Fix) { return std::nullopt; } +// Add NOLINT insert as code actions +std::optional 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 433040437830935d7a0822984cd5de53213b8af3 Mon Sep 17 00:00:00 2001 From: chomosuke Date: Sun, 18 Aug 2024 17:15:21 + Subject: [PATCH 02/15] 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 @@ -673,21 +672,6 @@ tryConvertToRename(const Diag *Diag, const Fix &Fix) { return std::nullopt; } -// Add NOLINT insert as code actions -std::optional 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 7b64a8fafd93af944f72b987d7abf4e167f03205 Mon Sep 17 00:00:00 2001 From: chomosuke Date: Mon, 19 Aug 2024 15:17:05 + Subject: [PATCH 03/15] inserted NOLINT with indent Retrieved CurLine and PrevLine --- clang-tools-extra/clangd/Diagnostics.cpp | 4 +- clang-tools-extra/cl
[clang-tools-extra] [clangd] Add quick fix to automatically adds NOLINTNEXTLINE comment (PR #114661)
chomosuke wrote: Hi @kadircet, I have just started looking into `FeatureModule` and could not find any example of it being used apart from testing. Just want to confirm that this infrastructure isn't currently being used by anything in Clangd. I assume I should extend `FeatureModule` to add my NOLINT fix feature and add it in `ClangMain.cpp` based on command line options. Would that be the correct approach? https://github.com/llvm/llvm-project/pull/114661 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Add quick fix to automatically adds NOLINTNEXTLINE comment (PR #114661)
https://github.com/chomosuke closed https://github.com/llvm/llvm-project/pull/114661 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd][clang-tidy] Make clangd run `format::cleanupAroundReplacements()` for all code actions just as clang-tidy does (PR #118569)
https://github.com/chomosuke updated https://github.com/llvm/llvm-project/pull/118569 >From d2257eb43bbd9ce2dfd8a13123c2048e1cf8b439 Mon Sep 17 00:00:00 2001 From: chomosuke Date: Wed, 4 Dec 2024 00:07:31 + Subject: [PATCH 1/3] clangd cleanupAroundReplacements just as clang-tidy does --- clang-tools-extra/clangd/Diagnostics.cpp | 48 +++- 1 file changed, 46 insertions(+), 2 deletions(-) diff --git a/clang-tools-extra/clangd/Diagnostics.cpp b/clang-tools-extra/clangd/Diagnostics.cpp index a59d1e7ac84096..af8d0d534af689 100644 --- a/clang-tools-extra/clangd/Diagnostics.cpp +++ b/clang-tools-extra/clangd/Diagnostics.cpp @@ -19,6 +19,7 @@ #include "clang/Basic/SourceLocation.h" #include "clang/Basic/SourceManager.h" #include "clang/Basic/TokenKinds.h" +#include "clang/Format/Format.h" #include "clang/Lex/Lexer.h" #include "clang/Lex/Token.h" #include "llvm/ADT/ArrayRef.h" @@ -741,7 +742,7 @@ void StoreDiags::HandleDiagnostic(DiagnosticsEngine::Level DiagLevel, return false; // Copy as we may modify the ranges. auto FixIts = Info.getFixItHints().vec(); -llvm::SmallVector Edits; +auto Replacements = std::make_optional(); for (auto &FixIt : FixIts) { // Allow fixits within a single macro-arg expansion to be applied. // This can be incorrect if the argument is expanded multiple times in @@ -761,7 +762,50 @@ void StoreDiags::HandleDiagnostic(DiagnosticsEngine::Level DiagLevel, return false; if (!isInsideMainFile(FixIt.RemoveRange.getBegin(), SM)) return false; - Edits.push_back(toTextEdit(FixIt, SM, *LangOpts)); + + auto R = tooling::Replacement(SM, FixIt.RemoveRange, FixIt.CodeToInsert, +*LangOpts); + auto Err = Replacements->add(R); + if (Err) { +// FIXME: Remove duplicated code as in ClangTidy.cpp +unsigned NewOffset = +Replacements->getShiftedCodePosition(R.getOffset()); +unsigned NewLength = Replacements->getShiftedCodePosition( + R.getOffset() + R.getLength()) - + NewOffset; +if (NewLength == R.getLength()) { + llvm::consumeError(std::move(Err)); + R = tooling::Replacement(R.getFilePath(), NewOffset, NewLength, + R.getReplacementText()); + Replacements = Replacements->merge(tooling::Replacements(R)); +} else { + log("Skipping formatting the replacement due to conflict: {0}", + llvm::toString(std::move(Err))); + Replacements = std::nullopt; + break; +} + } +} + +llvm::SmallVector Edits; + +if (Replacements) { + StringRef Code = SM.getBufferData(SM.getMainFileID()); + auto Repl = format::cleanupAroundReplacements(Code, *Replacements, +format::getNoStyle()); + if (!Repl) { +log("Skipping formatting the replacement due to conflict: {0}", + llvm::toString(std::move(Repl.takeError(; +Replacements = std::nullopt; + } else { +auto Es = replacementsToEdits(Code, *Repl); +Edits.append(Es.begin(), Es.end()); + } +} +if (!Replacements) { + for (auto &FixIt : FixIts) { +Edits.push_back(toTextEdit(FixIt, SM, *LangOpts)); + } } llvm::SmallString<64> Message; >From cbdc1e25e3a39472c068a742b0f40e9c71221521 Mon Sep 17 00:00:00 2001 From: chomosuke Date: Wed, 4 Dec 2024 01:15:57 + Subject: [PATCH 2/3] fixed tests --- .../clangd/unittests/DiagnosticsTests.cpp| 16 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp b/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp index 7a47d6ebebf3b2..94765b48099eda 100644 --- a/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp +++ b/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp @@ -876,17 +876,17 @@ TEST(DiagnosticTest, ClangTidySelfContainedDiags) { clangd::Fix ExpectedCFix; ExpectedCFix.Message = "variable 'C' is not initialized"; - ExpectedCFix.Edits.push_back(TextEdit{Main.range("CFix"), " = NAN"}); ExpectedCFix.Edits.push_back( TextEdit{Main.range("MathHeader"), "#include \n\n"}); + ExpectedCFix.Edits.push_back(TextEdit{Main.range("CFix"), " = NAN"}); // Again in clang-tidy only the include directive would be emitted for the // first warning. However we need the include attaching for both warnings. clangd::Fix ExpectedDFix; ExpectedDFix.Message = "variable 'D' is not initialized"; - ExpectedDFix.Edits.push_back(TextEdit{Main.range("DFix"), " = NAN"}); ExpectedDFix.Edits.push_back( TextEdit{Main.range("MathHeader"), "#include \n\n"}); + ExpectedDFix.Edits.push_back(TextEdit{Main.range("DFix"), " = NAN"}); EXPECT_THAT( TU.build().getDiagnostics(), ifT
[clang] [clang-tools-extra] [clangd][clang-tidy] Make clangd run `format::cleanupAroundReplacements()` for all code actions just as clang-tidy does (PR #118569)
https://github.com/chomosuke updated https://github.com/llvm/llvm-project/pull/118569 >From d2257eb43bbd9ce2dfd8a13123c2048e1cf8b439 Mon Sep 17 00:00:00 2001 From: chomosuke Date: Wed, 4 Dec 2024 00:07:31 + Subject: [PATCH 1/4] clangd cleanupAroundReplacements just as clang-tidy does --- clang-tools-extra/clangd/Diagnostics.cpp | 48 +++- 1 file changed, 46 insertions(+), 2 deletions(-) diff --git a/clang-tools-extra/clangd/Diagnostics.cpp b/clang-tools-extra/clangd/Diagnostics.cpp index a59d1e7ac84096..af8d0d534af689 100644 --- a/clang-tools-extra/clangd/Diagnostics.cpp +++ b/clang-tools-extra/clangd/Diagnostics.cpp @@ -19,6 +19,7 @@ #include "clang/Basic/SourceLocation.h" #include "clang/Basic/SourceManager.h" #include "clang/Basic/TokenKinds.h" +#include "clang/Format/Format.h" #include "clang/Lex/Lexer.h" #include "clang/Lex/Token.h" #include "llvm/ADT/ArrayRef.h" @@ -741,7 +742,7 @@ void StoreDiags::HandleDiagnostic(DiagnosticsEngine::Level DiagLevel, return false; // Copy as we may modify the ranges. auto FixIts = Info.getFixItHints().vec(); -llvm::SmallVector Edits; +auto Replacements = std::make_optional(); for (auto &FixIt : FixIts) { // Allow fixits within a single macro-arg expansion to be applied. // This can be incorrect if the argument is expanded multiple times in @@ -761,7 +762,50 @@ void StoreDiags::HandleDiagnostic(DiagnosticsEngine::Level DiagLevel, return false; if (!isInsideMainFile(FixIt.RemoveRange.getBegin(), SM)) return false; - Edits.push_back(toTextEdit(FixIt, SM, *LangOpts)); + + auto R = tooling::Replacement(SM, FixIt.RemoveRange, FixIt.CodeToInsert, +*LangOpts); + auto Err = Replacements->add(R); + if (Err) { +// FIXME: Remove duplicated code as in ClangTidy.cpp +unsigned NewOffset = +Replacements->getShiftedCodePosition(R.getOffset()); +unsigned NewLength = Replacements->getShiftedCodePosition( + R.getOffset() + R.getLength()) - + NewOffset; +if (NewLength == R.getLength()) { + llvm::consumeError(std::move(Err)); + R = tooling::Replacement(R.getFilePath(), NewOffset, NewLength, + R.getReplacementText()); + Replacements = Replacements->merge(tooling::Replacements(R)); +} else { + log("Skipping formatting the replacement due to conflict: {0}", + llvm::toString(std::move(Err))); + Replacements = std::nullopt; + break; +} + } +} + +llvm::SmallVector Edits; + +if (Replacements) { + StringRef Code = SM.getBufferData(SM.getMainFileID()); + auto Repl = format::cleanupAroundReplacements(Code, *Replacements, +format::getNoStyle()); + if (!Repl) { +log("Skipping formatting the replacement due to conflict: {0}", + llvm::toString(std::move(Repl.takeError(; +Replacements = std::nullopt; + } else { +auto Es = replacementsToEdits(Code, *Repl); +Edits.append(Es.begin(), Es.end()); + } +} +if (!Replacements) { + for (auto &FixIt : FixIts) { +Edits.push_back(toTextEdit(FixIt, SM, *LangOpts)); + } } llvm::SmallString<64> Message; >From cbdc1e25e3a39472c068a742b0f40e9c71221521 Mon Sep 17 00:00:00 2001 From: chomosuke Date: Wed, 4 Dec 2024 01:15:57 + Subject: [PATCH 2/4] fixed tests --- .../clangd/unittests/DiagnosticsTests.cpp| 16 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp b/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp index 7a47d6ebebf3b2..94765b48099eda 100644 --- a/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp +++ b/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp @@ -876,17 +876,17 @@ TEST(DiagnosticTest, ClangTidySelfContainedDiags) { clangd::Fix ExpectedCFix; ExpectedCFix.Message = "variable 'C' is not initialized"; - ExpectedCFix.Edits.push_back(TextEdit{Main.range("CFix"), " = NAN"}); ExpectedCFix.Edits.push_back( TextEdit{Main.range("MathHeader"), "#include \n\n"}); + ExpectedCFix.Edits.push_back(TextEdit{Main.range("CFix"), " = NAN"}); // Again in clang-tidy only the include directive would be emitted for the // first warning. However we need the include attaching for both warnings. clangd::Fix ExpectedDFix; ExpectedDFix.Message = "variable 'D' is not initialized"; - ExpectedDFix.Edits.push_back(TextEdit{Main.range("DFix"), " = NAN"}); ExpectedDFix.Edits.push_back( TextEdit{Main.range("MathHeader"), "#include \n\n"}); + ExpectedDFix.Edits.push_back(TextEdit{Main.range("DFix"), " = NAN"}); EXPECT_THAT( TU.build().getDiagnostics(), ifT
[clang] [clang-tools-extra] [clangd][clang-tidy] Make clangd run `format::cleanupAroundReplacements()` for all code actions just as clang-tidy does (PR #118569)
https://github.com/chomosuke updated https://github.com/llvm/llvm-project/pull/118569 >From d2257eb43bbd9ce2dfd8a13123c2048e1cf8b439 Mon Sep 17 00:00:00 2001 From: chomosuke Date: Wed, 4 Dec 2024 00:07:31 + Subject: [PATCH 1/4] clangd cleanupAroundReplacements just as clang-tidy does --- clang-tools-extra/clangd/Diagnostics.cpp | 48 +++- 1 file changed, 46 insertions(+), 2 deletions(-) diff --git a/clang-tools-extra/clangd/Diagnostics.cpp b/clang-tools-extra/clangd/Diagnostics.cpp index a59d1e7ac84096..af8d0d534af689 100644 --- a/clang-tools-extra/clangd/Diagnostics.cpp +++ b/clang-tools-extra/clangd/Diagnostics.cpp @@ -19,6 +19,7 @@ #include "clang/Basic/SourceLocation.h" #include "clang/Basic/SourceManager.h" #include "clang/Basic/TokenKinds.h" +#include "clang/Format/Format.h" #include "clang/Lex/Lexer.h" #include "clang/Lex/Token.h" #include "llvm/ADT/ArrayRef.h" @@ -741,7 +742,7 @@ void StoreDiags::HandleDiagnostic(DiagnosticsEngine::Level DiagLevel, return false; // Copy as we may modify the ranges. auto FixIts = Info.getFixItHints().vec(); -llvm::SmallVector Edits; +auto Replacements = std::make_optional(); for (auto &FixIt : FixIts) { // Allow fixits within a single macro-arg expansion to be applied. // This can be incorrect if the argument is expanded multiple times in @@ -761,7 +762,50 @@ void StoreDiags::HandleDiagnostic(DiagnosticsEngine::Level DiagLevel, return false; if (!isInsideMainFile(FixIt.RemoveRange.getBegin(), SM)) return false; - Edits.push_back(toTextEdit(FixIt, SM, *LangOpts)); + + auto R = tooling::Replacement(SM, FixIt.RemoveRange, FixIt.CodeToInsert, +*LangOpts); + auto Err = Replacements->add(R); + if (Err) { +// FIXME: Remove duplicated code as in ClangTidy.cpp +unsigned NewOffset = +Replacements->getShiftedCodePosition(R.getOffset()); +unsigned NewLength = Replacements->getShiftedCodePosition( + R.getOffset() + R.getLength()) - + NewOffset; +if (NewLength == R.getLength()) { + llvm::consumeError(std::move(Err)); + R = tooling::Replacement(R.getFilePath(), NewOffset, NewLength, + R.getReplacementText()); + Replacements = Replacements->merge(tooling::Replacements(R)); +} else { + log("Skipping formatting the replacement due to conflict: {0}", + llvm::toString(std::move(Err))); + Replacements = std::nullopt; + break; +} + } +} + +llvm::SmallVector Edits; + +if (Replacements) { + StringRef Code = SM.getBufferData(SM.getMainFileID()); + auto Repl = format::cleanupAroundReplacements(Code, *Replacements, +format::getNoStyle()); + if (!Repl) { +log("Skipping formatting the replacement due to conflict: {0}", + llvm::toString(std::move(Repl.takeError(; +Replacements = std::nullopt; + } else { +auto Es = replacementsToEdits(Code, *Repl); +Edits.append(Es.begin(), Es.end()); + } +} +if (!Replacements) { + for (auto &FixIt : FixIts) { +Edits.push_back(toTextEdit(FixIt, SM, *LangOpts)); + } } llvm::SmallString<64> Message; >From cbdc1e25e3a39472c068a742b0f40e9c71221521 Mon Sep 17 00:00:00 2001 From: chomosuke Date: Wed, 4 Dec 2024 01:15:57 + Subject: [PATCH 2/4] fixed tests --- .../clangd/unittests/DiagnosticsTests.cpp| 16 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp b/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp index 7a47d6ebebf3b2..94765b48099eda 100644 --- a/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp +++ b/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp @@ -876,17 +876,17 @@ TEST(DiagnosticTest, ClangTidySelfContainedDiags) { clangd::Fix ExpectedCFix; ExpectedCFix.Message = "variable 'C' is not initialized"; - ExpectedCFix.Edits.push_back(TextEdit{Main.range("CFix"), " = NAN"}); ExpectedCFix.Edits.push_back( TextEdit{Main.range("MathHeader"), "#include \n\n"}); + ExpectedCFix.Edits.push_back(TextEdit{Main.range("CFix"), " = NAN"}); // Again in clang-tidy only the include directive would be emitted for the // first warning. However we need the include attaching for both warnings. clangd::Fix ExpectedDFix; ExpectedDFix.Message = "variable 'D' is not initialized"; - ExpectedDFix.Edits.push_back(TextEdit{Main.range("DFix"), " = NAN"}); ExpectedDFix.Edits.push_back( TextEdit{Main.range("MathHeader"), "#include \n\n"}); + ExpectedDFix.Edits.push_back(TextEdit{Main.range("DFix"), " = NAN"}); EXPECT_THAT( TU.build().getDiagnostics(), ifT
[clang-tools-extra] [clang-tidy][clangd] Fixed removeFunctionArgs don't remove comma for use-ranges check (PR #118568)
https://github.com/chomosuke updated https://github.com/llvm/llvm-project/pull/118568 >From b43a2602025bdacea06ced5171904fb5d765de9f Mon Sep 17 00:00:00 2001 From: chomosuke Date: Tue, 3 Dec 2024 07:10:33 + Subject: [PATCH] fixed removeFunctionArgs don't remove comma --- .../clang-tidy/utils/UseRangesCheck.cpp | 19 ++- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/clang-tools-extra/clang-tidy/utils/UseRangesCheck.cpp b/clang-tools-extra/clang-tidy/utils/UseRangesCheck.cpp index aba4d17ccd035e..88cba70b931d5d 100644 --- a/clang-tools-extra/clang-tidy/utils/UseRangesCheck.cpp +++ b/clang-tools-extra/clang-tidy/utils/UseRangesCheck.cpp @@ -28,6 +28,7 @@ #include "llvm/ADT/Twine.h" #include "llvm/Support/raw_ostream.h" #include +#include #include #include @@ -173,21 +174,21 @@ static void removeFunctionArgs(DiagnosticBuilder &Diag, const CallExpr &Call, for (unsigned Index : Sorted) { const Expr *Arg = Call.getArg(Index); if (Commas[Index]) { - if (Index >= Commas.size()) { -Diag << FixItHint::CreateRemoval(Arg->getSourceRange()); - } else { + if (Index + 1 < Call.getNumArgs()) { // Remove the next comma Commas[Index + 1] = true; +const Expr *NextArg = Call.getArg(Index + 1); Diag << FixItHint::CreateRemoval(CharSourceRange::getTokenRange( -{Arg->getBeginLoc(), - Lexer::getLocForEndOfToken( - Arg->getEndLoc(), 0, Ctx.getSourceManager(), Ctx.getLangOpts()) - .getLocWithOffset(1)})); +{Arg->getBeginLoc(), NextArg->getBeginLoc().getLocWithOffset(-1)})); + } else { +Diag << FixItHint::CreateRemoval(Arg->getSourceRange()); } } else { - Diag << FixItHint::CreateRemoval(CharSourceRange::getTokenRange( - Arg->getBeginLoc().getLocWithOffset(-1), Arg->getEndLoc())); + // At this point we know Index > 0 because `Commas[0] = true` earlier Commas[Index] = true; + const Expr *PrevArg = Call.getArg(Index - 1); + Diag << FixItHint::CreateRemoval(CharSourceRange::getTokenRange( + PrevArg->getEndLoc().getLocWithOffset(1), Arg->getEndLoc())); } } } ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [clangd][clang-tidy] Make clangd run `format::cleanupAroundReplacements()` for all code actions just as clang-tidy does (PR #118569)
https://github.com/chomosuke edited https://github.com/llvm/llvm-project/pull/118569 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [clangd][clang-tidy] Make clangd run `format::cleanupAroundReplacements()` for all code actions just as clang-tidy does (PR #118569)
https://github.com/chomosuke updated https://github.com/llvm/llvm-project/pull/118569 >From d2257eb43bbd9ce2dfd8a13123c2048e1cf8b439 Mon Sep 17 00:00:00 2001 From: chomosuke Date: Wed, 4 Dec 2024 00:07:31 + Subject: [PATCH 1/4] clangd cleanupAroundReplacements just as clang-tidy does --- clang-tools-extra/clangd/Diagnostics.cpp | 48 +++- 1 file changed, 46 insertions(+), 2 deletions(-) diff --git a/clang-tools-extra/clangd/Diagnostics.cpp b/clang-tools-extra/clangd/Diagnostics.cpp index a59d1e7ac84096..af8d0d534af689 100644 --- a/clang-tools-extra/clangd/Diagnostics.cpp +++ b/clang-tools-extra/clangd/Diagnostics.cpp @@ -19,6 +19,7 @@ #include "clang/Basic/SourceLocation.h" #include "clang/Basic/SourceManager.h" #include "clang/Basic/TokenKinds.h" +#include "clang/Format/Format.h" #include "clang/Lex/Lexer.h" #include "clang/Lex/Token.h" #include "llvm/ADT/ArrayRef.h" @@ -741,7 +742,7 @@ void StoreDiags::HandleDiagnostic(DiagnosticsEngine::Level DiagLevel, return false; // Copy as we may modify the ranges. auto FixIts = Info.getFixItHints().vec(); -llvm::SmallVector Edits; +auto Replacements = std::make_optional(); for (auto &FixIt : FixIts) { // Allow fixits within a single macro-arg expansion to be applied. // This can be incorrect if the argument is expanded multiple times in @@ -761,7 +762,50 @@ void StoreDiags::HandleDiagnostic(DiagnosticsEngine::Level DiagLevel, return false; if (!isInsideMainFile(FixIt.RemoveRange.getBegin(), SM)) return false; - Edits.push_back(toTextEdit(FixIt, SM, *LangOpts)); + + auto R = tooling::Replacement(SM, FixIt.RemoveRange, FixIt.CodeToInsert, +*LangOpts); + auto Err = Replacements->add(R); + if (Err) { +// FIXME: Remove duplicated code as in ClangTidy.cpp +unsigned NewOffset = +Replacements->getShiftedCodePosition(R.getOffset()); +unsigned NewLength = Replacements->getShiftedCodePosition( + R.getOffset() + R.getLength()) - + NewOffset; +if (NewLength == R.getLength()) { + llvm::consumeError(std::move(Err)); + R = tooling::Replacement(R.getFilePath(), NewOffset, NewLength, + R.getReplacementText()); + Replacements = Replacements->merge(tooling::Replacements(R)); +} else { + log("Skipping formatting the replacement due to conflict: {0}", + llvm::toString(std::move(Err))); + Replacements = std::nullopt; + break; +} + } +} + +llvm::SmallVector Edits; + +if (Replacements) { + StringRef Code = SM.getBufferData(SM.getMainFileID()); + auto Repl = format::cleanupAroundReplacements(Code, *Replacements, +format::getNoStyle()); + if (!Repl) { +log("Skipping formatting the replacement due to conflict: {0}", + llvm::toString(std::move(Repl.takeError(; +Replacements = std::nullopt; + } else { +auto Es = replacementsToEdits(Code, *Repl); +Edits.append(Es.begin(), Es.end()); + } +} +if (!Replacements) { + for (auto &FixIt : FixIts) { +Edits.push_back(toTextEdit(FixIt, SM, *LangOpts)); + } } llvm::SmallString<64> Message; >From cbdc1e25e3a39472c068a742b0f40e9c71221521 Mon Sep 17 00:00:00 2001 From: chomosuke Date: Wed, 4 Dec 2024 01:15:57 + Subject: [PATCH 2/4] fixed tests --- .../clangd/unittests/DiagnosticsTests.cpp| 16 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp b/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp index 7a47d6ebebf3b2..94765b48099eda 100644 --- a/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp +++ b/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp @@ -876,17 +876,17 @@ TEST(DiagnosticTest, ClangTidySelfContainedDiags) { clangd::Fix ExpectedCFix; ExpectedCFix.Message = "variable 'C' is not initialized"; - ExpectedCFix.Edits.push_back(TextEdit{Main.range("CFix"), " = NAN"}); ExpectedCFix.Edits.push_back( TextEdit{Main.range("MathHeader"), "#include \n\n"}); + ExpectedCFix.Edits.push_back(TextEdit{Main.range("CFix"), " = NAN"}); // Again in clang-tidy only the include directive would be emitted for the // first warning. However we need the include attaching for both warnings. clangd::Fix ExpectedDFix; ExpectedDFix.Message = "variable 'D' is not initialized"; - ExpectedDFix.Edits.push_back(TextEdit{Main.range("DFix"), " = NAN"}); ExpectedDFix.Edits.push_back( TextEdit{Main.range("MathHeader"), "#include \n\n"}); + ExpectedDFix.Edits.push_back(TextEdit{Main.range("DFix"), " = NAN"}); EXPECT_THAT( TU.build().getDiagnostics(), ifT
[clang-tools-extra] [clang-tidy][clangd] Fixed removeFunctionArgs don't remove comma for use-ranges check (PR #118568)
chomosuke wrote: General cased fixed in #118569 https://github.com/llvm/llvm-project/pull/118568 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [clangd][clang-tidy] Make clangd run `format::cleanupAroundReplacements()` for all code actions just as clang-tidy does (PR #118569)
https://github.com/chomosuke updated https://github.com/llvm/llvm-project/pull/118569 >From efc17a803c9c22543de7d5f9e960a7267ade1f2e Mon Sep 17 00:00:00 2001 From: chomosuke Date: Wed, 4 Dec 2024 14:42:24 + Subject: [PATCH] [clangd][clang-tidy] Make clangd run `format::cleanupAroundReplacements()` for all code actions just as clang-tidy does --- clang-tools-extra/clang-tidy/ClangTidy.cpp| 22 ++-- clang-tools-extra/clangd/Diagnostics.cpp | 35 - .../clangd/unittests/DiagnosticsTests.cpp | 51 --- .../include/clang/Tooling/Core/Replacement.h | 4 ++ clang/lib/Tooling/Core/Replacement.cpp| 16 +- 5 files changed, 99 insertions(+), 29 deletions(-) diff --git a/clang-tools-extra/clang-tidy/ClangTidy.cpp b/clang-tools-extra/clang-tidy/ClangTidy.cpp index 9c8c93c5d16c72..82331c724eaaf2 100644 --- a/clang-tools-extra/clang-tidy/ClangTidy.cpp +++ b/clang-tools-extra/clang-tidy/ClangTidy.cpp @@ -149,26 +149,12 @@ class ErrorReporter { Repl.getLength(), Repl.getReplacementText()); auto &Entry = FileReplacements[R.getFilePath()]; Replacements &Replacements = Entry.Replaces; -llvm::Error Err = Replacements.add(R); +llvm::Error Err = Replacements.addOrMerge(R); if (Err) { // FIXME: Implement better conflict handling. - llvm::errs() << "Trying to resolve conflict: " - << llvm::toString(std::move(Err)) << "\n"; - unsigned NewOffset = - Replacements.getShiftedCodePosition(R.getOffset()); - unsigned NewLength = Replacements.getShiftedCodePosition( - R.getOffset() + R.getLength()) - - NewOffset; - if (NewLength == R.getLength()) { -R = Replacement(R.getFilePath(), NewOffset, NewLength, -R.getReplacementText()); -Replacements = Replacements.merge(tooling::Replacements(R)); -CanBeApplied = true; -++AppliedFixes; - } else { -llvm::errs() -<< "Can't resolve conflict, skipping the replacement.\n"; - } + llvm::errs() + << "Can't resolve conflict, skipping the replacement: " + << llvm::toString(std::move(Err)) << '\n'; } else { CanBeApplied = true; ++AppliedFixes; diff --git a/clang-tools-extra/clangd/Diagnostics.cpp b/clang-tools-extra/clangd/Diagnostics.cpp index a59d1e7ac84096..60c6ac7256b58c 100644 --- a/clang-tools-extra/clangd/Diagnostics.cpp +++ b/clang-tools-extra/clangd/Diagnostics.cpp @@ -19,6 +19,7 @@ #include "clang/Basic/SourceLocation.h" #include "clang/Basic/SourceManager.h" #include "clang/Basic/TokenKinds.h" +#include "clang/Format/Format.h" #include "clang/Lex/Lexer.h" #include "clang/Lex/Token.h" #include "llvm/ADT/ArrayRef.h" @@ -741,7 +742,7 @@ void StoreDiags::HandleDiagnostic(DiagnosticsEngine::Level DiagLevel, return false; // Copy as we may modify the ranges. auto FixIts = Info.getFixItHints().vec(); -llvm::SmallVector Edits; +auto Replacements = std::make_optional(); for (auto &FixIt : FixIts) { // Allow fixits within a single macro-arg expansion to be applied. // This can be incorrect if the argument is expanded multiple times in @@ -761,7 +762,37 @@ void StoreDiags::HandleDiagnostic(DiagnosticsEngine::Level DiagLevel, return false; if (!isInsideMainFile(FixIt.RemoveRange.getBegin(), SM)) return false; - Edits.push_back(toTextEdit(FixIt, SM, *LangOpts)); + + auto R = tooling::Replacement(SM, FixIt.RemoveRange, FixIt.CodeToInsert, +*LangOpts); + auto Err = Replacements->addOrMerge(R); + if (Err) { +log("Skipping formatting the replacement due to conflict: {0}", +llvm::toString(std::move(Err))); +Replacements = std::nullopt; +break; + } +} + +llvm::SmallVector Edits; + +if (Replacements) { + StringRef Code = SM.getBufferData(SM.getMainFileID()); + auto Repl = format::cleanupAroundReplacements(Code, *Replacements, +format::getNoStyle()); + if (!Repl) { +log("Skipping formatting the replacement due to conflict: {0}", +llvm::toString(std::move(Repl.takeError(; +Replacements = std::nullopt; + } else { +auto Es = replacementsToEdits(Code, *Repl); +Edits.append(Es.begin(), Es.end()); + } +} +if (!Replacements) { + for (auto &FixIt : FixIts) { +Edits.push_back(toTextEdit(FixIt, SM, *LangOpts)); + } } llvm::SmallString<64> Message; diff --git a/clang-tools-extra/clangd/unittests/
[clang] [clang-tools-extra] [clangd][clang-tidy] Make clangd run `format::cleanupAroundReplacements()` for all code actions just as clang-tidy does (PR #118569)
https://github.com/chomosuke ready_for_review https://github.com/llvm/llvm-project/pull/118569 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [clangd][clang-tidy] Make clangd run `format::cleanupAroundReplacements()` for all code actions just as clang-tidy does (PR #118569)
https://github.com/chomosuke updated https://github.com/llvm/llvm-project/pull/118569 >From efc17a803c9c22543de7d5f9e960a7267ade1f2e Mon Sep 17 00:00:00 2001 From: chomosuke Date: Wed, 4 Dec 2024 14:42:24 + Subject: [PATCH 1/2] [clangd][clang-tidy] Make clangd run `format::cleanupAroundReplacements()` for all code actions just as clang-tidy does --- clang-tools-extra/clang-tidy/ClangTidy.cpp| 22 ++-- clang-tools-extra/clangd/Diagnostics.cpp | 35 - .../clangd/unittests/DiagnosticsTests.cpp | 51 --- .../include/clang/Tooling/Core/Replacement.h | 4 ++ clang/lib/Tooling/Core/Replacement.cpp| 16 +- 5 files changed, 99 insertions(+), 29 deletions(-) diff --git a/clang-tools-extra/clang-tidy/ClangTidy.cpp b/clang-tools-extra/clang-tidy/ClangTidy.cpp index 9c8c93c5d16c72..82331c724eaaf2 100644 --- a/clang-tools-extra/clang-tidy/ClangTidy.cpp +++ b/clang-tools-extra/clang-tidy/ClangTidy.cpp @@ -149,26 +149,12 @@ class ErrorReporter { Repl.getLength(), Repl.getReplacementText()); auto &Entry = FileReplacements[R.getFilePath()]; Replacements &Replacements = Entry.Replaces; -llvm::Error Err = Replacements.add(R); +llvm::Error Err = Replacements.addOrMerge(R); if (Err) { // FIXME: Implement better conflict handling. - llvm::errs() << "Trying to resolve conflict: " - << llvm::toString(std::move(Err)) << "\n"; - unsigned NewOffset = - Replacements.getShiftedCodePosition(R.getOffset()); - unsigned NewLength = Replacements.getShiftedCodePosition( - R.getOffset() + R.getLength()) - - NewOffset; - if (NewLength == R.getLength()) { -R = Replacement(R.getFilePath(), NewOffset, NewLength, -R.getReplacementText()); -Replacements = Replacements.merge(tooling::Replacements(R)); -CanBeApplied = true; -++AppliedFixes; - } else { -llvm::errs() -<< "Can't resolve conflict, skipping the replacement.\n"; - } + llvm::errs() + << "Can't resolve conflict, skipping the replacement: " + << llvm::toString(std::move(Err)) << '\n'; } else { CanBeApplied = true; ++AppliedFixes; diff --git a/clang-tools-extra/clangd/Diagnostics.cpp b/clang-tools-extra/clangd/Diagnostics.cpp index a59d1e7ac84096..60c6ac7256b58c 100644 --- a/clang-tools-extra/clangd/Diagnostics.cpp +++ b/clang-tools-extra/clangd/Diagnostics.cpp @@ -19,6 +19,7 @@ #include "clang/Basic/SourceLocation.h" #include "clang/Basic/SourceManager.h" #include "clang/Basic/TokenKinds.h" +#include "clang/Format/Format.h" #include "clang/Lex/Lexer.h" #include "clang/Lex/Token.h" #include "llvm/ADT/ArrayRef.h" @@ -741,7 +742,7 @@ void StoreDiags::HandleDiagnostic(DiagnosticsEngine::Level DiagLevel, return false; // Copy as we may modify the ranges. auto FixIts = Info.getFixItHints().vec(); -llvm::SmallVector Edits; +auto Replacements = std::make_optional(); for (auto &FixIt : FixIts) { // Allow fixits within a single macro-arg expansion to be applied. // This can be incorrect if the argument is expanded multiple times in @@ -761,7 +762,37 @@ void StoreDiags::HandleDiagnostic(DiagnosticsEngine::Level DiagLevel, return false; if (!isInsideMainFile(FixIt.RemoveRange.getBegin(), SM)) return false; - Edits.push_back(toTextEdit(FixIt, SM, *LangOpts)); + + auto R = tooling::Replacement(SM, FixIt.RemoveRange, FixIt.CodeToInsert, +*LangOpts); + auto Err = Replacements->addOrMerge(R); + if (Err) { +log("Skipping formatting the replacement due to conflict: {0}", +llvm::toString(std::move(Err))); +Replacements = std::nullopt; +break; + } +} + +llvm::SmallVector Edits; + +if (Replacements) { + StringRef Code = SM.getBufferData(SM.getMainFileID()); + auto Repl = format::cleanupAroundReplacements(Code, *Replacements, +format::getNoStyle()); + if (!Repl) { +log("Skipping formatting the replacement due to conflict: {0}", +llvm::toString(std::move(Repl.takeError(; +Replacements = std::nullopt; + } else { +auto Es = replacementsToEdits(Code, *Repl); +Edits.append(Es.begin(), Es.end()); + } +} +if (!Replacements) { + for (auto &FixIt : FixIts) { +Edits.push_back(toTextEdit(FixIt, SM, *LangOpts)); + } } llvm::SmallString<64> Message; diff --git a/clang-tools-extra/clangd/unitte
[clang-tools-extra] [clang-tidy][clangd] Fixed removeFunctionArgs don't remove comma for use-ranges check (PR #118568)
chomosuke wrote: @EugeneZelenko Mentioned in ReleaseNotes in #118569, since the ReleaseNotes of #118569 would include the changes in this PR. Please let me know if more details are required in the ReleaseNotes. https://github.com/llvm/llvm-project/pull/118568 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [clangd][clang-tidy] Make clangd run `format::cleanupAroundReplacements()` for all code actions just as clang-tidy does (PR #118569)
https://github.com/chomosuke updated https://github.com/llvm/llvm-project/pull/118569 >From efc17a803c9c22543de7d5f9e960a7267ade1f2e Mon Sep 17 00:00:00 2001 From: chomosuke Date: Wed, 4 Dec 2024 14:42:24 + Subject: [PATCH 1/2] [clangd][clang-tidy] Make clangd run `format::cleanupAroundReplacements()` for all code actions just as clang-tidy does --- clang-tools-extra/clang-tidy/ClangTidy.cpp| 22 ++-- clang-tools-extra/clangd/Diagnostics.cpp | 35 - .../clangd/unittests/DiagnosticsTests.cpp | 51 --- .../include/clang/Tooling/Core/Replacement.h | 4 ++ clang/lib/Tooling/Core/Replacement.cpp| 16 +- 5 files changed, 99 insertions(+), 29 deletions(-) diff --git a/clang-tools-extra/clang-tidy/ClangTidy.cpp b/clang-tools-extra/clang-tidy/ClangTidy.cpp index 9c8c93c5d16c72..82331c724eaaf2 100644 --- a/clang-tools-extra/clang-tidy/ClangTidy.cpp +++ b/clang-tools-extra/clang-tidy/ClangTidy.cpp @@ -149,26 +149,12 @@ class ErrorReporter { Repl.getLength(), Repl.getReplacementText()); auto &Entry = FileReplacements[R.getFilePath()]; Replacements &Replacements = Entry.Replaces; -llvm::Error Err = Replacements.add(R); +llvm::Error Err = Replacements.addOrMerge(R); if (Err) { // FIXME: Implement better conflict handling. - llvm::errs() << "Trying to resolve conflict: " - << llvm::toString(std::move(Err)) << "\n"; - unsigned NewOffset = - Replacements.getShiftedCodePosition(R.getOffset()); - unsigned NewLength = Replacements.getShiftedCodePosition( - R.getOffset() + R.getLength()) - - NewOffset; - if (NewLength == R.getLength()) { -R = Replacement(R.getFilePath(), NewOffset, NewLength, -R.getReplacementText()); -Replacements = Replacements.merge(tooling::Replacements(R)); -CanBeApplied = true; -++AppliedFixes; - } else { -llvm::errs() -<< "Can't resolve conflict, skipping the replacement.\n"; - } + llvm::errs() + << "Can't resolve conflict, skipping the replacement: " + << llvm::toString(std::move(Err)) << '\n'; } else { CanBeApplied = true; ++AppliedFixes; diff --git a/clang-tools-extra/clangd/Diagnostics.cpp b/clang-tools-extra/clangd/Diagnostics.cpp index a59d1e7ac84096..60c6ac7256b58c 100644 --- a/clang-tools-extra/clangd/Diagnostics.cpp +++ b/clang-tools-extra/clangd/Diagnostics.cpp @@ -19,6 +19,7 @@ #include "clang/Basic/SourceLocation.h" #include "clang/Basic/SourceManager.h" #include "clang/Basic/TokenKinds.h" +#include "clang/Format/Format.h" #include "clang/Lex/Lexer.h" #include "clang/Lex/Token.h" #include "llvm/ADT/ArrayRef.h" @@ -741,7 +742,7 @@ void StoreDiags::HandleDiagnostic(DiagnosticsEngine::Level DiagLevel, return false; // Copy as we may modify the ranges. auto FixIts = Info.getFixItHints().vec(); -llvm::SmallVector Edits; +auto Replacements = std::make_optional(); for (auto &FixIt : FixIts) { // Allow fixits within a single macro-arg expansion to be applied. // This can be incorrect if the argument is expanded multiple times in @@ -761,7 +762,37 @@ void StoreDiags::HandleDiagnostic(DiagnosticsEngine::Level DiagLevel, return false; if (!isInsideMainFile(FixIt.RemoveRange.getBegin(), SM)) return false; - Edits.push_back(toTextEdit(FixIt, SM, *LangOpts)); + + auto R = tooling::Replacement(SM, FixIt.RemoveRange, FixIt.CodeToInsert, +*LangOpts); + auto Err = Replacements->addOrMerge(R); + if (Err) { +log("Skipping formatting the replacement due to conflict: {0}", +llvm::toString(std::move(Err))); +Replacements = std::nullopt; +break; + } +} + +llvm::SmallVector Edits; + +if (Replacements) { + StringRef Code = SM.getBufferData(SM.getMainFileID()); + auto Repl = format::cleanupAroundReplacements(Code, *Replacements, +format::getNoStyle()); + if (!Repl) { +log("Skipping formatting the replacement due to conflict: {0}", +llvm::toString(std::move(Repl.takeError(; +Replacements = std::nullopt; + } else { +auto Es = replacementsToEdits(Code, *Repl); +Edits.append(Es.begin(), Es.end()); + } +} +if (!Replacements) { + for (auto &FixIt : FixIts) { +Edits.push_back(toTextEdit(FixIt, SM, *LangOpts)); + } } llvm::SmallString<64> Message; diff --git a/clang-tools-extra/clangd/unitte
[clang] [clang-tools-extra] [clangd][clang-tidy] Make clangd run `format::cleanupAroundReplacements()` for all code actions just as clang-tidy does (PR #118569)
https://github.com/chomosuke updated https://github.com/llvm/llvm-project/pull/118569 >From efc17a803c9c22543de7d5f9e960a7267ade1f2e Mon Sep 17 00:00:00 2001 From: chomosuke Date: Wed, 4 Dec 2024 14:42:24 + Subject: [PATCH 1/2] [clangd][clang-tidy] Make clangd run `format::cleanupAroundReplacements()` for all code actions just as clang-tidy does --- clang-tools-extra/clang-tidy/ClangTidy.cpp| 22 ++-- clang-tools-extra/clangd/Diagnostics.cpp | 35 - .../clangd/unittests/DiagnosticsTests.cpp | 51 --- .../include/clang/Tooling/Core/Replacement.h | 4 ++ clang/lib/Tooling/Core/Replacement.cpp| 16 +- 5 files changed, 99 insertions(+), 29 deletions(-) diff --git a/clang-tools-extra/clang-tidy/ClangTidy.cpp b/clang-tools-extra/clang-tidy/ClangTidy.cpp index 9c8c93c5d16c72..82331c724eaaf2 100644 --- a/clang-tools-extra/clang-tidy/ClangTidy.cpp +++ b/clang-tools-extra/clang-tidy/ClangTidy.cpp @@ -149,26 +149,12 @@ class ErrorReporter { Repl.getLength(), Repl.getReplacementText()); auto &Entry = FileReplacements[R.getFilePath()]; Replacements &Replacements = Entry.Replaces; -llvm::Error Err = Replacements.add(R); +llvm::Error Err = Replacements.addOrMerge(R); if (Err) { // FIXME: Implement better conflict handling. - llvm::errs() << "Trying to resolve conflict: " - << llvm::toString(std::move(Err)) << "\n"; - unsigned NewOffset = - Replacements.getShiftedCodePosition(R.getOffset()); - unsigned NewLength = Replacements.getShiftedCodePosition( - R.getOffset() + R.getLength()) - - NewOffset; - if (NewLength == R.getLength()) { -R = Replacement(R.getFilePath(), NewOffset, NewLength, -R.getReplacementText()); -Replacements = Replacements.merge(tooling::Replacements(R)); -CanBeApplied = true; -++AppliedFixes; - } else { -llvm::errs() -<< "Can't resolve conflict, skipping the replacement.\n"; - } + llvm::errs() + << "Can't resolve conflict, skipping the replacement: " + << llvm::toString(std::move(Err)) << '\n'; } else { CanBeApplied = true; ++AppliedFixes; diff --git a/clang-tools-extra/clangd/Diagnostics.cpp b/clang-tools-extra/clangd/Diagnostics.cpp index a59d1e7ac84096..60c6ac7256b58c 100644 --- a/clang-tools-extra/clangd/Diagnostics.cpp +++ b/clang-tools-extra/clangd/Diagnostics.cpp @@ -19,6 +19,7 @@ #include "clang/Basic/SourceLocation.h" #include "clang/Basic/SourceManager.h" #include "clang/Basic/TokenKinds.h" +#include "clang/Format/Format.h" #include "clang/Lex/Lexer.h" #include "clang/Lex/Token.h" #include "llvm/ADT/ArrayRef.h" @@ -741,7 +742,7 @@ void StoreDiags::HandleDiagnostic(DiagnosticsEngine::Level DiagLevel, return false; // Copy as we may modify the ranges. auto FixIts = Info.getFixItHints().vec(); -llvm::SmallVector Edits; +auto Replacements = std::make_optional(); for (auto &FixIt : FixIts) { // Allow fixits within a single macro-arg expansion to be applied. // This can be incorrect if the argument is expanded multiple times in @@ -761,7 +762,37 @@ void StoreDiags::HandleDiagnostic(DiagnosticsEngine::Level DiagLevel, return false; if (!isInsideMainFile(FixIt.RemoveRange.getBegin(), SM)) return false; - Edits.push_back(toTextEdit(FixIt, SM, *LangOpts)); + + auto R = tooling::Replacement(SM, FixIt.RemoveRange, FixIt.CodeToInsert, +*LangOpts); + auto Err = Replacements->addOrMerge(R); + if (Err) { +log("Skipping formatting the replacement due to conflict: {0}", +llvm::toString(std::move(Err))); +Replacements = std::nullopt; +break; + } +} + +llvm::SmallVector Edits; + +if (Replacements) { + StringRef Code = SM.getBufferData(SM.getMainFileID()); + auto Repl = format::cleanupAroundReplacements(Code, *Replacements, +format::getNoStyle()); + if (!Repl) { +log("Skipping formatting the replacement due to conflict: {0}", +llvm::toString(std::move(Repl.takeError(; +Replacements = std::nullopt; + } else { +auto Es = replacementsToEdits(Code, *Repl); +Edits.append(Es.begin(), Es.end()); + } +} +if (!Replacements) { + for (auto &FixIt : FixIts) { +Edits.push_back(toTextEdit(FixIt, SM, *LangOpts)); + } } llvm::SmallString<64> Message; diff --git a/clang-tools-extra/clangd/unitte
[clang-tools-extra] [clangd][clang-tidy] Make clangd run `format::cleanupAroundReplacements()` for all code actions just as clang-tidy does (PR #118569)
https://github.com/chomosuke updated https://github.com/llvm/llvm-project/pull/118569 >From d2257eb43bbd9ce2dfd8a13123c2048e1cf8b439 Mon Sep 17 00:00:00 2001 From: chomosuke Date: Wed, 4 Dec 2024 00:07:31 + Subject: [PATCH 1/2] clangd cleanupAroundReplacements just as clang-tidy does --- clang-tools-extra/clangd/Diagnostics.cpp | 48 +++- 1 file changed, 46 insertions(+), 2 deletions(-) diff --git a/clang-tools-extra/clangd/Diagnostics.cpp b/clang-tools-extra/clangd/Diagnostics.cpp index a59d1e7ac84096..af8d0d534af689 100644 --- a/clang-tools-extra/clangd/Diagnostics.cpp +++ b/clang-tools-extra/clangd/Diagnostics.cpp @@ -19,6 +19,7 @@ #include "clang/Basic/SourceLocation.h" #include "clang/Basic/SourceManager.h" #include "clang/Basic/TokenKinds.h" +#include "clang/Format/Format.h" #include "clang/Lex/Lexer.h" #include "clang/Lex/Token.h" #include "llvm/ADT/ArrayRef.h" @@ -741,7 +742,7 @@ void StoreDiags::HandleDiagnostic(DiagnosticsEngine::Level DiagLevel, return false; // Copy as we may modify the ranges. auto FixIts = Info.getFixItHints().vec(); -llvm::SmallVector Edits; +auto Replacements = std::make_optional(); for (auto &FixIt : FixIts) { // Allow fixits within a single macro-arg expansion to be applied. // This can be incorrect if the argument is expanded multiple times in @@ -761,7 +762,50 @@ void StoreDiags::HandleDiagnostic(DiagnosticsEngine::Level DiagLevel, return false; if (!isInsideMainFile(FixIt.RemoveRange.getBegin(), SM)) return false; - Edits.push_back(toTextEdit(FixIt, SM, *LangOpts)); + + auto R = tooling::Replacement(SM, FixIt.RemoveRange, FixIt.CodeToInsert, +*LangOpts); + auto Err = Replacements->add(R); + if (Err) { +// FIXME: Remove duplicated code as in ClangTidy.cpp +unsigned NewOffset = +Replacements->getShiftedCodePosition(R.getOffset()); +unsigned NewLength = Replacements->getShiftedCodePosition( + R.getOffset() + R.getLength()) - + NewOffset; +if (NewLength == R.getLength()) { + llvm::consumeError(std::move(Err)); + R = tooling::Replacement(R.getFilePath(), NewOffset, NewLength, + R.getReplacementText()); + Replacements = Replacements->merge(tooling::Replacements(R)); +} else { + log("Skipping formatting the replacement due to conflict: {0}", + llvm::toString(std::move(Err))); + Replacements = std::nullopt; + break; +} + } +} + +llvm::SmallVector Edits; + +if (Replacements) { + StringRef Code = SM.getBufferData(SM.getMainFileID()); + auto Repl = format::cleanupAroundReplacements(Code, *Replacements, +format::getNoStyle()); + if (!Repl) { +log("Skipping formatting the replacement due to conflict: {0}", + llvm::toString(std::move(Repl.takeError(; +Replacements = std::nullopt; + } else { +auto Es = replacementsToEdits(Code, *Repl); +Edits.append(Es.begin(), Es.end()); + } +} +if (!Replacements) { + for (auto &FixIt : FixIts) { +Edits.push_back(toTextEdit(FixIt, SM, *LangOpts)); + } } llvm::SmallString<64> Message; >From cbdc1e25e3a39472c068a742b0f40e9c71221521 Mon Sep 17 00:00:00 2001 From: chomosuke Date: Wed, 4 Dec 2024 01:15:57 + Subject: [PATCH 2/2] fixed tests --- .../clangd/unittests/DiagnosticsTests.cpp| 16 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp b/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp index 7a47d6ebebf3b2..94765b48099eda 100644 --- a/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp +++ b/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp @@ -876,17 +876,17 @@ TEST(DiagnosticTest, ClangTidySelfContainedDiags) { clangd::Fix ExpectedCFix; ExpectedCFix.Message = "variable 'C' is not initialized"; - ExpectedCFix.Edits.push_back(TextEdit{Main.range("CFix"), " = NAN"}); ExpectedCFix.Edits.push_back( TextEdit{Main.range("MathHeader"), "#include \n\n"}); + ExpectedCFix.Edits.push_back(TextEdit{Main.range("CFix"), " = NAN"}); // Again in clang-tidy only the include directive would be emitted for the // first warning. However we need the include attaching for both warnings. clangd::Fix ExpectedDFix; ExpectedDFix.Message = "variable 'D' is not initialized"; - ExpectedDFix.Edits.push_back(TextEdit{Main.range("DFix"), " = NAN"}); ExpectedDFix.Edits.push_back( TextEdit{Main.range("MathHeader"), "#include \n\n"}); + ExpectedDFix.Edits.push_back(TextEdit{Main.range("DFix"), " = NAN"}); EXPECT_THAT( TU.build().getDiagnostics(), ifT
[clang-tools-extra] [clang-tidy][clangd] Fixed removeFunctionArgs don't remove comma for use-ranges check (PR #118568)
https://github.com/chomosuke updated https://github.com/llvm/llvm-project/pull/118568 >From b43a2602025bdacea06ced5171904fb5d765de9f Mon Sep 17 00:00:00 2001 From: chomosuke Date: Tue, 3 Dec 2024 07:10:33 + Subject: [PATCH] fixed removeFunctionArgs don't remove comma --- .../clang-tidy/utils/UseRangesCheck.cpp | 19 ++- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/clang-tools-extra/clang-tidy/utils/UseRangesCheck.cpp b/clang-tools-extra/clang-tidy/utils/UseRangesCheck.cpp index aba4d17ccd035e..88cba70b931d5d 100644 --- a/clang-tools-extra/clang-tidy/utils/UseRangesCheck.cpp +++ b/clang-tools-extra/clang-tidy/utils/UseRangesCheck.cpp @@ -28,6 +28,7 @@ #include "llvm/ADT/Twine.h" #include "llvm/Support/raw_ostream.h" #include +#include #include #include @@ -173,21 +174,21 @@ static void removeFunctionArgs(DiagnosticBuilder &Diag, const CallExpr &Call, for (unsigned Index : Sorted) { const Expr *Arg = Call.getArg(Index); if (Commas[Index]) { - if (Index >= Commas.size()) { -Diag << FixItHint::CreateRemoval(Arg->getSourceRange()); - } else { + if (Index + 1 < Call.getNumArgs()) { // Remove the next comma Commas[Index + 1] = true; +const Expr *NextArg = Call.getArg(Index + 1); Diag << FixItHint::CreateRemoval(CharSourceRange::getTokenRange( -{Arg->getBeginLoc(), - Lexer::getLocForEndOfToken( - Arg->getEndLoc(), 0, Ctx.getSourceManager(), Ctx.getLangOpts()) - .getLocWithOffset(1)})); +{Arg->getBeginLoc(), NextArg->getBeginLoc().getLocWithOffset(-1)})); + } else { +Diag << FixItHint::CreateRemoval(Arg->getSourceRange()); } } else { - Diag << FixItHint::CreateRemoval(CharSourceRange::getTokenRange( - Arg->getBeginLoc().getLocWithOffset(-1), Arg->getEndLoc())); + // At this point we know Index > 0 because `Commas[0] = true` earlier Commas[Index] = true; + const Expr *PrevArg = Call.getArg(Index - 1); + Diag << FixItHint::CreateRemoval(CharSourceRange::getTokenRange( + PrevArg->getEndLoc().getLocWithOffset(1), Arg->getEndLoc())); } } } ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] clangd cleanupAroundReplacements just as clang-tidy does (PR #118569)
https://github.com/chomosuke created https://github.com/llvm/llvm-project/pull/118569 None >From d2257eb43bbd9ce2dfd8a13123c2048e1cf8b439 Mon Sep 17 00:00:00 2001 From: chomosuke Date: Wed, 4 Dec 2024 00:07:31 + Subject: [PATCH] clangd cleanupAroundReplacements just as clang-tidy does --- clang-tools-extra/clangd/Diagnostics.cpp | 48 +++- 1 file changed, 46 insertions(+), 2 deletions(-) diff --git a/clang-tools-extra/clangd/Diagnostics.cpp b/clang-tools-extra/clangd/Diagnostics.cpp index a59d1e7ac84096..af8d0d534af689 100644 --- a/clang-tools-extra/clangd/Diagnostics.cpp +++ b/clang-tools-extra/clangd/Diagnostics.cpp @@ -19,6 +19,7 @@ #include "clang/Basic/SourceLocation.h" #include "clang/Basic/SourceManager.h" #include "clang/Basic/TokenKinds.h" +#include "clang/Format/Format.h" #include "clang/Lex/Lexer.h" #include "clang/Lex/Token.h" #include "llvm/ADT/ArrayRef.h" @@ -741,7 +742,7 @@ void StoreDiags::HandleDiagnostic(DiagnosticsEngine::Level DiagLevel, return false; // Copy as we may modify the ranges. auto FixIts = Info.getFixItHints().vec(); -llvm::SmallVector Edits; +auto Replacements = std::make_optional(); for (auto &FixIt : FixIts) { // Allow fixits within a single macro-arg expansion to be applied. // This can be incorrect if the argument is expanded multiple times in @@ -761,7 +762,50 @@ void StoreDiags::HandleDiagnostic(DiagnosticsEngine::Level DiagLevel, return false; if (!isInsideMainFile(FixIt.RemoveRange.getBegin(), SM)) return false; - Edits.push_back(toTextEdit(FixIt, SM, *LangOpts)); + + auto R = tooling::Replacement(SM, FixIt.RemoveRange, FixIt.CodeToInsert, +*LangOpts); + auto Err = Replacements->add(R); + if (Err) { +// FIXME: Remove duplicated code as in ClangTidy.cpp +unsigned NewOffset = +Replacements->getShiftedCodePosition(R.getOffset()); +unsigned NewLength = Replacements->getShiftedCodePosition( + R.getOffset() + R.getLength()) - + NewOffset; +if (NewLength == R.getLength()) { + llvm::consumeError(std::move(Err)); + R = tooling::Replacement(R.getFilePath(), NewOffset, NewLength, + R.getReplacementText()); + Replacements = Replacements->merge(tooling::Replacements(R)); +} else { + log("Skipping formatting the replacement due to conflict: {0}", + llvm::toString(std::move(Err))); + Replacements = std::nullopt; + break; +} + } +} + +llvm::SmallVector Edits; + +if (Replacements) { + StringRef Code = SM.getBufferData(SM.getMainFileID()); + auto Repl = format::cleanupAroundReplacements(Code, *Replacements, +format::getNoStyle()); + if (!Repl) { +log("Skipping formatting the replacement due to conflict: {0}", + llvm::toString(std::move(Repl.takeError(; +Replacements = std::nullopt; + } else { +auto Es = replacementsToEdits(Code, *Repl); +Edits.append(Es.begin(), Es.end()); + } +} +if (!Replacements) { + for (auto &FixIt : FixIts) { +Edits.push_back(toTextEdit(FixIt, SM, *LangOpts)); + } } llvm::SmallString<64> Message; ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy][clangd] Fixed removeFunctionArgs don't remove comma for use-ranges check (PR #118568)
https://github.com/chomosuke created https://github.com/llvm/llvm-project/pull/118568 The problem: When running the code action for `mordernize-use-ranges` on `std::reverse(v.begin(), v.end())`, the code gets modified to `std::reverse(v,)` instead of `std::reverse(v)`. This PR fixes both `mordernize-use-ranges` and `boost-use-ranges`. The more general problem: This does not show up in tests for clang-tidy because `clang-tidy --fix` runs `format::cleanupAroundReplacements()` before committing the fixes where as clangd does not. There are many other clang-tidy check that work with `clant-tidy --fix` but does not work with clangd code action. I will soon open an PR that fix this in the general case by running `format::cleanupAroundReplacements()` in clangd. >From b43a2602025bdacea06ced5171904fb5d765de9f Mon Sep 17 00:00:00 2001 From: chomosuke Date: Tue, 3 Dec 2024 07:10:33 + Subject: [PATCH] fixed removeFunctionArgs don't remove comma --- .../clang-tidy/utils/UseRangesCheck.cpp | 19 ++- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/clang-tools-extra/clang-tidy/utils/UseRangesCheck.cpp b/clang-tools-extra/clang-tidy/utils/UseRangesCheck.cpp index aba4d17ccd035e..88cba70b931d5d 100644 --- a/clang-tools-extra/clang-tidy/utils/UseRangesCheck.cpp +++ b/clang-tools-extra/clang-tidy/utils/UseRangesCheck.cpp @@ -28,6 +28,7 @@ #include "llvm/ADT/Twine.h" #include "llvm/Support/raw_ostream.h" #include +#include #include #include @@ -173,21 +174,21 @@ static void removeFunctionArgs(DiagnosticBuilder &Diag, const CallExpr &Call, for (unsigned Index : Sorted) { const Expr *Arg = Call.getArg(Index); if (Commas[Index]) { - if (Index >= Commas.size()) { -Diag << FixItHint::CreateRemoval(Arg->getSourceRange()); - } else { + if (Index + 1 < Call.getNumArgs()) { // Remove the next comma Commas[Index + 1] = true; +const Expr *NextArg = Call.getArg(Index + 1); Diag << FixItHint::CreateRemoval(CharSourceRange::getTokenRange( -{Arg->getBeginLoc(), - Lexer::getLocForEndOfToken( - Arg->getEndLoc(), 0, Ctx.getSourceManager(), Ctx.getLangOpts()) - .getLocWithOffset(1)})); +{Arg->getBeginLoc(), NextArg->getBeginLoc().getLocWithOffset(-1)})); + } else { +Diag << FixItHint::CreateRemoval(Arg->getSourceRange()); } } else { - Diag << FixItHint::CreateRemoval(CharSourceRange::getTokenRange( - Arg->getBeginLoc().getLocWithOffset(-1), Arg->getEndLoc())); + // At this point we know Index > 0 because `Commas[0] = true` earlier Commas[Index] = true; + const Expr *PrevArg = Call.getArg(Index - 1); + Diag << FixItHint::CreateRemoval(CharSourceRange::getTokenRange( + PrevArg->getEndLoc().getLocWithOffset(1), Arg->getEndLoc())); } } } ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd][clang-tidy] Make clangd run `format::cleanupAroundReplacements()` for all code actions just as clang-tidy does (PR #118569)
https://github.com/chomosuke edited https://github.com/llvm/llvm-project/pull/118569 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] clangd cleanupAroundReplacements just as clang-tidy does (PR #118569)
https://github.com/chomosuke updated https://github.com/llvm/llvm-project/pull/118569 >From d2257eb43bbd9ce2dfd8a13123c2048e1cf8b439 Mon Sep 17 00:00:00 2001 From: chomosuke Date: Wed, 4 Dec 2024 00:07:31 + Subject: [PATCH] clangd cleanupAroundReplacements just as clang-tidy does --- clang-tools-extra/clangd/Diagnostics.cpp | 48 +++- 1 file changed, 46 insertions(+), 2 deletions(-) diff --git a/clang-tools-extra/clangd/Diagnostics.cpp b/clang-tools-extra/clangd/Diagnostics.cpp index a59d1e7ac84096..af8d0d534af689 100644 --- a/clang-tools-extra/clangd/Diagnostics.cpp +++ b/clang-tools-extra/clangd/Diagnostics.cpp @@ -19,6 +19,7 @@ #include "clang/Basic/SourceLocation.h" #include "clang/Basic/SourceManager.h" #include "clang/Basic/TokenKinds.h" +#include "clang/Format/Format.h" #include "clang/Lex/Lexer.h" #include "clang/Lex/Token.h" #include "llvm/ADT/ArrayRef.h" @@ -741,7 +742,7 @@ void StoreDiags::HandleDiagnostic(DiagnosticsEngine::Level DiagLevel, return false; // Copy as we may modify the ranges. auto FixIts = Info.getFixItHints().vec(); -llvm::SmallVector Edits; +auto Replacements = std::make_optional(); for (auto &FixIt : FixIts) { // Allow fixits within a single macro-arg expansion to be applied. // This can be incorrect if the argument is expanded multiple times in @@ -761,7 +762,50 @@ void StoreDiags::HandleDiagnostic(DiagnosticsEngine::Level DiagLevel, return false; if (!isInsideMainFile(FixIt.RemoveRange.getBegin(), SM)) return false; - Edits.push_back(toTextEdit(FixIt, SM, *LangOpts)); + + auto R = tooling::Replacement(SM, FixIt.RemoveRange, FixIt.CodeToInsert, +*LangOpts); + auto Err = Replacements->add(R); + if (Err) { +// FIXME: Remove duplicated code as in ClangTidy.cpp +unsigned NewOffset = +Replacements->getShiftedCodePosition(R.getOffset()); +unsigned NewLength = Replacements->getShiftedCodePosition( + R.getOffset() + R.getLength()) - + NewOffset; +if (NewLength == R.getLength()) { + llvm::consumeError(std::move(Err)); + R = tooling::Replacement(R.getFilePath(), NewOffset, NewLength, + R.getReplacementText()); + Replacements = Replacements->merge(tooling::Replacements(R)); +} else { + log("Skipping formatting the replacement due to conflict: {0}", + llvm::toString(std::move(Err))); + Replacements = std::nullopt; + break; +} + } +} + +llvm::SmallVector Edits; + +if (Replacements) { + StringRef Code = SM.getBufferData(SM.getMainFileID()); + auto Repl = format::cleanupAroundReplacements(Code, *Replacements, +format::getNoStyle()); + if (!Repl) { +log("Skipping formatting the replacement due to conflict: {0}", + llvm::toString(std::move(Repl.takeError(; +Replacements = std::nullopt; + } else { +auto Es = replacementsToEdits(Code, *Repl); +Edits.append(Es.begin(), Es.end()); + } +} +if (!Replacements) { + for (auto &FixIt : FixIts) { +Edits.push_back(toTextEdit(FixIt, SM, *LangOpts)); + } } llvm::SmallString<64> Message; ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [clangd][clang-tidy] Make clangd run `format::cleanupAroundReplacements()` for all code actions just as clang-tidy does (PR #118569)
https://github.com/chomosuke updated https://github.com/llvm/llvm-project/pull/118569 >From efc17a803c9c22543de7d5f9e960a7267ade1f2e Mon Sep 17 00:00:00 2001 From: chomosuke Date: Wed, 4 Dec 2024 14:42:24 + Subject: [PATCH 1/2] [clangd][clang-tidy] Make clangd run `format::cleanupAroundReplacements()` for all code actions just as clang-tidy does --- clang-tools-extra/clang-tidy/ClangTidy.cpp| 22 ++-- clang-tools-extra/clangd/Diagnostics.cpp | 35 - .../clangd/unittests/DiagnosticsTests.cpp | 51 --- .../include/clang/Tooling/Core/Replacement.h | 4 ++ clang/lib/Tooling/Core/Replacement.cpp| 16 +- 5 files changed, 99 insertions(+), 29 deletions(-) diff --git a/clang-tools-extra/clang-tidy/ClangTidy.cpp b/clang-tools-extra/clang-tidy/ClangTidy.cpp index 9c8c93c5d16c72..82331c724eaaf2 100644 --- a/clang-tools-extra/clang-tidy/ClangTidy.cpp +++ b/clang-tools-extra/clang-tidy/ClangTidy.cpp @@ -149,26 +149,12 @@ class ErrorReporter { Repl.getLength(), Repl.getReplacementText()); auto &Entry = FileReplacements[R.getFilePath()]; Replacements &Replacements = Entry.Replaces; -llvm::Error Err = Replacements.add(R); +llvm::Error Err = Replacements.addOrMerge(R); if (Err) { // FIXME: Implement better conflict handling. - llvm::errs() << "Trying to resolve conflict: " - << llvm::toString(std::move(Err)) << "\n"; - unsigned NewOffset = - Replacements.getShiftedCodePosition(R.getOffset()); - unsigned NewLength = Replacements.getShiftedCodePosition( - R.getOffset() + R.getLength()) - - NewOffset; - if (NewLength == R.getLength()) { -R = Replacement(R.getFilePath(), NewOffset, NewLength, -R.getReplacementText()); -Replacements = Replacements.merge(tooling::Replacements(R)); -CanBeApplied = true; -++AppliedFixes; - } else { -llvm::errs() -<< "Can't resolve conflict, skipping the replacement.\n"; - } + llvm::errs() + << "Can't resolve conflict, skipping the replacement: " + << llvm::toString(std::move(Err)) << '\n'; } else { CanBeApplied = true; ++AppliedFixes; diff --git a/clang-tools-extra/clangd/Diagnostics.cpp b/clang-tools-extra/clangd/Diagnostics.cpp index a59d1e7ac84096..60c6ac7256b58c 100644 --- a/clang-tools-extra/clangd/Diagnostics.cpp +++ b/clang-tools-extra/clangd/Diagnostics.cpp @@ -19,6 +19,7 @@ #include "clang/Basic/SourceLocation.h" #include "clang/Basic/SourceManager.h" #include "clang/Basic/TokenKinds.h" +#include "clang/Format/Format.h" #include "clang/Lex/Lexer.h" #include "clang/Lex/Token.h" #include "llvm/ADT/ArrayRef.h" @@ -741,7 +742,7 @@ void StoreDiags::HandleDiagnostic(DiagnosticsEngine::Level DiagLevel, return false; // Copy as we may modify the ranges. auto FixIts = Info.getFixItHints().vec(); -llvm::SmallVector Edits; +auto Replacements = std::make_optional(); for (auto &FixIt : FixIts) { // Allow fixits within a single macro-arg expansion to be applied. // This can be incorrect if the argument is expanded multiple times in @@ -761,7 +762,37 @@ void StoreDiags::HandleDiagnostic(DiagnosticsEngine::Level DiagLevel, return false; if (!isInsideMainFile(FixIt.RemoveRange.getBegin(), SM)) return false; - Edits.push_back(toTextEdit(FixIt, SM, *LangOpts)); + + auto R = tooling::Replacement(SM, FixIt.RemoveRange, FixIt.CodeToInsert, +*LangOpts); + auto Err = Replacements->addOrMerge(R); + if (Err) { +log("Skipping formatting the replacement due to conflict: {0}", +llvm::toString(std::move(Err))); +Replacements = std::nullopt; +break; + } +} + +llvm::SmallVector Edits; + +if (Replacements) { + StringRef Code = SM.getBufferData(SM.getMainFileID()); + auto Repl = format::cleanupAroundReplacements(Code, *Replacements, +format::getNoStyle()); + if (!Repl) { +log("Skipping formatting the replacement due to conflict: {0}", +llvm::toString(std::move(Repl.takeError(; +Replacements = std::nullopt; + } else { +auto Es = replacementsToEdits(Code, *Repl); +Edits.append(Es.begin(), Es.end()); + } +} +if (!Replacements) { + for (auto &FixIt : FixIts) { +Edits.push_back(toTextEdit(FixIt, SM, *LangOpts)); + } } llvm::SmallString<64> Message; diff --git a/clang-tools-extra/clangd/unitte
[clang-tools-extra] [clang-tidy][clangd] Fixed removeFunctionArgs don't remove comma for use-ranges check (PR #118568)
https://github.com/chomosuke updated https://github.com/llvm/llvm-project/pull/118568 >From b43a2602025bdacea06ced5171904fb5d765de9f Mon Sep 17 00:00:00 2001 From: chomosuke Date: Tue, 3 Dec 2024 07:10:33 + Subject: [PATCH 1/2] fixed removeFunctionArgs don't remove comma --- .../clang-tidy/utils/UseRangesCheck.cpp | 19 ++- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/clang-tools-extra/clang-tidy/utils/UseRangesCheck.cpp b/clang-tools-extra/clang-tidy/utils/UseRangesCheck.cpp index aba4d17ccd035e..88cba70b931d5d 100644 --- a/clang-tools-extra/clang-tidy/utils/UseRangesCheck.cpp +++ b/clang-tools-extra/clang-tidy/utils/UseRangesCheck.cpp @@ -28,6 +28,7 @@ #include "llvm/ADT/Twine.h" #include "llvm/Support/raw_ostream.h" #include +#include #include #include @@ -173,21 +174,21 @@ static void removeFunctionArgs(DiagnosticBuilder &Diag, const CallExpr &Call, for (unsigned Index : Sorted) { const Expr *Arg = Call.getArg(Index); if (Commas[Index]) { - if (Index >= Commas.size()) { -Diag << FixItHint::CreateRemoval(Arg->getSourceRange()); - } else { + if (Index + 1 < Call.getNumArgs()) { // Remove the next comma Commas[Index + 1] = true; +const Expr *NextArg = Call.getArg(Index + 1); Diag << FixItHint::CreateRemoval(CharSourceRange::getTokenRange( -{Arg->getBeginLoc(), - Lexer::getLocForEndOfToken( - Arg->getEndLoc(), 0, Ctx.getSourceManager(), Ctx.getLangOpts()) - .getLocWithOffset(1)})); +{Arg->getBeginLoc(), NextArg->getBeginLoc().getLocWithOffset(-1)})); + } else { +Diag << FixItHint::CreateRemoval(Arg->getSourceRange()); } } else { - Diag << FixItHint::CreateRemoval(CharSourceRange::getTokenRange( - Arg->getBeginLoc().getLocWithOffset(-1), Arg->getEndLoc())); + // At this point we know Index > 0 because `Commas[0] = true` earlier Commas[Index] = true; + const Expr *PrevArg = Call.getArg(Index - 1); + Diag << FixItHint::CreateRemoval(CharSourceRange::getTokenRange( + PrevArg->getEndLoc().getLocWithOffset(1), Arg->getEndLoc())); } } } >From 644c8491e0fba203e89595827781d0c2c0609081 Mon Sep 17 00:00:00 2001 From: chomosuke Date: Mon, 23 Dec 2024 05:17:08 +1100 Subject: [PATCH 2/2] find , and remove only , --- .../clang-tidy/utils/UseRangesCheck.cpp | 49 +++ 1 file changed, 40 insertions(+), 9 deletions(-) diff --git a/clang-tools-extra/clang-tidy/utils/UseRangesCheck.cpp b/clang-tools-extra/clang-tidy/utils/UseRangesCheck.cpp index 88cba70b931d5d..8b8e44a9898fdd 100644 --- a/clang-tools-extra/clang-tidy/utils/UseRangesCheck.cpp +++ b/clang-tools-extra/clang-tidy/utils/UseRangesCheck.cpp @@ -28,7 +28,6 @@ #include "llvm/ADT/Twine.h" #include "llvm/Support/raw_ostream.h" #include -#include #include #include @@ -165,6 +164,33 @@ void UseRangesCheck::registerMatchers(MatchFinder *Finder) { static void removeFunctionArgs(DiagnosticBuilder &Diag, const CallExpr &Call, ArrayRef Indexes, const ASTContext &Ctx) { + auto GetCommaLoc = + [&](SourceLocation BeginLoc, + SourceLocation EndLoc) -> std::optional { +auto Invalid = false; +auto SourceText = Lexer::getSourceText( +CharSourceRange::getCharRange({BeginLoc, EndLoc}), +Ctx.getSourceManager(), Ctx.getLangOpts(), &Invalid); +assert(!Invalid); + +size_t I = 0; +while (I < SourceText.size() && SourceText[I] != ',') { + I++; +} + +if (I < SourceText.size()) { + // also remove space after , + size_t J = I + 1; + while (J < SourceText.size() && SourceText[J] == ' ') { +J++; + } + + return std::make_optional(CharSourceRange::getCharRange( + {BeginLoc.getLocWithOffset(I), BeginLoc.getLocWithOffset(J)})); +} +return std::nullopt; + }; + llvm::SmallVector Sorted(Indexes); llvm::sort(Sorted); // Keep track of commas removed @@ -176,20 +202,25 @@ static void removeFunctionArgs(DiagnosticBuilder &Diag, const CallExpr &Call, if (Commas[Index]) { if (Index + 1 < Call.getNumArgs()) { // Remove the next comma -Commas[Index + 1] = true; const Expr *NextArg = Call.getArg(Index + 1); -Diag << FixItHint::CreateRemoval(CharSourceRange::getTokenRange( -{Arg->getBeginLoc(), NextArg->getBeginLoc().getLocWithOffset(-1)})); - } else { -Diag << FixItHint::CreateRemoval(Arg->getSourceRange()); +auto CommaLoc = GetCommaLoc(Arg->getEndLoc().getLocWithOffset(1), +NextArg->getBeginLoc()); +if (CommaLoc) { + Commas[Index + 1] = true; + Diag << FixItHint::CreateRemoval(*CommaLoc); +} } } else { // At this point we know Index > 0 becau
[clang] [clang-tools-extra] [clangd] Make clangd run `format::cleanupAroundReplacements()` for all code actions just as clang-tidy does (PR #118569)
https://github.com/chomosuke updated https://github.com/llvm/llvm-project/pull/118569 >From efc17a803c9c22543de7d5f9e960a7267ade1f2e Mon Sep 17 00:00:00 2001 From: chomosuke Date: Wed, 4 Dec 2024 14:42:24 + Subject: [PATCH 1/3] [clangd][clang-tidy] Make clangd run `format::cleanupAroundReplacements()` for all code actions just as clang-tidy does --- clang-tools-extra/clang-tidy/ClangTidy.cpp| 22 ++-- clang-tools-extra/clangd/Diagnostics.cpp | 35 - .../clangd/unittests/DiagnosticsTests.cpp | 51 --- .../include/clang/Tooling/Core/Replacement.h | 4 ++ clang/lib/Tooling/Core/Replacement.cpp| 16 +- 5 files changed, 99 insertions(+), 29 deletions(-) diff --git a/clang-tools-extra/clang-tidy/ClangTidy.cpp b/clang-tools-extra/clang-tidy/ClangTidy.cpp index 9c8c93c5d16c72d..82331c724eaaf21 100644 --- a/clang-tools-extra/clang-tidy/ClangTidy.cpp +++ b/clang-tools-extra/clang-tidy/ClangTidy.cpp @@ -149,26 +149,12 @@ class ErrorReporter { Repl.getLength(), Repl.getReplacementText()); auto &Entry = FileReplacements[R.getFilePath()]; Replacements &Replacements = Entry.Replaces; -llvm::Error Err = Replacements.add(R); +llvm::Error Err = Replacements.addOrMerge(R); if (Err) { // FIXME: Implement better conflict handling. - llvm::errs() << "Trying to resolve conflict: " - << llvm::toString(std::move(Err)) << "\n"; - unsigned NewOffset = - Replacements.getShiftedCodePosition(R.getOffset()); - unsigned NewLength = Replacements.getShiftedCodePosition( - R.getOffset() + R.getLength()) - - NewOffset; - if (NewLength == R.getLength()) { -R = Replacement(R.getFilePath(), NewOffset, NewLength, -R.getReplacementText()); -Replacements = Replacements.merge(tooling::Replacements(R)); -CanBeApplied = true; -++AppliedFixes; - } else { -llvm::errs() -<< "Can't resolve conflict, skipping the replacement.\n"; - } + llvm::errs() + << "Can't resolve conflict, skipping the replacement: " + << llvm::toString(std::move(Err)) << '\n'; } else { CanBeApplied = true; ++AppliedFixes; diff --git a/clang-tools-extra/clangd/Diagnostics.cpp b/clang-tools-extra/clangd/Diagnostics.cpp index a59d1e7ac84096a..60c6ac7256b58c3 100644 --- a/clang-tools-extra/clangd/Diagnostics.cpp +++ b/clang-tools-extra/clangd/Diagnostics.cpp @@ -19,6 +19,7 @@ #include "clang/Basic/SourceLocation.h" #include "clang/Basic/SourceManager.h" #include "clang/Basic/TokenKinds.h" +#include "clang/Format/Format.h" #include "clang/Lex/Lexer.h" #include "clang/Lex/Token.h" #include "llvm/ADT/ArrayRef.h" @@ -741,7 +742,7 @@ void StoreDiags::HandleDiagnostic(DiagnosticsEngine::Level DiagLevel, return false; // Copy as we may modify the ranges. auto FixIts = Info.getFixItHints().vec(); -llvm::SmallVector Edits; +auto Replacements = std::make_optional(); for (auto &FixIt : FixIts) { // Allow fixits within a single macro-arg expansion to be applied. // This can be incorrect if the argument is expanded multiple times in @@ -761,7 +762,37 @@ void StoreDiags::HandleDiagnostic(DiagnosticsEngine::Level DiagLevel, return false; if (!isInsideMainFile(FixIt.RemoveRange.getBegin(), SM)) return false; - Edits.push_back(toTextEdit(FixIt, SM, *LangOpts)); + + auto R = tooling::Replacement(SM, FixIt.RemoveRange, FixIt.CodeToInsert, +*LangOpts); + auto Err = Replacements->addOrMerge(R); + if (Err) { +log("Skipping formatting the replacement due to conflict: {0}", +llvm::toString(std::move(Err))); +Replacements = std::nullopt; +break; + } +} + +llvm::SmallVector Edits; + +if (Replacements) { + StringRef Code = SM.getBufferData(SM.getMainFileID()); + auto Repl = format::cleanupAroundReplacements(Code, *Replacements, +format::getNoStyle()); + if (!Repl) { +log("Skipping formatting the replacement due to conflict: {0}", +llvm::toString(std::move(Repl.takeError(; +Replacements = std::nullopt; + } else { +auto Es = replacementsToEdits(Code, *Repl); +Edits.append(Es.begin(), Es.end()); + } +} +if (!Replacements) { + for (auto &FixIt : FixIts) { +Edits.push_back(toTextEdit(FixIt, SM, *LangOpts)); + } } llvm::SmallString<64> Message; diff --git a/clang-tools-extra/clangd/un
[clang-tools-extra] [clang-tidy][clangd] Fixed removeFunctionArgs don't remove comma for use-ranges check (PR #118568)
chomosuke wrote: Ping~~~ https://github.com/llvm/llvm-project/pull/118568 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy][clangd] Fixed removeFunctionArgs don't remove comma for use-ranges check (PR #118568)
https://github.com/chomosuke updated https://github.com/llvm/llvm-project/pull/118568 >From b43a2602025bdacea06ced5171904fb5d765de9f Mon Sep 17 00:00:00 2001 From: chomosuke Date: Tue, 3 Dec 2024 07:10:33 + Subject: [PATCH 1/5] fixed removeFunctionArgs don't remove comma --- .../clang-tidy/utils/UseRangesCheck.cpp | 19 ++- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/clang-tools-extra/clang-tidy/utils/UseRangesCheck.cpp b/clang-tools-extra/clang-tidy/utils/UseRangesCheck.cpp index aba4d17ccd035eb..88cba70b931d5dd 100644 --- a/clang-tools-extra/clang-tidy/utils/UseRangesCheck.cpp +++ b/clang-tools-extra/clang-tidy/utils/UseRangesCheck.cpp @@ -28,6 +28,7 @@ #include "llvm/ADT/Twine.h" #include "llvm/Support/raw_ostream.h" #include +#include #include #include @@ -173,21 +174,21 @@ static void removeFunctionArgs(DiagnosticBuilder &Diag, const CallExpr &Call, for (unsigned Index : Sorted) { const Expr *Arg = Call.getArg(Index); if (Commas[Index]) { - if (Index >= Commas.size()) { -Diag << FixItHint::CreateRemoval(Arg->getSourceRange()); - } else { + if (Index + 1 < Call.getNumArgs()) { // Remove the next comma Commas[Index + 1] = true; +const Expr *NextArg = Call.getArg(Index + 1); Diag << FixItHint::CreateRemoval(CharSourceRange::getTokenRange( -{Arg->getBeginLoc(), - Lexer::getLocForEndOfToken( - Arg->getEndLoc(), 0, Ctx.getSourceManager(), Ctx.getLangOpts()) - .getLocWithOffset(1)})); +{Arg->getBeginLoc(), NextArg->getBeginLoc().getLocWithOffset(-1)})); + } else { +Diag << FixItHint::CreateRemoval(Arg->getSourceRange()); } } else { - Diag << FixItHint::CreateRemoval(CharSourceRange::getTokenRange( - Arg->getBeginLoc().getLocWithOffset(-1), Arg->getEndLoc())); + // At this point we know Index > 0 because `Commas[0] = true` earlier Commas[Index] = true; + const Expr *PrevArg = Call.getArg(Index - 1); + Diag << FixItHint::CreateRemoval(CharSourceRange::getTokenRange( + PrevArg->getEndLoc().getLocWithOffset(1), Arg->getEndLoc())); } } } >From 644c8491e0fba203e89595827781d0c2c0609081 Mon Sep 17 00:00:00 2001 From: chomosuke Date: Mon, 23 Dec 2024 05:17:08 +1100 Subject: [PATCH 2/5] find , and remove only , --- .../clang-tidy/utils/UseRangesCheck.cpp | 49 +++ 1 file changed, 40 insertions(+), 9 deletions(-) diff --git a/clang-tools-extra/clang-tidy/utils/UseRangesCheck.cpp b/clang-tools-extra/clang-tidy/utils/UseRangesCheck.cpp index 88cba70b931d5dd..8b8e44a9898fdd3 100644 --- a/clang-tools-extra/clang-tidy/utils/UseRangesCheck.cpp +++ b/clang-tools-extra/clang-tidy/utils/UseRangesCheck.cpp @@ -28,7 +28,6 @@ #include "llvm/ADT/Twine.h" #include "llvm/Support/raw_ostream.h" #include -#include #include #include @@ -165,6 +164,33 @@ void UseRangesCheck::registerMatchers(MatchFinder *Finder) { static void removeFunctionArgs(DiagnosticBuilder &Diag, const CallExpr &Call, ArrayRef Indexes, const ASTContext &Ctx) { + auto GetCommaLoc = + [&](SourceLocation BeginLoc, + SourceLocation EndLoc) -> std::optional { +auto Invalid = false; +auto SourceText = Lexer::getSourceText( +CharSourceRange::getCharRange({BeginLoc, EndLoc}), +Ctx.getSourceManager(), Ctx.getLangOpts(), &Invalid); +assert(!Invalid); + +size_t I = 0; +while (I < SourceText.size() && SourceText[I] != ',') { + I++; +} + +if (I < SourceText.size()) { + // also remove space after , + size_t J = I + 1; + while (J < SourceText.size() && SourceText[J] == ' ') { +J++; + } + + return std::make_optional(CharSourceRange::getCharRange( + {BeginLoc.getLocWithOffset(I), BeginLoc.getLocWithOffset(J)})); +} +return std::nullopt; + }; + llvm::SmallVector Sorted(Indexes); llvm::sort(Sorted); // Keep track of commas removed @@ -176,20 +202,25 @@ static void removeFunctionArgs(DiagnosticBuilder &Diag, const CallExpr &Call, if (Commas[Index]) { if (Index + 1 < Call.getNumArgs()) { // Remove the next comma -Commas[Index + 1] = true; const Expr *NextArg = Call.getArg(Index + 1); -Diag << FixItHint::CreateRemoval(CharSourceRange::getTokenRange( -{Arg->getBeginLoc(), NextArg->getBeginLoc().getLocWithOffset(-1)})); - } else { -Diag << FixItHint::CreateRemoval(Arg->getSourceRange()); +auto CommaLoc = GetCommaLoc(Arg->getEndLoc().getLocWithOffset(1), +NextArg->getBeginLoc()); +if (CommaLoc) { + Commas[Index + 1] = true; + Diag << FixItHint::CreateRemoval(*CommaLoc); +} } } else { // At this point we know Index > 0 b
[clang-tools-extra] [clang-tidy][clangd] Fixed removeFunctionArgs don't remove comma for use-ranges check (PR #118568)
https://github.com/chomosuke updated https://github.com/llvm/llvm-project/pull/118568 >From b43a2602025bdacea06ced5171904fb5d765de9f Mon Sep 17 00:00:00 2001 From: chomosuke Date: Tue, 3 Dec 2024 07:10:33 + Subject: [PATCH 1/5] fixed removeFunctionArgs don't remove comma --- .../clang-tidy/utils/UseRangesCheck.cpp | 19 ++- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/clang-tools-extra/clang-tidy/utils/UseRangesCheck.cpp b/clang-tools-extra/clang-tidy/utils/UseRangesCheck.cpp index aba4d17ccd035eb..88cba70b931d5dd 100644 --- a/clang-tools-extra/clang-tidy/utils/UseRangesCheck.cpp +++ b/clang-tools-extra/clang-tidy/utils/UseRangesCheck.cpp @@ -28,6 +28,7 @@ #include "llvm/ADT/Twine.h" #include "llvm/Support/raw_ostream.h" #include +#include #include #include @@ -173,21 +174,21 @@ static void removeFunctionArgs(DiagnosticBuilder &Diag, const CallExpr &Call, for (unsigned Index : Sorted) { const Expr *Arg = Call.getArg(Index); if (Commas[Index]) { - if (Index >= Commas.size()) { -Diag << FixItHint::CreateRemoval(Arg->getSourceRange()); - } else { + if (Index + 1 < Call.getNumArgs()) { // Remove the next comma Commas[Index + 1] = true; +const Expr *NextArg = Call.getArg(Index + 1); Diag << FixItHint::CreateRemoval(CharSourceRange::getTokenRange( -{Arg->getBeginLoc(), - Lexer::getLocForEndOfToken( - Arg->getEndLoc(), 0, Ctx.getSourceManager(), Ctx.getLangOpts()) - .getLocWithOffset(1)})); +{Arg->getBeginLoc(), NextArg->getBeginLoc().getLocWithOffset(-1)})); + } else { +Diag << FixItHint::CreateRemoval(Arg->getSourceRange()); } } else { - Diag << FixItHint::CreateRemoval(CharSourceRange::getTokenRange( - Arg->getBeginLoc().getLocWithOffset(-1), Arg->getEndLoc())); + // At this point we know Index > 0 because `Commas[0] = true` earlier Commas[Index] = true; + const Expr *PrevArg = Call.getArg(Index - 1); + Diag << FixItHint::CreateRemoval(CharSourceRange::getTokenRange( + PrevArg->getEndLoc().getLocWithOffset(1), Arg->getEndLoc())); } } } >From 644c8491e0fba203e89595827781d0c2c0609081 Mon Sep 17 00:00:00 2001 From: chomosuke Date: Mon, 23 Dec 2024 05:17:08 +1100 Subject: [PATCH 2/5] find , and remove only , --- .../clang-tidy/utils/UseRangesCheck.cpp | 49 +++ 1 file changed, 40 insertions(+), 9 deletions(-) diff --git a/clang-tools-extra/clang-tidy/utils/UseRangesCheck.cpp b/clang-tools-extra/clang-tidy/utils/UseRangesCheck.cpp index 88cba70b931d5dd..8b8e44a9898fdd3 100644 --- a/clang-tools-extra/clang-tidy/utils/UseRangesCheck.cpp +++ b/clang-tools-extra/clang-tidy/utils/UseRangesCheck.cpp @@ -28,7 +28,6 @@ #include "llvm/ADT/Twine.h" #include "llvm/Support/raw_ostream.h" #include -#include #include #include @@ -165,6 +164,33 @@ void UseRangesCheck::registerMatchers(MatchFinder *Finder) { static void removeFunctionArgs(DiagnosticBuilder &Diag, const CallExpr &Call, ArrayRef Indexes, const ASTContext &Ctx) { + auto GetCommaLoc = + [&](SourceLocation BeginLoc, + SourceLocation EndLoc) -> std::optional { +auto Invalid = false; +auto SourceText = Lexer::getSourceText( +CharSourceRange::getCharRange({BeginLoc, EndLoc}), +Ctx.getSourceManager(), Ctx.getLangOpts(), &Invalid); +assert(!Invalid); + +size_t I = 0; +while (I < SourceText.size() && SourceText[I] != ',') { + I++; +} + +if (I < SourceText.size()) { + // also remove space after , + size_t J = I + 1; + while (J < SourceText.size() && SourceText[J] == ' ') { +J++; + } + + return std::make_optional(CharSourceRange::getCharRange( + {BeginLoc.getLocWithOffset(I), BeginLoc.getLocWithOffset(J)})); +} +return std::nullopt; + }; + llvm::SmallVector Sorted(Indexes); llvm::sort(Sorted); // Keep track of commas removed @@ -176,20 +202,25 @@ static void removeFunctionArgs(DiagnosticBuilder &Diag, const CallExpr &Call, if (Commas[Index]) { if (Index + 1 < Call.getNumArgs()) { // Remove the next comma -Commas[Index + 1] = true; const Expr *NextArg = Call.getArg(Index + 1); -Diag << FixItHint::CreateRemoval(CharSourceRange::getTokenRange( -{Arg->getBeginLoc(), NextArg->getBeginLoc().getLocWithOffset(-1)})); - } else { -Diag << FixItHint::CreateRemoval(Arg->getSourceRange()); +auto CommaLoc = GetCommaLoc(Arg->getEndLoc().getLocWithOffset(1), +NextArg->getBeginLoc()); +if (CommaLoc) { + Commas[Index + 1] = true; + Diag << FixItHint::CreateRemoval(*CommaLoc); +} } } else { // At this point we know Index > 0 b
[clang] [clang-tools-extra] [clangd][clang-tidy] Make clangd run `format::cleanupAroundReplacements()` for all code actions just as clang-tidy does (PR #118569)
https://github.com/chomosuke updated https://github.com/llvm/llvm-project/pull/118569 >From efc17a803c9c22543de7d5f9e960a7267ade1f2e Mon Sep 17 00:00:00 2001 From: chomosuke Date: Wed, 4 Dec 2024 14:42:24 + Subject: [PATCH 1/2] [clangd][clang-tidy] Make clangd run `format::cleanupAroundReplacements()` for all code actions just as clang-tidy does --- clang-tools-extra/clang-tidy/ClangTidy.cpp| 22 ++-- clang-tools-extra/clangd/Diagnostics.cpp | 35 - .../clangd/unittests/DiagnosticsTests.cpp | 51 --- .../include/clang/Tooling/Core/Replacement.h | 4 ++ clang/lib/Tooling/Core/Replacement.cpp| 16 +- 5 files changed, 99 insertions(+), 29 deletions(-) diff --git a/clang-tools-extra/clang-tidy/ClangTidy.cpp b/clang-tools-extra/clang-tidy/ClangTidy.cpp index 9c8c93c5d16c72..82331c724eaaf2 100644 --- a/clang-tools-extra/clang-tidy/ClangTidy.cpp +++ b/clang-tools-extra/clang-tidy/ClangTidy.cpp @@ -149,26 +149,12 @@ class ErrorReporter { Repl.getLength(), Repl.getReplacementText()); auto &Entry = FileReplacements[R.getFilePath()]; Replacements &Replacements = Entry.Replaces; -llvm::Error Err = Replacements.add(R); +llvm::Error Err = Replacements.addOrMerge(R); if (Err) { // FIXME: Implement better conflict handling. - llvm::errs() << "Trying to resolve conflict: " - << llvm::toString(std::move(Err)) << "\n"; - unsigned NewOffset = - Replacements.getShiftedCodePosition(R.getOffset()); - unsigned NewLength = Replacements.getShiftedCodePosition( - R.getOffset() + R.getLength()) - - NewOffset; - if (NewLength == R.getLength()) { -R = Replacement(R.getFilePath(), NewOffset, NewLength, -R.getReplacementText()); -Replacements = Replacements.merge(tooling::Replacements(R)); -CanBeApplied = true; -++AppliedFixes; - } else { -llvm::errs() -<< "Can't resolve conflict, skipping the replacement.\n"; - } + llvm::errs() + << "Can't resolve conflict, skipping the replacement: " + << llvm::toString(std::move(Err)) << '\n'; } else { CanBeApplied = true; ++AppliedFixes; diff --git a/clang-tools-extra/clangd/Diagnostics.cpp b/clang-tools-extra/clangd/Diagnostics.cpp index a59d1e7ac84096..60c6ac7256b58c 100644 --- a/clang-tools-extra/clangd/Diagnostics.cpp +++ b/clang-tools-extra/clangd/Diagnostics.cpp @@ -19,6 +19,7 @@ #include "clang/Basic/SourceLocation.h" #include "clang/Basic/SourceManager.h" #include "clang/Basic/TokenKinds.h" +#include "clang/Format/Format.h" #include "clang/Lex/Lexer.h" #include "clang/Lex/Token.h" #include "llvm/ADT/ArrayRef.h" @@ -741,7 +742,7 @@ void StoreDiags::HandleDiagnostic(DiagnosticsEngine::Level DiagLevel, return false; // Copy as we may modify the ranges. auto FixIts = Info.getFixItHints().vec(); -llvm::SmallVector Edits; +auto Replacements = std::make_optional(); for (auto &FixIt : FixIts) { // Allow fixits within a single macro-arg expansion to be applied. // This can be incorrect if the argument is expanded multiple times in @@ -761,7 +762,37 @@ void StoreDiags::HandleDiagnostic(DiagnosticsEngine::Level DiagLevel, return false; if (!isInsideMainFile(FixIt.RemoveRange.getBegin(), SM)) return false; - Edits.push_back(toTextEdit(FixIt, SM, *LangOpts)); + + auto R = tooling::Replacement(SM, FixIt.RemoveRange, FixIt.CodeToInsert, +*LangOpts); + auto Err = Replacements->addOrMerge(R); + if (Err) { +log("Skipping formatting the replacement due to conflict: {0}", +llvm::toString(std::move(Err))); +Replacements = std::nullopt; +break; + } +} + +llvm::SmallVector Edits; + +if (Replacements) { + StringRef Code = SM.getBufferData(SM.getMainFileID()); + auto Repl = format::cleanupAroundReplacements(Code, *Replacements, +format::getNoStyle()); + if (!Repl) { +log("Skipping formatting the replacement due to conflict: {0}", +llvm::toString(std::move(Repl.takeError(; +Replacements = std::nullopt; + } else { +auto Es = replacementsToEdits(Code, *Repl); +Edits.append(Es.begin(), Es.end()); + } +} +if (!Replacements) { + for (auto &FixIt : FixIts) { +Edits.push_back(toTextEdit(FixIt, SM, *LangOpts)); + } } llvm::SmallString<64> Message; diff --git a/clang-tools-extra/clangd/unitte
[clang] [clang-tools-extra] [clangd][clang-tidy] Make clangd run `format::cleanupAroundReplacements()` for all code actions just as clang-tidy does (PR #118569)
chomosuke wrote: Ping https://github.com/llvm/llvm-project/pull/118569 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [clangd] Make clangd run `format::cleanupAroundReplacements()` for all code actions just as clang-tidy does (PR #118569)
https://github.com/chomosuke edited https://github.com/llvm/llvm-project/pull/118569 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [clangd] Make clangd run `format::cleanupAroundReplacements()` for all code actions just as clang-tidy does (PR #118569)
https://github.com/chomosuke updated https://github.com/llvm/llvm-project/pull/118569 >From efc17a803c9c22543de7d5f9e960a7267ade1f2e Mon Sep 17 00:00:00 2001 From: chomosuke Date: Wed, 4 Dec 2024 14:42:24 + Subject: [PATCH 1/2] [clangd][clang-tidy] Make clangd run `format::cleanupAroundReplacements()` for all code actions just as clang-tidy does --- clang-tools-extra/clang-tidy/ClangTidy.cpp| 22 ++-- clang-tools-extra/clangd/Diagnostics.cpp | 35 - .../clangd/unittests/DiagnosticsTests.cpp | 51 --- .../include/clang/Tooling/Core/Replacement.h | 4 ++ clang/lib/Tooling/Core/Replacement.cpp| 16 +- 5 files changed, 99 insertions(+), 29 deletions(-) diff --git a/clang-tools-extra/clang-tidy/ClangTidy.cpp b/clang-tools-extra/clang-tidy/ClangTidy.cpp index 9c8c93c5d16c72..82331c724eaaf2 100644 --- a/clang-tools-extra/clang-tidy/ClangTidy.cpp +++ b/clang-tools-extra/clang-tidy/ClangTidy.cpp @@ -149,26 +149,12 @@ class ErrorReporter { Repl.getLength(), Repl.getReplacementText()); auto &Entry = FileReplacements[R.getFilePath()]; Replacements &Replacements = Entry.Replaces; -llvm::Error Err = Replacements.add(R); +llvm::Error Err = Replacements.addOrMerge(R); if (Err) { // FIXME: Implement better conflict handling. - llvm::errs() << "Trying to resolve conflict: " - << llvm::toString(std::move(Err)) << "\n"; - unsigned NewOffset = - Replacements.getShiftedCodePosition(R.getOffset()); - unsigned NewLength = Replacements.getShiftedCodePosition( - R.getOffset() + R.getLength()) - - NewOffset; - if (NewLength == R.getLength()) { -R = Replacement(R.getFilePath(), NewOffset, NewLength, -R.getReplacementText()); -Replacements = Replacements.merge(tooling::Replacements(R)); -CanBeApplied = true; -++AppliedFixes; - } else { -llvm::errs() -<< "Can't resolve conflict, skipping the replacement.\n"; - } + llvm::errs() + << "Can't resolve conflict, skipping the replacement: " + << llvm::toString(std::move(Err)) << '\n'; } else { CanBeApplied = true; ++AppliedFixes; diff --git a/clang-tools-extra/clangd/Diagnostics.cpp b/clang-tools-extra/clangd/Diagnostics.cpp index a59d1e7ac84096..60c6ac7256b58c 100644 --- a/clang-tools-extra/clangd/Diagnostics.cpp +++ b/clang-tools-extra/clangd/Diagnostics.cpp @@ -19,6 +19,7 @@ #include "clang/Basic/SourceLocation.h" #include "clang/Basic/SourceManager.h" #include "clang/Basic/TokenKinds.h" +#include "clang/Format/Format.h" #include "clang/Lex/Lexer.h" #include "clang/Lex/Token.h" #include "llvm/ADT/ArrayRef.h" @@ -741,7 +742,7 @@ void StoreDiags::HandleDiagnostic(DiagnosticsEngine::Level DiagLevel, return false; // Copy as we may modify the ranges. auto FixIts = Info.getFixItHints().vec(); -llvm::SmallVector Edits; +auto Replacements = std::make_optional(); for (auto &FixIt : FixIts) { // Allow fixits within a single macro-arg expansion to be applied. // This can be incorrect if the argument is expanded multiple times in @@ -761,7 +762,37 @@ void StoreDiags::HandleDiagnostic(DiagnosticsEngine::Level DiagLevel, return false; if (!isInsideMainFile(FixIt.RemoveRange.getBegin(), SM)) return false; - Edits.push_back(toTextEdit(FixIt, SM, *LangOpts)); + + auto R = tooling::Replacement(SM, FixIt.RemoveRange, FixIt.CodeToInsert, +*LangOpts); + auto Err = Replacements->addOrMerge(R); + if (Err) { +log("Skipping formatting the replacement due to conflict: {0}", +llvm::toString(std::move(Err))); +Replacements = std::nullopt; +break; + } +} + +llvm::SmallVector Edits; + +if (Replacements) { + StringRef Code = SM.getBufferData(SM.getMainFileID()); + auto Repl = format::cleanupAroundReplacements(Code, *Replacements, +format::getNoStyle()); + if (!Repl) { +log("Skipping formatting the replacement due to conflict: {0}", +llvm::toString(std::move(Repl.takeError(; +Replacements = std::nullopt; + } else { +auto Es = replacementsToEdits(Code, *Repl); +Edits.append(Es.begin(), Es.end()); + } +} +if (!Replacements) { + for (auto &FixIt : FixIts) { +Edits.push_back(toTextEdit(FixIt, SM, *LangOpts)); + } } llvm::SmallString<64> Message; diff --git a/clang-tools-extra/clangd/unitte
[clang-tools-extra] [clang-tidy][clangd] Fixed removeFunctionArgs don't remove comma for use-ranges check (PR #118568)
https://github.com/chomosuke updated https://github.com/llvm/llvm-project/pull/118568 >From b43a2602025bdacea06ced5171904fb5d765de9f Mon Sep 17 00:00:00 2001 From: chomosuke Date: Tue, 3 Dec 2024 07:10:33 + Subject: [PATCH 1/4] fixed removeFunctionArgs don't remove comma --- .../clang-tidy/utils/UseRangesCheck.cpp | 19 ++- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/clang-tools-extra/clang-tidy/utils/UseRangesCheck.cpp b/clang-tools-extra/clang-tidy/utils/UseRangesCheck.cpp index aba4d17ccd035e..88cba70b931d5d 100644 --- a/clang-tools-extra/clang-tidy/utils/UseRangesCheck.cpp +++ b/clang-tools-extra/clang-tidy/utils/UseRangesCheck.cpp @@ -28,6 +28,7 @@ #include "llvm/ADT/Twine.h" #include "llvm/Support/raw_ostream.h" #include +#include #include #include @@ -173,21 +174,21 @@ static void removeFunctionArgs(DiagnosticBuilder &Diag, const CallExpr &Call, for (unsigned Index : Sorted) { const Expr *Arg = Call.getArg(Index); if (Commas[Index]) { - if (Index >= Commas.size()) { -Diag << FixItHint::CreateRemoval(Arg->getSourceRange()); - } else { + if (Index + 1 < Call.getNumArgs()) { // Remove the next comma Commas[Index + 1] = true; +const Expr *NextArg = Call.getArg(Index + 1); Diag << FixItHint::CreateRemoval(CharSourceRange::getTokenRange( -{Arg->getBeginLoc(), - Lexer::getLocForEndOfToken( - Arg->getEndLoc(), 0, Ctx.getSourceManager(), Ctx.getLangOpts()) - .getLocWithOffset(1)})); +{Arg->getBeginLoc(), NextArg->getBeginLoc().getLocWithOffset(-1)})); + } else { +Diag << FixItHint::CreateRemoval(Arg->getSourceRange()); } } else { - Diag << FixItHint::CreateRemoval(CharSourceRange::getTokenRange( - Arg->getBeginLoc().getLocWithOffset(-1), Arg->getEndLoc())); + // At this point we know Index > 0 because `Commas[0] = true` earlier Commas[Index] = true; + const Expr *PrevArg = Call.getArg(Index - 1); + Diag << FixItHint::CreateRemoval(CharSourceRange::getTokenRange( + PrevArg->getEndLoc().getLocWithOffset(1), Arg->getEndLoc())); } } } >From 644c8491e0fba203e89595827781d0c2c0609081 Mon Sep 17 00:00:00 2001 From: chomosuke Date: Mon, 23 Dec 2024 05:17:08 +1100 Subject: [PATCH 2/4] find , and remove only , --- .../clang-tidy/utils/UseRangesCheck.cpp | 49 +++ 1 file changed, 40 insertions(+), 9 deletions(-) diff --git a/clang-tools-extra/clang-tidy/utils/UseRangesCheck.cpp b/clang-tools-extra/clang-tidy/utils/UseRangesCheck.cpp index 88cba70b931d5d..8b8e44a9898fdd 100644 --- a/clang-tools-extra/clang-tidy/utils/UseRangesCheck.cpp +++ b/clang-tools-extra/clang-tidy/utils/UseRangesCheck.cpp @@ -28,7 +28,6 @@ #include "llvm/ADT/Twine.h" #include "llvm/Support/raw_ostream.h" #include -#include #include #include @@ -165,6 +164,33 @@ void UseRangesCheck::registerMatchers(MatchFinder *Finder) { static void removeFunctionArgs(DiagnosticBuilder &Diag, const CallExpr &Call, ArrayRef Indexes, const ASTContext &Ctx) { + auto GetCommaLoc = + [&](SourceLocation BeginLoc, + SourceLocation EndLoc) -> std::optional { +auto Invalid = false; +auto SourceText = Lexer::getSourceText( +CharSourceRange::getCharRange({BeginLoc, EndLoc}), +Ctx.getSourceManager(), Ctx.getLangOpts(), &Invalid); +assert(!Invalid); + +size_t I = 0; +while (I < SourceText.size() && SourceText[I] != ',') { + I++; +} + +if (I < SourceText.size()) { + // also remove space after , + size_t J = I + 1; + while (J < SourceText.size() && SourceText[J] == ' ') { +J++; + } + + return std::make_optional(CharSourceRange::getCharRange( + {BeginLoc.getLocWithOffset(I), BeginLoc.getLocWithOffset(J)})); +} +return std::nullopt; + }; + llvm::SmallVector Sorted(Indexes); llvm::sort(Sorted); // Keep track of commas removed @@ -176,20 +202,25 @@ static void removeFunctionArgs(DiagnosticBuilder &Diag, const CallExpr &Call, if (Commas[Index]) { if (Index + 1 < Call.getNumArgs()) { // Remove the next comma -Commas[Index + 1] = true; const Expr *NextArg = Call.getArg(Index + 1); -Diag << FixItHint::CreateRemoval(CharSourceRange::getTokenRange( -{Arg->getBeginLoc(), NextArg->getBeginLoc().getLocWithOffset(-1)})); - } else { -Diag << FixItHint::CreateRemoval(Arg->getSourceRange()); +auto CommaLoc = GetCommaLoc(Arg->getEndLoc().getLocWithOffset(1), +NextArg->getBeginLoc()); +if (CommaLoc) { + Commas[Index + 1] = true; + Diag << FixItHint::CreateRemoval(*CommaLoc); +} } } else { // At this point we know Index > 0 becau
[clang-tools-extra] [clang-tidy][clangd] Fixed removeFunctionArgs don't remove comma for use-ranges check (PR #118568)
https://github.com/chomosuke updated https://github.com/llvm/llvm-project/pull/118568 >From b43a2602025bdacea06ced5171904fb5d765de9f Mon Sep 17 00:00:00 2001 From: chomosuke Date: Tue, 3 Dec 2024 07:10:33 + Subject: [PATCH 1/3] fixed removeFunctionArgs don't remove comma --- .../clang-tidy/utils/UseRangesCheck.cpp | 19 ++- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/clang-tools-extra/clang-tidy/utils/UseRangesCheck.cpp b/clang-tools-extra/clang-tidy/utils/UseRangesCheck.cpp index aba4d17ccd035e..88cba70b931d5d 100644 --- a/clang-tools-extra/clang-tidy/utils/UseRangesCheck.cpp +++ b/clang-tools-extra/clang-tidy/utils/UseRangesCheck.cpp @@ -28,6 +28,7 @@ #include "llvm/ADT/Twine.h" #include "llvm/Support/raw_ostream.h" #include +#include #include #include @@ -173,21 +174,21 @@ static void removeFunctionArgs(DiagnosticBuilder &Diag, const CallExpr &Call, for (unsigned Index : Sorted) { const Expr *Arg = Call.getArg(Index); if (Commas[Index]) { - if (Index >= Commas.size()) { -Diag << FixItHint::CreateRemoval(Arg->getSourceRange()); - } else { + if (Index + 1 < Call.getNumArgs()) { // Remove the next comma Commas[Index + 1] = true; +const Expr *NextArg = Call.getArg(Index + 1); Diag << FixItHint::CreateRemoval(CharSourceRange::getTokenRange( -{Arg->getBeginLoc(), - Lexer::getLocForEndOfToken( - Arg->getEndLoc(), 0, Ctx.getSourceManager(), Ctx.getLangOpts()) - .getLocWithOffset(1)})); +{Arg->getBeginLoc(), NextArg->getBeginLoc().getLocWithOffset(-1)})); + } else { +Diag << FixItHint::CreateRemoval(Arg->getSourceRange()); } } else { - Diag << FixItHint::CreateRemoval(CharSourceRange::getTokenRange( - Arg->getBeginLoc().getLocWithOffset(-1), Arg->getEndLoc())); + // At this point we know Index > 0 because `Commas[0] = true` earlier Commas[Index] = true; + const Expr *PrevArg = Call.getArg(Index - 1); + Diag << FixItHint::CreateRemoval(CharSourceRange::getTokenRange( + PrevArg->getEndLoc().getLocWithOffset(1), Arg->getEndLoc())); } } } >From 644c8491e0fba203e89595827781d0c2c0609081 Mon Sep 17 00:00:00 2001 From: chomosuke Date: Mon, 23 Dec 2024 05:17:08 +1100 Subject: [PATCH 2/3] find , and remove only , --- .../clang-tidy/utils/UseRangesCheck.cpp | 49 +++ 1 file changed, 40 insertions(+), 9 deletions(-) diff --git a/clang-tools-extra/clang-tidy/utils/UseRangesCheck.cpp b/clang-tools-extra/clang-tidy/utils/UseRangesCheck.cpp index 88cba70b931d5d..8b8e44a9898fdd 100644 --- a/clang-tools-extra/clang-tidy/utils/UseRangesCheck.cpp +++ b/clang-tools-extra/clang-tidy/utils/UseRangesCheck.cpp @@ -28,7 +28,6 @@ #include "llvm/ADT/Twine.h" #include "llvm/Support/raw_ostream.h" #include -#include #include #include @@ -165,6 +164,33 @@ void UseRangesCheck::registerMatchers(MatchFinder *Finder) { static void removeFunctionArgs(DiagnosticBuilder &Diag, const CallExpr &Call, ArrayRef Indexes, const ASTContext &Ctx) { + auto GetCommaLoc = + [&](SourceLocation BeginLoc, + SourceLocation EndLoc) -> std::optional { +auto Invalid = false; +auto SourceText = Lexer::getSourceText( +CharSourceRange::getCharRange({BeginLoc, EndLoc}), +Ctx.getSourceManager(), Ctx.getLangOpts(), &Invalid); +assert(!Invalid); + +size_t I = 0; +while (I < SourceText.size() && SourceText[I] != ',') { + I++; +} + +if (I < SourceText.size()) { + // also remove space after , + size_t J = I + 1; + while (J < SourceText.size() && SourceText[J] == ' ') { +J++; + } + + return std::make_optional(CharSourceRange::getCharRange( + {BeginLoc.getLocWithOffset(I), BeginLoc.getLocWithOffset(J)})); +} +return std::nullopt; + }; + llvm::SmallVector Sorted(Indexes); llvm::sort(Sorted); // Keep track of commas removed @@ -176,20 +202,25 @@ static void removeFunctionArgs(DiagnosticBuilder &Diag, const CallExpr &Call, if (Commas[Index]) { if (Index + 1 < Call.getNumArgs()) { // Remove the next comma -Commas[Index + 1] = true; const Expr *NextArg = Call.getArg(Index + 1); -Diag << FixItHint::CreateRemoval(CharSourceRange::getTokenRange( -{Arg->getBeginLoc(), NextArg->getBeginLoc().getLocWithOffset(-1)})); - } else { -Diag << FixItHint::CreateRemoval(Arg->getSourceRange()); +auto CommaLoc = GetCommaLoc(Arg->getEndLoc().getLocWithOffset(1), +NextArg->getBeginLoc()); +if (CommaLoc) { + Commas[Index + 1] = true; + Diag << FixItHint::CreateRemoval(*CommaLoc); +} } } else { // At this point we know Index > 0 becau
[clang-tools-extra] [clang-tidy][clangd] Fixed removeFunctionArgs don't remove comma for use-ranges check (PR #118568)
https://github.com/chomosuke edited https://github.com/llvm/llvm-project/pull/118568 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy][clangd] Fixed removeFunctionArgs don't remove comma for use-ranges check (PR #118568)
https://github.com/chomosuke edited https://github.com/llvm/llvm-project/pull/118568 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy][clangd] Fixed removeFunctionArgs don't remove comma for use-ranges check (PR #118568)
https://github.com/chomosuke edited https://github.com/llvm/llvm-project/pull/118568 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy][clangd] Fixed removeFunctionArgs don't remove comma for use-ranges check (PR #118568)
@@ -173,21 +174,21 @@ static void removeFunctionArgs(DiagnosticBuilder &Diag, const CallExpr &Call, for (unsigned Index : Sorted) { const Expr *Arg = Call.getArg(Index); if (Commas[Index]) { - if (Index >= Commas.size()) { -Diag << FixItHint::CreateRemoval(Arg->getSourceRange()); - } else { + if (Index + 1 < Call.getNumArgs()) { // Remove the next comma Commas[Index + 1] = true; +const Expr *NextArg = Call.getArg(Index + 1); Diag << FixItHint::CreateRemoval(CharSourceRange::getTokenRange( -{Arg->getBeginLoc(), - Lexer::getLocForEndOfToken( - Arg->getEndLoc(), 0, Ctx.getSourceManager(), Ctx.getLangOpts()) - .getLocWithOffset(1)})); +{Arg->getBeginLoc(), NextArg->getBeginLoc().getLocWithOffset(-1)})); chomosuke wrote: My newest commit finds the exact location of `,` and removes it, so ```diff void f(int a, int b); void g() { - f(/*a*/0, /*b*/1); + f(/*a*/0 /*b*/); } ``` I think this is better than assuming the inline comment are parameter documentation. My code would fail though, in the case of a comment between the argument and the comma containing a comma in itself, something like ```diff void f(int a, int b); void g() { - f(/*a*/0 /*bla, bla*/, /*b*/1); + f(/*a*/0 /*bla bla*/, /*b*/); } ``` In this case the comma in the comment gets removed instead of the comma after the comment. I would appreciate if someone can point me to how to avoid that but I also think it's not a big deal since I don't people put their inline comment after an argument but before the comma like that very often. https://github.com/llvm/llvm-project/pull/118568 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy][clangd] Fixed removeFunctionArgs don't remove comma for use-ranges check (PR #118568)
https://github.com/chomosuke edited https://github.com/llvm/llvm-project/pull/118568 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy][clangd] Fixed removeFunctionArgs don't remove comma for use-ranges check (PR #118568)
https://github.com/chomosuke edited https://github.com/llvm/llvm-project/pull/118568 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy][clangd] Fixed removeFunctionArgs don't remove comma for use-ranges check (PR #118568)
https://github.com/chomosuke updated https://github.com/llvm/llvm-project/pull/118568 >From b43a2602025bdacea06ced5171904fb5d765de9f Mon Sep 17 00:00:00 2001 From: chomosuke Date: Tue, 3 Dec 2024 07:10:33 + Subject: [PATCH] fixed removeFunctionArgs don't remove comma --- .../clang-tidy/utils/UseRangesCheck.cpp | 19 ++- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/clang-tools-extra/clang-tidy/utils/UseRangesCheck.cpp b/clang-tools-extra/clang-tidy/utils/UseRangesCheck.cpp index aba4d17ccd035e..88cba70b931d5d 100644 --- a/clang-tools-extra/clang-tidy/utils/UseRangesCheck.cpp +++ b/clang-tools-extra/clang-tidy/utils/UseRangesCheck.cpp @@ -28,6 +28,7 @@ #include "llvm/ADT/Twine.h" #include "llvm/Support/raw_ostream.h" #include +#include #include #include @@ -173,21 +174,21 @@ static void removeFunctionArgs(DiagnosticBuilder &Diag, const CallExpr &Call, for (unsigned Index : Sorted) { const Expr *Arg = Call.getArg(Index); if (Commas[Index]) { - if (Index >= Commas.size()) { -Diag << FixItHint::CreateRemoval(Arg->getSourceRange()); - } else { + if (Index + 1 < Call.getNumArgs()) { // Remove the next comma Commas[Index + 1] = true; +const Expr *NextArg = Call.getArg(Index + 1); Diag << FixItHint::CreateRemoval(CharSourceRange::getTokenRange( -{Arg->getBeginLoc(), - Lexer::getLocForEndOfToken( - Arg->getEndLoc(), 0, Ctx.getSourceManager(), Ctx.getLangOpts()) - .getLocWithOffset(1)})); +{Arg->getBeginLoc(), NextArg->getBeginLoc().getLocWithOffset(-1)})); + } else { +Diag << FixItHint::CreateRemoval(Arg->getSourceRange()); } } else { - Diag << FixItHint::CreateRemoval(CharSourceRange::getTokenRange( - Arg->getBeginLoc().getLocWithOffset(-1), Arg->getEndLoc())); + // At this point we know Index > 0 because `Commas[0] = true` earlier Commas[Index] = true; + const Expr *PrevArg = Call.getArg(Index - 1); + Diag << FixItHint::CreateRemoval(CharSourceRange::getTokenRange( + PrevArg->getEndLoc().getLocWithOffset(1), Arg->getEndLoc())); } } } ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy][clangd] Fixed removeFunctionArgs don't remove comma for use-ranges check (PR #118568)
https://github.com/chomosuke updated https://github.com/llvm/llvm-project/pull/118568 >From b43a2602025bdacea06ced5171904fb5d765de9f Mon Sep 17 00:00:00 2001 From: chomosuke Date: Tue, 3 Dec 2024 07:10:33 + Subject: [PATCH 1/2] fixed removeFunctionArgs don't remove comma --- .../clang-tidy/utils/UseRangesCheck.cpp | 19 ++- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/clang-tools-extra/clang-tidy/utils/UseRangesCheck.cpp b/clang-tools-extra/clang-tidy/utils/UseRangesCheck.cpp index aba4d17ccd035e..88cba70b931d5d 100644 --- a/clang-tools-extra/clang-tidy/utils/UseRangesCheck.cpp +++ b/clang-tools-extra/clang-tidy/utils/UseRangesCheck.cpp @@ -28,6 +28,7 @@ #include "llvm/ADT/Twine.h" #include "llvm/Support/raw_ostream.h" #include +#include #include #include @@ -173,21 +174,21 @@ static void removeFunctionArgs(DiagnosticBuilder &Diag, const CallExpr &Call, for (unsigned Index : Sorted) { const Expr *Arg = Call.getArg(Index); if (Commas[Index]) { - if (Index >= Commas.size()) { -Diag << FixItHint::CreateRemoval(Arg->getSourceRange()); - } else { + if (Index + 1 < Call.getNumArgs()) { // Remove the next comma Commas[Index + 1] = true; +const Expr *NextArg = Call.getArg(Index + 1); Diag << FixItHint::CreateRemoval(CharSourceRange::getTokenRange( -{Arg->getBeginLoc(), - Lexer::getLocForEndOfToken( - Arg->getEndLoc(), 0, Ctx.getSourceManager(), Ctx.getLangOpts()) - .getLocWithOffset(1)})); +{Arg->getBeginLoc(), NextArg->getBeginLoc().getLocWithOffset(-1)})); + } else { +Diag << FixItHint::CreateRemoval(Arg->getSourceRange()); } } else { - Diag << FixItHint::CreateRemoval(CharSourceRange::getTokenRange( - Arg->getBeginLoc().getLocWithOffset(-1), Arg->getEndLoc())); + // At this point we know Index > 0 because `Commas[0] = true` earlier Commas[Index] = true; + const Expr *PrevArg = Call.getArg(Index - 1); + Diag << FixItHint::CreateRemoval(CharSourceRange::getTokenRange( + PrevArg->getEndLoc().getLocWithOffset(1), Arg->getEndLoc())); } } } >From 644c8491e0fba203e89595827781d0c2c0609081 Mon Sep 17 00:00:00 2001 From: chomosuke Date: Mon, 23 Dec 2024 05:17:08 +1100 Subject: [PATCH 2/2] find , and remove only , --- .../clang-tidy/utils/UseRangesCheck.cpp | 49 +++ 1 file changed, 40 insertions(+), 9 deletions(-) diff --git a/clang-tools-extra/clang-tidy/utils/UseRangesCheck.cpp b/clang-tools-extra/clang-tidy/utils/UseRangesCheck.cpp index 88cba70b931d5d..8b8e44a9898fdd 100644 --- a/clang-tools-extra/clang-tidy/utils/UseRangesCheck.cpp +++ b/clang-tools-extra/clang-tidy/utils/UseRangesCheck.cpp @@ -28,7 +28,6 @@ #include "llvm/ADT/Twine.h" #include "llvm/Support/raw_ostream.h" #include -#include #include #include @@ -165,6 +164,33 @@ void UseRangesCheck::registerMatchers(MatchFinder *Finder) { static void removeFunctionArgs(DiagnosticBuilder &Diag, const CallExpr &Call, ArrayRef Indexes, const ASTContext &Ctx) { + auto GetCommaLoc = + [&](SourceLocation BeginLoc, + SourceLocation EndLoc) -> std::optional { +auto Invalid = false; +auto SourceText = Lexer::getSourceText( +CharSourceRange::getCharRange({BeginLoc, EndLoc}), +Ctx.getSourceManager(), Ctx.getLangOpts(), &Invalid); +assert(!Invalid); + +size_t I = 0; +while (I < SourceText.size() && SourceText[I] != ',') { + I++; +} + +if (I < SourceText.size()) { + // also remove space after , + size_t J = I + 1; + while (J < SourceText.size() && SourceText[J] == ' ') { +J++; + } + + return std::make_optional(CharSourceRange::getCharRange( + {BeginLoc.getLocWithOffset(I), BeginLoc.getLocWithOffset(J)})); +} +return std::nullopt; + }; + llvm::SmallVector Sorted(Indexes); llvm::sort(Sorted); // Keep track of commas removed @@ -176,20 +202,25 @@ static void removeFunctionArgs(DiagnosticBuilder &Diag, const CallExpr &Call, if (Commas[Index]) { if (Index + 1 < Call.getNumArgs()) { // Remove the next comma -Commas[Index + 1] = true; const Expr *NextArg = Call.getArg(Index + 1); -Diag << FixItHint::CreateRemoval(CharSourceRange::getTokenRange( -{Arg->getBeginLoc(), NextArg->getBeginLoc().getLocWithOffset(-1)})); - } else { -Diag << FixItHint::CreateRemoval(Arg->getSourceRange()); +auto CommaLoc = GetCommaLoc(Arg->getEndLoc().getLocWithOffset(1), +NextArg->getBeginLoc()); +if (CommaLoc) { + Commas[Index + 1] = true; + Diag << FixItHint::CreateRemoval(*CommaLoc); +} } } else { // At this point we know Index > 0 becau
[clang] [clang-tools-extra] [clangd] Make clangd run `format::cleanupAroundReplacements()` for all code actions just as clang-tidy does (PR #118569)
https://github.com/chomosuke updated https://github.com/llvm/llvm-project/pull/118569 >From efc17a803c9c22543de7d5f9e960a7267ade1f2e Mon Sep 17 00:00:00 2001 From: chomosuke Date: Wed, 4 Dec 2024 14:42:24 + Subject: [PATCH 1/3] [clangd][clang-tidy] Make clangd run `format::cleanupAroundReplacements()` for all code actions just as clang-tidy does --- clang-tools-extra/clang-tidy/ClangTidy.cpp| 22 ++-- clang-tools-extra/clangd/Diagnostics.cpp | 35 - .../clangd/unittests/DiagnosticsTests.cpp | 51 --- .../include/clang/Tooling/Core/Replacement.h | 4 ++ clang/lib/Tooling/Core/Replacement.cpp| 16 +- 5 files changed, 99 insertions(+), 29 deletions(-) diff --git a/clang-tools-extra/clang-tidy/ClangTidy.cpp b/clang-tools-extra/clang-tidy/ClangTidy.cpp index 9c8c93c5d16c72..82331c724eaaf2 100644 --- a/clang-tools-extra/clang-tidy/ClangTidy.cpp +++ b/clang-tools-extra/clang-tidy/ClangTidy.cpp @@ -149,26 +149,12 @@ class ErrorReporter { Repl.getLength(), Repl.getReplacementText()); auto &Entry = FileReplacements[R.getFilePath()]; Replacements &Replacements = Entry.Replaces; -llvm::Error Err = Replacements.add(R); +llvm::Error Err = Replacements.addOrMerge(R); if (Err) { // FIXME: Implement better conflict handling. - llvm::errs() << "Trying to resolve conflict: " - << llvm::toString(std::move(Err)) << "\n"; - unsigned NewOffset = - Replacements.getShiftedCodePosition(R.getOffset()); - unsigned NewLength = Replacements.getShiftedCodePosition( - R.getOffset() + R.getLength()) - - NewOffset; - if (NewLength == R.getLength()) { -R = Replacement(R.getFilePath(), NewOffset, NewLength, -R.getReplacementText()); -Replacements = Replacements.merge(tooling::Replacements(R)); -CanBeApplied = true; -++AppliedFixes; - } else { -llvm::errs() -<< "Can't resolve conflict, skipping the replacement.\n"; - } + llvm::errs() + << "Can't resolve conflict, skipping the replacement: " + << llvm::toString(std::move(Err)) << '\n'; } else { CanBeApplied = true; ++AppliedFixes; diff --git a/clang-tools-extra/clangd/Diagnostics.cpp b/clang-tools-extra/clangd/Diagnostics.cpp index a59d1e7ac84096..60c6ac7256b58c 100644 --- a/clang-tools-extra/clangd/Diagnostics.cpp +++ b/clang-tools-extra/clangd/Diagnostics.cpp @@ -19,6 +19,7 @@ #include "clang/Basic/SourceLocation.h" #include "clang/Basic/SourceManager.h" #include "clang/Basic/TokenKinds.h" +#include "clang/Format/Format.h" #include "clang/Lex/Lexer.h" #include "clang/Lex/Token.h" #include "llvm/ADT/ArrayRef.h" @@ -741,7 +742,7 @@ void StoreDiags::HandleDiagnostic(DiagnosticsEngine::Level DiagLevel, return false; // Copy as we may modify the ranges. auto FixIts = Info.getFixItHints().vec(); -llvm::SmallVector Edits; +auto Replacements = std::make_optional(); for (auto &FixIt : FixIts) { // Allow fixits within a single macro-arg expansion to be applied. // This can be incorrect if the argument is expanded multiple times in @@ -761,7 +762,37 @@ void StoreDiags::HandleDiagnostic(DiagnosticsEngine::Level DiagLevel, return false; if (!isInsideMainFile(FixIt.RemoveRange.getBegin(), SM)) return false; - Edits.push_back(toTextEdit(FixIt, SM, *LangOpts)); + + auto R = tooling::Replacement(SM, FixIt.RemoveRange, FixIt.CodeToInsert, +*LangOpts); + auto Err = Replacements->addOrMerge(R); + if (Err) { +log("Skipping formatting the replacement due to conflict: {0}", +llvm::toString(std::move(Err))); +Replacements = std::nullopt; +break; + } +} + +llvm::SmallVector Edits; + +if (Replacements) { + StringRef Code = SM.getBufferData(SM.getMainFileID()); + auto Repl = format::cleanupAroundReplacements(Code, *Replacements, +format::getNoStyle()); + if (!Repl) { +log("Skipping formatting the replacement due to conflict: {0}", +llvm::toString(std::move(Repl.takeError(; +Replacements = std::nullopt; + } else { +auto Es = replacementsToEdits(Code, *Repl); +Edits.append(Es.begin(), Es.end()); + } +} +if (!Replacements) { + for (auto &FixIt : FixIts) { +Edits.push_back(toTextEdit(FixIt, SM, *LangOpts)); + } } llvm::SmallString<64> Message; diff --git a/clang-tools-extra/clangd/unitte
[clang] [clang-tools-extra] [clangd] Make clangd run `format::cleanupAroundReplacements()` for all code actions just as clang-tidy does (PR #118569)
chomosuke wrote: Thank you for the review @llvm-beanz, I've fixed the suggestions :). https://github.com/llvm/llvm-project/pull/118569 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [clangd] Make clangd run `format::cleanupAroundReplacements()` for all code actions just as clang-tidy does (PR #118569)
https://github.com/chomosuke updated https://github.com/llvm/llvm-project/pull/118569 >From efc17a803c9c22543de7d5f9e960a7267ade1f2e Mon Sep 17 00:00:00 2001 From: chomosuke Date: Wed, 4 Dec 2024 14:42:24 + Subject: [PATCH 1/3] [clangd][clang-tidy] Make clangd run `format::cleanupAroundReplacements()` for all code actions just as clang-tidy does --- clang-tools-extra/clang-tidy/ClangTidy.cpp| 22 ++-- clang-tools-extra/clangd/Diagnostics.cpp | 35 - .../clangd/unittests/DiagnosticsTests.cpp | 51 --- .../include/clang/Tooling/Core/Replacement.h | 4 ++ clang/lib/Tooling/Core/Replacement.cpp| 16 +- 5 files changed, 99 insertions(+), 29 deletions(-) diff --git a/clang-tools-extra/clang-tidy/ClangTidy.cpp b/clang-tools-extra/clang-tidy/ClangTidy.cpp index 9c8c93c5d16c72..82331c724eaaf2 100644 --- a/clang-tools-extra/clang-tidy/ClangTidy.cpp +++ b/clang-tools-extra/clang-tidy/ClangTidy.cpp @@ -149,26 +149,12 @@ class ErrorReporter { Repl.getLength(), Repl.getReplacementText()); auto &Entry = FileReplacements[R.getFilePath()]; Replacements &Replacements = Entry.Replaces; -llvm::Error Err = Replacements.add(R); +llvm::Error Err = Replacements.addOrMerge(R); if (Err) { // FIXME: Implement better conflict handling. - llvm::errs() << "Trying to resolve conflict: " - << llvm::toString(std::move(Err)) << "\n"; - unsigned NewOffset = - Replacements.getShiftedCodePosition(R.getOffset()); - unsigned NewLength = Replacements.getShiftedCodePosition( - R.getOffset() + R.getLength()) - - NewOffset; - if (NewLength == R.getLength()) { -R = Replacement(R.getFilePath(), NewOffset, NewLength, -R.getReplacementText()); -Replacements = Replacements.merge(tooling::Replacements(R)); -CanBeApplied = true; -++AppliedFixes; - } else { -llvm::errs() -<< "Can't resolve conflict, skipping the replacement.\n"; - } + llvm::errs() + << "Can't resolve conflict, skipping the replacement: " + << llvm::toString(std::move(Err)) << '\n'; } else { CanBeApplied = true; ++AppliedFixes; diff --git a/clang-tools-extra/clangd/Diagnostics.cpp b/clang-tools-extra/clangd/Diagnostics.cpp index a59d1e7ac84096..60c6ac7256b58c 100644 --- a/clang-tools-extra/clangd/Diagnostics.cpp +++ b/clang-tools-extra/clangd/Diagnostics.cpp @@ -19,6 +19,7 @@ #include "clang/Basic/SourceLocation.h" #include "clang/Basic/SourceManager.h" #include "clang/Basic/TokenKinds.h" +#include "clang/Format/Format.h" #include "clang/Lex/Lexer.h" #include "clang/Lex/Token.h" #include "llvm/ADT/ArrayRef.h" @@ -741,7 +742,7 @@ void StoreDiags::HandleDiagnostic(DiagnosticsEngine::Level DiagLevel, return false; // Copy as we may modify the ranges. auto FixIts = Info.getFixItHints().vec(); -llvm::SmallVector Edits; +auto Replacements = std::make_optional(); for (auto &FixIt : FixIts) { // Allow fixits within a single macro-arg expansion to be applied. // This can be incorrect if the argument is expanded multiple times in @@ -761,7 +762,37 @@ void StoreDiags::HandleDiagnostic(DiagnosticsEngine::Level DiagLevel, return false; if (!isInsideMainFile(FixIt.RemoveRange.getBegin(), SM)) return false; - Edits.push_back(toTextEdit(FixIt, SM, *LangOpts)); + + auto R = tooling::Replacement(SM, FixIt.RemoveRange, FixIt.CodeToInsert, +*LangOpts); + auto Err = Replacements->addOrMerge(R); + if (Err) { +log("Skipping formatting the replacement due to conflict: {0}", +llvm::toString(std::move(Err))); +Replacements = std::nullopt; +break; + } +} + +llvm::SmallVector Edits; + +if (Replacements) { + StringRef Code = SM.getBufferData(SM.getMainFileID()); + auto Repl = format::cleanupAroundReplacements(Code, *Replacements, +format::getNoStyle()); + if (!Repl) { +log("Skipping formatting the replacement due to conflict: {0}", +llvm::toString(std::move(Repl.takeError(; +Replacements = std::nullopt; + } else { +auto Es = replacementsToEdits(Code, *Repl); +Edits.append(Es.begin(), Es.end()); + } +} +if (!Replacements) { + for (auto &FixIt : FixIts) { +Edits.push_back(toTextEdit(FixIt, SM, *LangOpts)); + } } llvm::SmallString<64> Message; diff --git a/clang-tools-extra/clangd/unitte
[clang-tools-extra] [clang-tidy][clangd] Fixed removeFunctionArgs don't remove comma for use-ranges check (PR #118568)
https://github.com/chomosuke updated https://github.com/llvm/llvm-project/pull/118568 >From b43a2602025bdacea06ced5171904fb5d765de9f Mon Sep 17 00:00:00 2001 From: chomosuke Date: Tue, 3 Dec 2024 07:10:33 + Subject: [PATCH 1/4] fixed removeFunctionArgs don't remove comma --- .../clang-tidy/utils/UseRangesCheck.cpp | 19 ++- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/clang-tools-extra/clang-tidy/utils/UseRangesCheck.cpp b/clang-tools-extra/clang-tidy/utils/UseRangesCheck.cpp index aba4d17ccd035e..88cba70b931d5d 100644 --- a/clang-tools-extra/clang-tidy/utils/UseRangesCheck.cpp +++ b/clang-tools-extra/clang-tidy/utils/UseRangesCheck.cpp @@ -28,6 +28,7 @@ #include "llvm/ADT/Twine.h" #include "llvm/Support/raw_ostream.h" #include +#include #include #include @@ -173,21 +174,21 @@ static void removeFunctionArgs(DiagnosticBuilder &Diag, const CallExpr &Call, for (unsigned Index : Sorted) { const Expr *Arg = Call.getArg(Index); if (Commas[Index]) { - if (Index >= Commas.size()) { -Diag << FixItHint::CreateRemoval(Arg->getSourceRange()); - } else { + if (Index + 1 < Call.getNumArgs()) { // Remove the next comma Commas[Index + 1] = true; +const Expr *NextArg = Call.getArg(Index + 1); Diag << FixItHint::CreateRemoval(CharSourceRange::getTokenRange( -{Arg->getBeginLoc(), - Lexer::getLocForEndOfToken( - Arg->getEndLoc(), 0, Ctx.getSourceManager(), Ctx.getLangOpts()) - .getLocWithOffset(1)})); +{Arg->getBeginLoc(), NextArg->getBeginLoc().getLocWithOffset(-1)})); + } else { +Diag << FixItHint::CreateRemoval(Arg->getSourceRange()); } } else { - Diag << FixItHint::CreateRemoval(CharSourceRange::getTokenRange( - Arg->getBeginLoc().getLocWithOffset(-1), Arg->getEndLoc())); + // At this point we know Index > 0 because `Commas[0] = true` earlier Commas[Index] = true; + const Expr *PrevArg = Call.getArg(Index - 1); + Diag << FixItHint::CreateRemoval(CharSourceRange::getTokenRange( + PrevArg->getEndLoc().getLocWithOffset(1), Arg->getEndLoc())); } } } >From 644c8491e0fba203e89595827781d0c2c0609081 Mon Sep 17 00:00:00 2001 From: chomosuke Date: Mon, 23 Dec 2024 05:17:08 +1100 Subject: [PATCH 2/4] find , and remove only , --- .../clang-tidy/utils/UseRangesCheck.cpp | 49 +++ 1 file changed, 40 insertions(+), 9 deletions(-) diff --git a/clang-tools-extra/clang-tidy/utils/UseRangesCheck.cpp b/clang-tools-extra/clang-tidy/utils/UseRangesCheck.cpp index 88cba70b931d5d..8b8e44a9898fdd 100644 --- a/clang-tools-extra/clang-tidy/utils/UseRangesCheck.cpp +++ b/clang-tools-extra/clang-tidy/utils/UseRangesCheck.cpp @@ -28,7 +28,6 @@ #include "llvm/ADT/Twine.h" #include "llvm/Support/raw_ostream.h" #include -#include #include #include @@ -165,6 +164,33 @@ void UseRangesCheck::registerMatchers(MatchFinder *Finder) { static void removeFunctionArgs(DiagnosticBuilder &Diag, const CallExpr &Call, ArrayRef Indexes, const ASTContext &Ctx) { + auto GetCommaLoc = + [&](SourceLocation BeginLoc, + SourceLocation EndLoc) -> std::optional { +auto Invalid = false; +auto SourceText = Lexer::getSourceText( +CharSourceRange::getCharRange({BeginLoc, EndLoc}), +Ctx.getSourceManager(), Ctx.getLangOpts(), &Invalid); +assert(!Invalid); + +size_t I = 0; +while (I < SourceText.size() && SourceText[I] != ',') { + I++; +} + +if (I < SourceText.size()) { + // also remove space after , + size_t J = I + 1; + while (J < SourceText.size() && SourceText[J] == ' ') { +J++; + } + + return std::make_optional(CharSourceRange::getCharRange( + {BeginLoc.getLocWithOffset(I), BeginLoc.getLocWithOffset(J)})); +} +return std::nullopt; + }; + llvm::SmallVector Sorted(Indexes); llvm::sort(Sorted); // Keep track of commas removed @@ -176,20 +202,25 @@ static void removeFunctionArgs(DiagnosticBuilder &Diag, const CallExpr &Call, if (Commas[Index]) { if (Index + 1 < Call.getNumArgs()) { // Remove the next comma -Commas[Index + 1] = true; const Expr *NextArg = Call.getArg(Index + 1); -Diag << FixItHint::CreateRemoval(CharSourceRange::getTokenRange( -{Arg->getBeginLoc(), NextArg->getBeginLoc().getLocWithOffset(-1)})); - } else { -Diag << FixItHint::CreateRemoval(Arg->getSourceRange()); +auto CommaLoc = GetCommaLoc(Arg->getEndLoc().getLocWithOffset(1), +NextArg->getBeginLoc()); +if (CommaLoc) { + Commas[Index + 1] = true; + Diag << FixItHint::CreateRemoval(*CommaLoc); +} } } else { // At this point we know Index > 0 becau
[clang-tools-extra] [clang-tidy][clangd] Fixed removeFunctionArgs don't remove comma for use-ranges check (PR #118568)
https://github.com/chomosuke updated https://github.com/llvm/llvm-project/pull/118568 >From b43a2602025bdacea06ced5171904fb5d765de9f Mon Sep 17 00:00:00 2001 From: chomosuke Date: Tue, 3 Dec 2024 07:10:33 + Subject: [PATCH 1/4] fixed removeFunctionArgs don't remove comma --- .../clang-tidy/utils/UseRangesCheck.cpp | 19 ++- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/clang-tools-extra/clang-tidy/utils/UseRangesCheck.cpp b/clang-tools-extra/clang-tidy/utils/UseRangesCheck.cpp index aba4d17ccd035e..88cba70b931d5d 100644 --- a/clang-tools-extra/clang-tidy/utils/UseRangesCheck.cpp +++ b/clang-tools-extra/clang-tidy/utils/UseRangesCheck.cpp @@ -28,6 +28,7 @@ #include "llvm/ADT/Twine.h" #include "llvm/Support/raw_ostream.h" #include +#include #include #include @@ -173,21 +174,21 @@ static void removeFunctionArgs(DiagnosticBuilder &Diag, const CallExpr &Call, for (unsigned Index : Sorted) { const Expr *Arg = Call.getArg(Index); if (Commas[Index]) { - if (Index >= Commas.size()) { -Diag << FixItHint::CreateRemoval(Arg->getSourceRange()); - } else { + if (Index + 1 < Call.getNumArgs()) { // Remove the next comma Commas[Index + 1] = true; +const Expr *NextArg = Call.getArg(Index + 1); Diag << FixItHint::CreateRemoval(CharSourceRange::getTokenRange( -{Arg->getBeginLoc(), - Lexer::getLocForEndOfToken( - Arg->getEndLoc(), 0, Ctx.getSourceManager(), Ctx.getLangOpts()) - .getLocWithOffset(1)})); +{Arg->getBeginLoc(), NextArg->getBeginLoc().getLocWithOffset(-1)})); + } else { +Diag << FixItHint::CreateRemoval(Arg->getSourceRange()); } } else { - Diag << FixItHint::CreateRemoval(CharSourceRange::getTokenRange( - Arg->getBeginLoc().getLocWithOffset(-1), Arg->getEndLoc())); + // At this point we know Index > 0 because `Commas[0] = true` earlier Commas[Index] = true; + const Expr *PrevArg = Call.getArg(Index - 1); + Diag << FixItHint::CreateRemoval(CharSourceRange::getTokenRange( + PrevArg->getEndLoc().getLocWithOffset(1), Arg->getEndLoc())); } } } >From 644c8491e0fba203e89595827781d0c2c0609081 Mon Sep 17 00:00:00 2001 From: chomosuke Date: Mon, 23 Dec 2024 05:17:08 +1100 Subject: [PATCH 2/4] find , and remove only , --- .../clang-tidy/utils/UseRangesCheck.cpp | 49 +++ 1 file changed, 40 insertions(+), 9 deletions(-) diff --git a/clang-tools-extra/clang-tidy/utils/UseRangesCheck.cpp b/clang-tools-extra/clang-tidy/utils/UseRangesCheck.cpp index 88cba70b931d5d..8b8e44a9898fdd 100644 --- a/clang-tools-extra/clang-tidy/utils/UseRangesCheck.cpp +++ b/clang-tools-extra/clang-tidy/utils/UseRangesCheck.cpp @@ -28,7 +28,6 @@ #include "llvm/ADT/Twine.h" #include "llvm/Support/raw_ostream.h" #include -#include #include #include @@ -165,6 +164,33 @@ void UseRangesCheck::registerMatchers(MatchFinder *Finder) { static void removeFunctionArgs(DiagnosticBuilder &Diag, const CallExpr &Call, ArrayRef Indexes, const ASTContext &Ctx) { + auto GetCommaLoc = + [&](SourceLocation BeginLoc, + SourceLocation EndLoc) -> std::optional { +auto Invalid = false; +auto SourceText = Lexer::getSourceText( +CharSourceRange::getCharRange({BeginLoc, EndLoc}), +Ctx.getSourceManager(), Ctx.getLangOpts(), &Invalid); +assert(!Invalid); + +size_t I = 0; +while (I < SourceText.size() && SourceText[I] != ',') { + I++; +} + +if (I < SourceText.size()) { + // also remove space after , + size_t J = I + 1; + while (J < SourceText.size() && SourceText[J] == ' ') { +J++; + } + + return std::make_optional(CharSourceRange::getCharRange( + {BeginLoc.getLocWithOffset(I), BeginLoc.getLocWithOffset(J)})); +} +return std::nullopt; + }; + llvm::SmallVector Sorted(Indexes); llvm::sort(Sorted); // Keep track of commas removed @@ -176,20 +202,25 @@ static void removeFunctionArgs(DiagnosticBuilder &Diag, const CallExpr &Call, if (Commas[Index]) { if (Index + 1 < Call.getNumArgs()) { // Remove the next comma -Commas[Index + 1] = true; const Expr *NextArg = Call.getArg(Index + 1); -Diag << FixItHint::CreateRemoval(CharSourceRange::getTokenRange( -{Arg->getBeginLoc(), NextArg->getBeginLoc().getLocWithOffset(-1)})); - } else { -Diag << FixItHint::CreateRemoval(Arg->getSourceRange()); +auto CommaLoc = GetCommaLoc(Arg->getEndLoc().getLocWithOffset(1), +NextArg->getBeginLoc()); +if (CommaLoc) { + Commas[Index + 1] = true; + Diag << FixItHint::CreateRemoval(*CommaLoc); +} } } else { // At this point we know Index > 0 becau
[clang] [clang-tools-extra] [clangd] Make clangd run `format::cleanupAroundReplacements()` for all code actions just as clang-tidy does (PR #118569)
https://github.com/chomosuke updated https://github.com/llvm/llvm-project/pull/118569 >From efc17a803c9c22543de7d5f9e960a7267ade1f2e Mon Sep 17 00:00:00 2001 From: chomosuke Date: Wed, 4 Dec 2024 14:42:24 + Subject: [PATCH 1/3] [clangd][clang-tidy] Make clangd run `format::cleanupAroundReplacements()` for all code actions just as clang-tidy does --- clang-tools-extra/clang-tidy/ClangTidy.cpp| 22 ++-- clang-tools-extra/clangd/Diagnostics.cpp | 35 - .../clangd/unittests/DiagnosticsTests.cpp | 51 --- .../include/clang/Tooling/Core/Replacement.h | 4 ++ clang/lib/Tooling/Core/Replacement.cpp| 16 +- 5 files changed, 99 insertions(+), 29 deletions(-) diff --git a/clang-tools-extra/clang-tidy/ClangTidy.cpp b/clang-tools-extra/clang-tidy/ClangTidy.cpp index 9c8c93c5d16c72..82331c724eaaf2 100644 --- a/clang-tools-extra/clang-tidy/ClangTidy.cpp +++ b/clang-tools-extra/clang-tidy/ClangTidy.cpp @@ -149,26 +149,12 @@ class ErrorReporter { Repl.getLength(), Repl.getReplacementText()); auto &Entry = FileReplacements[R.getFilePath()]; Replacements &Replacements = Entry.Replaces; -llvm::Error Err = Replacements.add(R); +llvm::Error Err = Replacements.addOrMerge(R); if (Err) { // FIXME: Implement better conflict handling. - llvm::errs() << "Trying to resolve conflict: " - << llvm::toString(std::move(Err)) << "\n"; - unsigned NewOffset = - Replacements.getShiftedCodePosition(R.getOffset()); - unsigned NewLength = Replacements.getShiftedCodePosition( - R.getOffset() + R.getLength()) - - NewOffset; - if (NewLength == R.getLength()) { -R = Replacement(R.getFilePath(), NewOffset, NewLength, -R.getReplacementText()); -Replacements = Replacements.merge(tooling::Replacements(R)); -CanBeApplied = true; -++AppliedFixes; - } else { -llvm::errs() -<< "Can't resolve conflict, skipping the replacement.\n"; - } + llvm::errs() + << "Can't resolve conflict, skipping the replacement: " + << llvm::toString(std::move(Err)) << '\n'; } else { CanBeApplied = true; ++AppliedFixes; diff --git a/clang-tools-extra/clangd/Diagnostics.cpp b/clang-tools-extra/clangd/Diagnostics.cpp index a59d1e7ac84096..60c6ac7256b58c 100644 --- a/clang-tools-extra/clangd/Diagnostics.cpp +++ b/clang-tools-extra/clangd/Diagnostics.cpp @@ -19,6 +19,7 @@ #include "clang/Basic/SourceLocation.h" #include "clang/Basic/SourceManager.h" #include "clang/Basic/TokenKinds.h" +#include "clang/Format/Format.h" #include "clang/Lex/Lexer.h" #include "clang/Lex/Token.h" #include "llvm/ADT/ArrayRef.h" @@ -741,7 +742,7 @@ void StoreDiags::HandleDiagnostic(DiagnosticsEngine::Level DiagLevel, return false; // Copy as we may modify the ranges. auto FixIts = Info.getFixItHints().vec(); -llvm::SmallVector Edits; +auto Replacements = std::make_optional(); for (auto &FixIt : FixIts) { // Allow fixits within a single macro-arg expansion to be applied. // This can be incorrect if the argument is expanded multiple times in @@ -761,7 +762,37 @@ void StoreDiags::HandleDiagnostic(DiagnosticsEngine::Level DiagLevel, return false; if (!isInsideMainFile(FixIt.RemoveRange.getBegin(), SM)) return false; - Edits.push_back(toTextEdit(FixIt, SM, *LangOpts)); + + auto R = tooling::Replacement(SM, FixIt.RemoveRange, FixIt.CodeToInsert, +*LangOpts); + auto Err = Replacements->addOrMerge(R); + if (Err) { +log("Skipping formatting the replacement due to conflict: {0}", +llvm::toString(std::move(Err))); +Replacements = std::nullopt; +break; + } +} + +llvm::SmallVector Edits; + +if (Replacements) { + StringRef Code = SM.getBufferData(SM.getMainFileID()); + auto Repl = format::cleanupAroundReplacements(Code, *Replacements, +format::getNoStyle()); + if (!Repl) { +log("Skipping formatting the replacement due to conflict: {0}", +llvm::toString(std::move(Repl.takeError(; +Replacements = std::nullopt; + } else { +auto Es = replacementsToEdits(Code, *Repl); +Edits.append(Es.begin(), Es.end()); + } +} +if (!Replacements) { + for (auto &FixIt : FixIts) { +Edits.push_back(toTextEdit(FixIt, SM, *LangOpts)); + } } llvm::SmallString<64> Message; diff --git a/clang-tools-extra/clangd/unitte
[clang-tools-extra] [clang-tidy][clangd] Fixed removeFunctionArgs don't remove comma for use-ranges check (PR #118568)
https://github.com/chomosuke updated https://github.com/llvm/llvm-project/pull/118568 >From b43a2602025bdacea06ced5171904fb5d765de9f Mon Sep 17 00:00:00 2001 From: chomosuke Date: Tue, 3 Dec 2024 07:10:33 + Subject: [PATCH 1/3] fixed removeFunctionArgs don't remove comma --- .../clang-tidy/utils/UseRangesCheck.cpp | 19 ++- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/clang-tools-extra/clang-tidy/utils/UseRangesCheck.cpp b/clang-tools-extra/clang-tidy/utils/UseRangesCheck.cpp index aba4d17ccd035e..88cba70b931d5d 100644 --- a/clang-tools-extra/clang-tidy/utils/UseRangesCheck.cpp +++ b/clang-tools-extra/clang-tidy/utils/UseRangesCheck.cpp @@ -28,6 +28,7 @@ #include "llvm/ADT/Twine.h" #include "llvm/Support/raw_ostream.h" #include +#include #include #include @@ -173,21 +174,21 @@ static void removeFunctionArgs(DiagnosticBuilder &Diag, const CallExpr &Call, for (unsigned Index : Sorted) { const Expr *Arg = Call.getArg(Index); if (Commas[Index]) { - if (Index >= Commas.size()) { -Diag << FixItHint::CreateRemoval(Arg->getSourceRange()); - } else { + if (Index + 1 < Call.getNumArgs()) { // Remove the next comma Commas[Index + 1] = true; +const Expr *NextArg = Call.getArg(Index + 1); Diag << FixItHint::CreateRemoval(CharSourceRange::getTokenRange( -{Arg->getBeginLoc(), - Lexer::getLocForEndOfToken( - Arg->getEndLoc(), 0, Ctx.getSourceManager(), Ctx.getLangOpts()) - .getLocWithOffset(1)})); +{Arg->getBeginLoc(), NextArg->getBeginLoc().getLocWithOffset(-1)})); + } else { +Diag << FixItHint::CreateRemoval(Arg->getSourceRange()); } } else { - Diag << FixItHint::CreateRemoval(CharSourceRange::getTokenRange( - Arg->getBeginLoc().getLocWithOffset(-1), Arg->getEndLoc())); + // At this point we know Index > 0 because `Commas[0] = true` earlier Commas[Index] = true; + const Expr *PrevArg = Call.getArg(Index - 1); + Diag << FixItHint::CreateRemoval(CharSourceRange::getTokenRange( + PrevArg->getEndLoc().getLocWithOffset(1), Arg->getEndLoc())); } } } >From 644c8491e0fba203e89595827781d0c2c0609081 Mon Sep 17 00:00:00 2001 From: chomosuke Date: Mon, 23 Dec 2024 05:17:08 +1100 Subject: [PATCH 2/3] find , and remove only , --- .../clang-tidy/utils/UseRangesCheck.cpp | 49 +++ 1 file changed, 40 insertions(+), 9 deletions(-) diff --git a/clang-tools-extra/clang-tidy/utils/UseRangesCheck.cpp b/clang-tools-extra/clang-tidy/utils/UseRangesCheck.cpp index 88cba70b931d5d..8b8e44a9898fdd 100644 --- a/clang-tools-extra/clang-tidy/utils/UseRangesCheck.cpp +++ b/clang-tools-extra/clang-tidy/utils/UseRangesCheck.cpp @@ -28,7 +28,6 @@ #include "llvm/ADT/Twine.h" #include "llvm/Support/raw_ostream.h" #include -#include #include #include @@ -165,6 +164,33 @@ void UseRangesCheck::registerMatchers(MatchFinder *Finder) { static void removeFunctionArgs(DiagnosticBuilder &Diag, const CallExpr &Call, ArrayRef Indexes, const ASTContext &Ctx) { + auto GetCommaLoc = + [&](SourceLocation BeginLoc, + SourceLocation EndLoc) -> std::optional { +auto Invalid = false; +auto SourceText = Lexer::getSourceText( +CharSourceRange::getCharRange({BeginLoc, EndLoc}), +Ctx.getSourceManager(), Ctx.getLangOpts(), &Invalid); +assert(!Invalid); + +size_t I = 0; +while (I < SourceText.size() && SourceText[I] != ',') { + I++; +} + +if (I < SourceText.size()) { + // also remove space after , + size_t J = I + 1; + while (J < SourceText.size() && SourceText[J] == ' ') { +J++; + } + + return std::make_optional(CharSourceRange::getCharRange( + {BeginLoc.getLocWithOffset(I), BeginLoc.getLocWithOffset(J)})); +} +return std::nullopt; + }; + llvm::SmallVector Sorted(Indexes); llvm::sort(Sorted); // Keep track of commas removed @@ -176,20 +202,25 @@ static void removeFunctionArgs(DiagnosticBuilder &Diag, const CallExpr &Call, if (Commas[Index]) { if (Index + 1 < Call.getNumArgs()) { // Remove the next comma -Commas[Index + 1] = true; const Expr *NextArg = Call.getArg(Index + 1); -Diag << FixItHint::CreateRemoval(CharSourceRange::getTokenRange( -{Arg->getBeginLoc(), NextArg->getBeginLoc().getLocWithOffset(-1)})); - } else { -Diag << FixItHint::CreateRemoval(Arg->getSourceRange()); +auto CommaLoc = GetCommaLoc(Arg->getEndLoc().getLocWithOffset(1), +NextArg->getBeginLoc()); +if (CommaLoc) { + Commas[Index + 1] = true; + Diag << FixItHint::CreateRemoval(*CommaLoc); +} } } else { // At this point we know Index > 0 becau
[clang-tools-extra] [clang-tidy][clangd] Fixed removeFunctionArgs don't remove comma for use-ranges check (PR #118568)
https://github.com/chomosuke updated https://github.com/llvm/llvm-project/pull/118568 >From b43a2602025bdacea06ced5171904fb5d765de9f Mon Sep 17 00:00:00 2001 From: chomosuke Date: Tue, 3 Dec 2024 07:10:33 + Subject: [PATCH 1/3] fixed removeFunctionArgs don't remove comma --- .../clang-tidy/utils/UseRangesCheck.cpp | 19 ++- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/clang-tools-extra/clang-tidy/utils/UseRangesCheck.cpp b/clang-tools-extra/clang-tidy/utils/UseRangesCheck.cpp index aba4d17ccd035e..88cba70b931d5d 100644 --- a/clang-tools-extra/clang-tidy/utils/UseRangesCheck.cpp +++ b/clang-tools-extra/clang-tidy/utils/UseRangesCheck.cpp @@ -28,6 +28,7 @@ #include "llvm/ADT/Twine.h" #include "llvm/Support/raw_ostream.h" #include +#include #include #include @@ -173,21 +174,21 @@ static void removeFunctionArgs(DiagnosticBuilder &Diag, const CallExpr &Call, for (unsigned Index : Sorted) { const Expr *Arg = Call.getArg(Index); if (Commas[Index]) { - if (Index >= Commas.size()) { -Diag << FixItHint::CreateRemoval(Arg->getSourceRange()); - } else { + if (Index + 1 < Call.getNumArgs()) { // Remove the next comma Commas[Index + 1] = true; +const Expr *NextArg = Call.getArg(Index + 1); Diag << FixItHint::CreateRemoval(CharSourceRange::getTokenRange( -{Arg->getBeginLoc(), - Lexer::getLocForEndOfToken( - Arg->getEndLoc(), 0, Ctx.getSourceManager(), Ctx.getLangOpts()) - .getLocWithOffset(1)})); +{Arg->getBeginLoc(), NextArg->getBeginLoc().getLocWithOffset(-1)})); + } else { +Diag << FixItHint::CreateRemoval(Arg->getSourceRange()); } } else { - Diag << FixItHint::CreateRemoval(CharSourceRange::getTokenRange( - Arg->getBeginLoc().getLocWithOffset(-1), Arg->getEndLoc())); + // At this point we know Index > 0 because `Commas[0] = true` earlier Commas[Index] = true; + const Expr *PrevArg = Call.getArg(Index - 1); + Diag << FixItHint::CreateRemoval(CharSourceRange::getTokenRange( + PrevArg->getEndLoc().getLocWithOffset(1), Arg->getEndLoc())); } } } >From 644c8491e0fba203e89595827781d0c2c0609081 Mon Sep 17 00:00:00 2001 From: chomosuke Date: Mon, 23 Dec 2024 05:17:08 +1100 Subject: [PATCH 2/3] find , and remove only , --- .../clang-tidy/utils/UseRangesCheck.cpp | 49 +++ 1 file changed, 40 insertions(+), 9 deletions(-) diff --git a/clang-tools-extra/clang-tidy/utils/UseRangesCheck.cpp b/clang-tools-extra/clang-tidy/utils/UseRangesCheck.cpp index 88cba70b931d5d..8b8e44a9898fdd 100644 --- a/clang-tools-extra/clang-tidy/utils/UseRangesCheck.cpp +++ b/clang-tools-extra/clang-tidy/utils/UseRangesCheck.cpp @@ -28,7 +28,6 @@ #include "llvm/ADT/Twine.h" #include "llvm/Support/raw_ostream.h" #include -#include #include #include @@ -165,6 +164,33 @@ void UseRangesCheck::registerMatchers(MatchFinder *Finder) { static void removeFunctionArgs(DiagnosticBuilder &Diag, const CallExpr &Call, ArrayRef Indexes, const ASTContext &Ctx) { + auto GetCommaLoc = + [&](SourceLocation BeginLoc, + SourceLocation EndLoc) -> std::optional { +auto Invalid = false; +auto SourceText = Lexer::getSourceText( +CharSourceRange::getCharRange({BeginLoc, EndLoc}), +Ctx.getSourceManager(), Ctx.getLangOpts(), &Invalid); +assert(!Invalid); + +size_t I = 0; +while (I < SourceText.size() && SourceText[I] != ',') { + I++; +} + +if (I < SourceText.size()) { + // also remove space after , + size_t J = I + 1; + while (J < SourceText.size() && SourceText[J] == ' ') { +J++; + } + + return std::make_optional(CharSourceRange::getCharRange( + {BeginLoc.getLocWithOffset(I), BeginLoc.getLocWithOffset(J)})); +} +return std::nullopt; + }; + llvm::SmallVector Sorted(Indexes); llvm::sort(Sorted); // Keep track of commas removed @@ -176,20 +202,25 @@ static void removeFunctionArgs(DiagnosticBuilder &Diag, const CallExpr &Call, if (Commas[Index]) { if (Index + 1 < Call.getNumArgs()) { // Remove the next comma -Commas[Index + 1] = true; const Expr *NextArg = Call.getArg(Index + 1); -Diag << FixItHint::CreateRemoval(CharSourceRange::getTokenRange( -{Arg->getBeginLoc(), NextArg->getBeginLoc().getLocWithOffset(-1)})); - } else { -Diag << FixItHint::CreateRemoval(Arg->getSourceRange()); +auto CommaLoc = GetCommaLoc(Arg->getEndLoc().getLocWithOffset(1), +NextArg->getBeginLoc()); +if (CommaLoc) { + Commas[Index + 1] = true; + Diag << FixItHint::CreateRemoval(*CommaLoc); +} } } else { // At this point we know Index > 0 becau
[clang-tools-extra] [clang-tidy][clangd] Fixed removeFunctionArgs don't remove comma for use-ranges check (PR #118568)
@@ -164,6 +164,33 @@ void UseRangesCheck::registerMatchers(MatchFinder *Finder) { static void removeFunctionArgs(DiagnosticBuilder &Diag, const CallExpr &Call, ArrayRef Indexes, const ASTContext &Ctx) { + auto GetCommaLoc = + [&](SourceLocation BeginLoc, + SourceLocation EndLoc) -> std::optional { +auto Invalid = false; +auto SourceText = Lexer::getSourceText( chomosuke wrote: Thank you very much, I didn't know that function existed. https://github.com/llvm/llvm-project/pull/118568 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy][clangd] Fixed removeFunctionArgs don't remove comma for use-ranges check (PR #118568)
https://github.com/chomosuke updated https://github.com/llvm/llvm-project/pull/118568 >From b43a2602025bdacea06ced5171904fb5d765de9f Mon Sep 17 00:00:00 2001 From: chomosuke Date: Tue, 3 Dec 2024 07:10:33 + Subject: [PATCH 1/5] fixed removeFunctionArgs don't remove comma --- .../clang-tidy/utils/UseRangesCheck.cpp | 19 ++- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/clang-tools-extra/clang-tidy/utils/UseRangesCheck.cpp b/clang-tools-extra/clang-tidy/utils/UseRangesCheck.cpp index aba4d17ccd035..88cba70b931d5 100644 --- a/clang-tools-extra/clang-tidy/utils/UseRangesCheck.cpp +++ b/clang-tools-extra/clang-tidy/utils/UseRangesCheck.cpp @@ -28,6 +28,7 @@ #include "llvm/ADT/Twine.h" #include "llvm/Support/raw_ostream.h" #include +#include #include #include @@ -173,21 +174,21 @@ static void removeFunctionArgs(DiagnosticBuilder &Diag, const CallExpr &Call, for (unsigned Index : Sorted) { const Expr *Arg = Call.getArg(Index); if (Commas[Index]) { - if (Index >= Commas.size()) { -Diag << FixItHint::CreateRemoval(Arg->getSourceRange()); - } else { + if (Index + 1 < Call.getNumArgs()) { // Remove the next comma Commas[Index + 1] = true; +const Expr *NextArg = Call.getArg(Index + 1); Diag << FixItHint::CreateRemoval(CharSourceRange::getTokenRange( -{Arg->getBeginLoc(), - Lexer::getLocForEndOfToken( - Arg->getEndLoc(), 0, Ctx.getSourceManager(), Ctx.getLangOpts()) - .getLocWithOffset(1)})); +{Arg->getBeginLoc(), NextArg->getBeginLoc().getLocWithOffset(-1)})); + } else { +Diag << FixItHint::CreateRemoval(Arg->getSourceRange()); } } else { - Diag << FixItHint::CreateRemoval(CharSourceRange::getTokenRange( - Arg->getBeginLoc().getLocWithOffset(-1), Arg->getEndLoc())); + // At this point we know Index > 0 because `Commas[0] = true` earlier Commas[Index] = true; + const Expr *PrevArg = Call.getArg(Index - 1); + Diag << FixItHint::CreateRemoval(CharSourceRange::getTokenRange( + PrevArg->getEndLoc().getLocWithOffset(1), Arg->getEndLoc())); } } } >From 644c8491e0fba203e89595827781d0c2c0609081 Mon Sep 17 00:00:00 2001 From: chomosuke Date: Mon, 23 Dec 2024 05:17:08 +1100 Subject: [PATCH 2/5] find , and remove only , --- .../clang-tidy/utils/UseRangesCheck.cpp | 49 +++ 1 file changed, 40 insertions(+), 9 deletions(-) diff --git a/clang-tools-extra/clang-tidy/utils/UseRangesCheck.cpp b/clang-tools-extra/clang-tidy/utils/UseRangesCheck.cpp index 88cba70b931d5..8b8e44a9898fd 100644 --- a/clang-tools-extra/clang-tidy/utils/UseRangesCheck.cpp +++ b/clang-tools-extra/clang-tidy/utils/UseRangesCheck.cpp @@ -28,7 +28,6 @@ #include "llvm/ADT/Twine.h" #include "llvm/Support/raw_ostream.h" #include -#include #include #include @@ -165,6 +164,33 @@ void UseRangesCheck::registerMatchers(MatchFinder *Finder) { static void removeFunctionArgs(DiagnosticBuilder &Diag, const CallExpr &Call, ArrayRef Indexes, const ASTContext &Ctx) { + auto GetCommaLoc = + [&](SourceLocation BeginLoc, + SourceLocation EndLoc) -> std::optional { +auto Invalid = false; +auto SourceText = Lexer::getSourceText( +CharSourceRange::getCharRange({BeginLoc, EndLoc}), +Ctx.getSourceManager(), Ctx.getLangOpts(), &Invalid); +assert(!Invalid); + +size_t I = 0; +while (I < SourceText.size() && SourceText[I] != ',') { + I++; +} + +if (I < SourceText.size()) { + // also remove space after , + size_t J = I + 1; + while (J < SourceText.size() && SourceText[J] == ' ') { +J++; + } + + return std::make_optional(CharSourceRange::getCharRange( + {BeginLoc.getLocWithOffset(I), BeginLoc.getLocWithOffset(J)})); +} +return std::nullopt; + }; + llvm::SmallVector Sorted(Indexes); llvm::sort(Sorted); // Keep track of commas removed @@ -176,20 +202,25 @@ static void removeFunctionArgs(DiagnosticBuilder &Diag, const CallExpr &Call, if (Commas[Index]) { if (Index + 1 < Call.getNumArgs()) { // Remove the next comma -Commas[Index + 1] = true; const Expr *NextArg = Call.getArg(Index + 1); -Diag << FixItHint::CreateRemoval(CharSourceRange::getTokenRange( -{Arg->getBeginLoc(), NextArg->getBeginLoc().getLocWithOffset(-1)})); - } else { -Diag << FixItHint::CreateRemoval(Arg->getSourceRange()); +auto CommaLoc = GetCommaLoc(Arg->getEndLoc().getLocWithOffset(1), +NextArg->getBeginLoc()); +if (CommaLoc) { + Commas[Index + 1] = true; + Diag << FixItHint::CreateRemoval(*CommaLoc); +} } } else { // At this point we know Index > 0 because `
[clang-tools-extra] [clang-tidy][clangd] Fixed removeFunctionArgs don't remove comma for use-ranges check (PR #118568)
chomosuke wrote: Ping https://github.com/llvm/llvm-project/pull/118568 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy][clangd] Fixed removeFunctionArgs don't remove comma for use-ranges check (PR #118568)
https://github.com/chomosuke updated https://github.com/llvm/llvm-project/pull/118568 >From b43a2602025bdacea06ced5171904fb5d765de9f Mon Sep 17 00:00:00 2001 From: chomosuke Date: Tue, 3 Dec 2024 07:10:33 + Subject: [PATCH 1/5] fixed removeFunctionArgs don't remove comma --- .../clang-tidy/utils/UseRangesCheck.cpp | 19 ++- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/clang-tools-extra/clang-tidy/utils/UseRangesCheck.cpp b/clang-tools-extra/clang-tidy/utils/UseRangesCheck.cpp index aba4d17ccd035..88cba70b931d5 100644 --- a/clang-tools-extra/clang-tidy/utils/UseRangesCheck.cpp +++ b/clang-tools-extra/clang-tidy/utils/UseRangesCheck.cpp @@ -28,6 +28,7 @@ #include "llvm/ADT/Twine.h" #include "llvm/Support/raw_ostream.h" #include +#include #include #include @@ -173,21 +174,21 @@ static void removeFunctionArgs(DiagnosticBuilder &Diag, const CallExpr &Call, for (unsigned Index : Sorted) { const Expr *Arg = Call.getArg(Index); if (Commas[Index]) { - if (Index >= Commas.size()) { -Diag << FixItHint::CreateRemoval(Arg->getSourceRange()); - } else { + if (Index + 1 < Call.getNumArgs()) { // Remove the next comma Commas[Index + 1] = true; +const Expr *NextArg = Call.getArg(Index + 1); Diag << FixItHint::CreateRemoval(CharSourceRange::getTokenRange( -{Arg->getBeginLoc(), - Lexer::getLocForEndOfToken( - Arg->getEndLoc(), 0, Ctx.getSourceManager(), Ctx.getLangOpts()) - .getLocWithOffset(1)})); +{Arg->getBeginLoc(), NextArg->getBeginLoc().getLocWithOffset(-1)})); + } else { +Diag << FixItHint::CreateRemoval(Arg->getSourceRange()); } } else { - Diag << FixItHint::CreateRemoval(CharSourceRange::getTokenRange( - Arg->getBeginLoc().getLocWithOffset(-1), Arg->getEndLoc())); + // At this point we know Index > 0 because `Commas[0] = true` earlier Commas[Index] = true; + const Expr *PrevArg = Call.getArg(Index - 1); + Diag << FixItHint::CreateRemoval(CharSourceRange::getTokenRange( + PrevArg->getEndLoc().getLocWithOffset(1), Arg->getEndLoc())); } } } >From 644c8491e0fba203e89595827781d0c2c0609081 Mon Sep 17 00:00:00 2001 From: chomosuke Date: Mon, 23 Dec 2024 05:17:08 +1100 Subject: [PATCH 2/5] find , and remove only , --- .../clang-tidy/utils/UseRangesCheck.cpp | 49 +++ 1 file changed, 40 insertions(+), 9 deletions(-) diff --git a/clang-tools-extra/clang-tidy/utils/UseRangesCheck.cpp b/clang-tools-extra/clang-tidy/utils/UseRangesCheck.cpp index 88cba70b931d5..8b8e44a9898fd 100644 --- a/clang-tools-extra/clang-tidy/utils/UseRangesCheck.cpp +++ b/clang-tools-extra/clang-tidy/utils/UseRangesCheck.cpp @@ -28,7 +28,6 @@ #include "llvm/ADT/Twine.h" #include "llvm/Support/raw_ostream.h" #include -#include #include #include @@ -165,6 +164,33 @@ void UseRangesCheck::registerMatchers(MatchFinder *Finder) { static void removeFunctionArgs(DiagnosticBuilder &Diag, const CallExpr &Call, ArrayRef Indexes, const ASTContext &Ctx) { + auto GetCommaLoc = + [&](SourceLocation BeginLoc, + SourceLocation EndLoc) -> std::optional { +auto Invalid = false; +auto SourceText = Lexer::getSourceText( +CharSourceRange::getCharRange({BeginLoc, EndLoc}), +Ctx.getSourceManager(), Ctx.getLangOpts(), &Invalid); +assert(!Invalid); + +size_t I = 0; +while (I < SourceText.size() && SourceText[I] != ',') { + I++; +} + +if (I < SourceText.size()) { + // also remove space after , + size_t J = I + 1; + while (J < SourceText.size() && SourceText[J] == ' ') { +J++; + } + + return std::make_optional(CharSourceRange::getCharRange( + {BeginLoc.getLocWithOffset(I), BeginLoc.getLocWithOffset(J)})); +} +return std::nullopt; + }; + llvm::SmallVector Sorted(Indexes); llvm::sort(Sorted); // Keep track of commas removed @@ -176,20 +202,25 @@ static void removeFunctionArgs(DiagnosticBuilder &Diag, const CallExpr &Call, if (Commas[Index]) { if (Index + 1 < Call.getNumArgs()) { // Remove the next comma -Commas[Index + 1] = true; const Expr *NextArg = Call.getArg(Index + 1); -Diag << FixItHint::CreateRemoval(CharSourceRange::getTokenRange( -{Arg->getBeginLoc(), NextArg->getBeginLoc().getLocWithOffset(-1)})); - } else { -Diag << FixItHint::CreateRemoval(Arg->getSourceRange()); +auto CommaLoc = GetCommaLoc(Arg->getEndLoc().getLocWithOffset(1), +NextArg->getBeginLoc()); +if (CommaLoc) { + Commas[Index + 1] = true; + Diag << FixItHint::CreateRemoval(*CommaLoc); +} } } else { // At this point we know Index > 0 because `
[clang-tools-extra] [clang-tidy][clangd] Fixed removeFunctionArgs don't remove comma for use-ranges check (PR #118568)
https://github.com/chomosuke updated https://github.com/llvm/llvm-project/pull/118568 >From b43a2602025bdacea06ced5171904fb5d765de9f Mon Sep 17 00:00:00 2001 From: chomosuke Date: Tue, 3 Dec 2024 07:10:33 + Subject: [PATCH 1/5] fixed removeFunctionArgs don't remove comma --- .../clang-tidy/utils/UseRangesCheck.cpp | 19 ++- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/clang-tools-extra/clang-tidy/utils/UseRangesCheck.cpp b/clang-tools-extra/clang-tidy/utils/UseRangesCheck.cpp index aba4d17ccd035..88cba70b931d5 100644 --- a/clang-tools-extra/clang-tidy/utils/UseRangesCheck.cpp +++ b/clang-tools-extra/clang-tidy/utils/UseRangesCheck.cpp @@ -28,6 +28,7 @@ #include "llvm/ADT/Twine.h" #include "llvm/Support/raw_ostream.h" #include +#include #include #include @@ -173,21 +174,21 @@ static void removeFunctionArgs(DiagnosticBuilder &Diag, const CallExpr &Call, for (unsigned Index : Sorted) { const Expr *Arg = Call.getArg(Index); if (Commas[Index]) { - if (Index >= Commas.size()) { -Diag << FixItHint::CreateRemoval(Arg->getSourceRange()); - } else { + if (Index + 1 < Call.getNumArgs()) { // Remove the next comma Commas[Index + 1] = true; +const Expr *NextArg = Call.getArg(Index + 1); Diag << FixItHint::CreateRemoval(CharSourceRange::getTokenRange( -{Arg->getBeginLoc(), - Lexer::getLocForEndOfToken( - Arg->getEndLoc(), 0, Ctx.getSourceManager(), Ctx.getLangOpts()) - .getLocWithOffset(1)})); +{Arg->getBeginLoc(), NextArg->getBeginLoc().getLocWithOffset(-1)})); + } else { +Diag << FixItHint::CreateRemoval(Arg->getSourceRange()); } } else { - Diag << FixItHint::CreateRemoval(CharSourceRange::getTokenRange( - Arg->getBeginLoc().getLocWithOffset(-1), Arg->getEndLoc())); + // At this point we know Index > 0 because `Commas[0] = true` earlier Commas[Index] = true; + const Expr *PrevArg = Call.getArg(Index - 1); + Diag << FixItHint::CreateRemoval(CharSourceRange::getTokenRange( + PrevArg->getEndLoc().getLocWithOffset(1), Arg->getEndLoc())); } } } >From 644c8491e0fba203e89595827781d0c2c0609081 Mon Sep 17 00:00:00 2001 From: chomosuke Date: Mon, 23 Dec 2024 05:17:08 +1100 Subject: [PATCH 2/5] find , and remove only , --- .../clang-tidy/utils/UseRangesCheck.cpp | 49 +++ 1 file changed, 40 insertions(+), 9 deletions(-) diff --git a/clang-tools-extra/clang-tidy/utils/UseRangesCheck.cpp b/clang-tools-extra/clang-tidy/utils/UseRangesCheck.cpp index 88cba70b931d5..8b8e44a9898fd 100644 --- a/clang-tools-extra/clang-tidy/utils/UseRangesCheck.cpp +++ b/clang-tools-extra/clang-tidy/utils/UseRangesCheck.cpp @@ -28,7 +28,6 @@ #include "llvm/ADT/Twine.h" #include "llvm/Support/raw_ostream.h" #include -#include #include #include @@ -165,6 +164,33 @@ void UseRangesCheck::registerMatchers(MatchFinder *Finder) { static void removeFunctionArgs(DiagnosticBuilder &Diag, const CallExpr &Call, ArrayRef Indexes, const ASTContext &Ctx) { + auto GetCommaLoc = + [&](SourceLocation BeginLoc, + SourceLocation EndLoc) -> std::optional { +auto Invalid = false; +auto SourceText = Lexer::getSourceText( +CharSourceRange::getCharRange({BeginLoc, EndLoc}), +Ctx.getSourceManager(), Ctx.getLangOpts(), &Invalid); +assert(!Invalid); + +size_t I = 0; +while (I < SourceText.size() && SourceText[I] != ',') { + I++; +} + +if (I < SourceText.size()) { + // also remove space after , + size_t J = I + 1; + while (J < SourceText.size() && SourceText[J] == ' ') { +J++; + } + + return std::make_optional(CharSourceRange::getCharRange( + {BeginLoc.getLocWithOffset(I), BeginLoc.getLocWithOffset(J)})); +} +return std::nullopt; + }; + llvm::SmallVector Sorted(Indexes); llvm::sort(Sorted); // Keep track of commas removed @@ -176,20 +202,25 @@ static void removeFunctionArgs(DiagnosticBuilder &Diag, const CallExpr &Call, if (Commas[Index]) { if (Index + 1 < Call.getNumArgs()) { // Remove the next comma -Commas[Index + 1] = true; const Expr *NextArg = Call.getArg(Index + 1); -Diag << FixItHint::CreateRemoval(CharSourceRange::getTokenRange( -{Arg->getBeginLoc(), NextArg->getBeginLoc().getLocWithOffset(-1)})); - } else { -Diag << FixItHint::CreateRemoval(Arg->getSourceRange()); +auto CommaLoc = GetCommaLoc(Arg->getEndLoc().getLocWithOffset(1), +NextArg->getBeginLoc()); +if (CommaLoc) { + Commas[Index + 1] = true; + Diag << FixItHint::CreateRemoval(*CommaLoc); +} } } else { // At this point we know Index > 0 because `
[clang-tools-extra] [clang-tidy][clangd] Fixed removeFunctionArgs don't remove comma for use-ranges check (PR #118568)
chomosuke wrote: Ping https://github.com/llvm/llvm-project/pull/118568 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits