[clang-tools-extra] [clangd][clang-tidy] Make clangd run `format::cleanupAroundReplacements()` for all code actions just as clang-tidy does (PR #118569)
Richard Li via cfe-commits
cfe-commits at lists.llvm.org
Wed Dec 4 05:16:47 PST 2024
https://github.com/chomosuke updated https://github.com/llvm/llvm-project/pull/118569
>From d2257eb43bbd9ce2dfd8a13123c2048e1cf8b439 Mon Sep 17 00:00:00 2001
From: chomosuke <a13323600 at gmail.com>
Date: Wed, 4 Dec 2024 00:07:31 +0000
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<TextEdit, 1> Edits;
+ auto Replacements = std::make_optional<tooling::Replacements>();
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<TextEdit, 1> 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 <a13323600 at gmail.com>
Date: Wed, 4 Dec 2024 01:15:57 +0000
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 <math.h>\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 <math.h>\n\n"});
+ ExpectedDFix.Edits.push_back(TextEdit{Main.range("DFix"), " = NAN"});
EXPECT_THAT(
TU.build().getDiagnostics(),
ifTidyChecks(UnorderedElementsAre(
@@ -921,14 +921,14 @@ TEST(DiagnosticTest, ClangTidySelfContainedDiagsFormatting) {
clangd::Fix const ExpectedFix1{
"prefer using 'override' or (rarely) 'final' "
"instead of 'virtual'",
- {TextEdit{Main.range("override1"), " override"},
- TextEdit{Main.range("virtual1"), ""}},
+ {TextEdit{Main.range("virtual1"), ""},
+ TextEdit{Main.range("override1"), " override"}},
{}};
clangd::Fix const ExpectedFix2{
"prefer using 'override' or (rarely) 'final' "
"instead of 'virtual'",
- {TextEdit{Main.range("override2"), " override"},
- TextEdit{Main.range("virtual2"), ""}},
+ {TextEdit{Main.range("virtual2"), ""},
+ TextEdit{Main.range("override2"), " override"}},
{}};
// Note that in the Fix we expect the "virtual" keyword and the following
// whitespace to be deleted
@@ -1930,10 +1930,10 @@ TEST(ParsedASTTest, ModuleSawDiag) {
TestTU TU;
auto AST = TU.build();
- #if 0
+#if 0
EXPECT_THAT(AST.getDiagnostics(),
testing::Contains(Diag(Code.range(), KDiagMsg.str())));
- #endif
+#endif
}
TEST(Preamble, EndsOnNonEmptyLine) {
>From 0735f2821c36827c1b123d63d4ca9e716cc790dd Mon Sep 17 00:00:00 2001
From: chomosuke <a13323600 at gmail.com>
Date: Wed, 4 Dec 2024 13:16:31 +0000
Subject: [PATCH 3/3] Added test
---
.../clangd/unittests/DiagnosticsTests.cpp | 37 +++++++++++++++++++
1 file changed, 37 insertions(+)
diff --git a/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp b/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
index 94765b48099eda..6d9f8d5931a0e5 100644
--- a/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
+++ b/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
@@ -2136,6 +2136,43 @@ TEST(DiagnosticsTest, UnusedInHeader) {
EXPECT_THAT(TU.build().getDiagnostics(), IsEmpty());
}
+TEST(DiagnosticsTest, CleanupAroundReplacements) {
+ Annotations Test(R"cpp(
+ struct PositiveValueChar {
+ PositiveValueChar() : $c0fix[[c0()]]$comma[[,]] $c1fix[[c1()]] {}
+ const char $c0[[c0]];
+ wchar_t $c1[[c1]];
+ };
+ )cpp");
+ auto TU = TestTU::withCode(Test.code());
+ TU.ClangTidyProvider = addTidyChecks("modernize-use-default-member-init");
+
+ clangd::Fix C0Fix;
+ C0Fix.Message = "use default member initializer for 'c0'";
+ C0Fix.Edits.push_back(
+ TextEdit{{Test.range("c0fix").start, Test.range("comma").end}, ""});
+ C0Fix.Edits.push_back(
+ TextEdit{{Test.range("c0").end, Test.range("c0").end}, "{}"});
+
+ clangd::Fix C1Fix;
+ C1Fix.Message = "use default member initializer for 'c1'";
+ C1Fix.Edits.push_back(
+ TextEdit{Test.range("comma"), ""});
+ C1Fix.Edits.push_back(
+ TextEdit{Test.range("c1fix"), ""});
+ C1Fix.Edits.push_back(
+ TextEdit{{Test.range("c1").end, Test.range("c1").end}, "{}"});
+
+ EXPECT_THAT(TU.build().getDiagnostics(),
+ ifTidyChecks(UnorderedElementsAre(
+ AllOf(Diag(Test.range("c0"),
+ "use default member initializer for 'c0'"),
+ withFix(equalToFix(C0Fix))),
+ AllOf(Diag(Test.range("c1"),
+ "use default member initializer for 'c1'"),
+ withFix(equalToFix(C1Fix))))));
+}
+
} // namespace
} // namespace clangd
} // namespace clang
More information about the cfe-commits
mailing list