https://github.com/chomosuke updated https://github.com/llvm/llvm-project/pull/118569
>From d2257eb43bbd9ce2dfd8a13123c2048e1cf8b439 Mon Sep 17 00:00:00 2001 From: chomosuke <a13323...@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 <a13323...@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 <a13323...@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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits