[clang-tools-extra] [clangd] Clean formatting modernize-use-override (PR #81435)
Kevin Joseph via cfe-commits
cfe-commits at lists.llvm.org
Sun Feb 11 21:58:10 PST 2024
https://github.com/kevinjoseph1995 updated https://github.com/llvm/llvm-project/pull/81435
>From db2c4ee74ffb0592ec7f3fd5557dbb5399ef998d Mon Sep 17 00:00:00 2001
From: Kevin Joseph <kevinjoseph1995 at gmail.com>
Date: Sun, 11 Feb 2024 13:39:51 -0800
Subject: [PATCH 1/2] [clangd] Clean formatting modernize-use-override
When applying the recommended fix for the
"modernize-use-override" clang-tidy diagnostic
there was a stray whitespace.
This commit fixes: https://github.com/clangd/clangd/issues/1704
---
.../clang-tidy/modernize/UseOverrideCheck.cpp | 4 +--
.../clangd/unittests/DiagnosticsTests.cpp | 28 +++++++++++++++++++
2 files changed, 30 insertions(+), 2 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/modernize/UseOverrideCheck.cpp b/clang-tools-extra/clang-tidy/modernize/UseOverrideCheck.cpp
index e348968b325a5a..4db32b02ee5a0c 100644
--- a/clang-tools-extra/clang-tidy/modernize/UseOverrideCheck.cpp
+++ b/clang-tools-extra/clang-tidy/modernize/UseOverrideCheck.cpp
@@ -228,8 +228,8 @@ void UseOverrideCheck::check(const MatchFinder::MatchResult &Result) {
if (HasVirtual) {
for (Token Tok : Tokens) {
if (Tok.is(tok::kw_virtual)) {
- Diag << FixItHint::CreateRemoval(CharSourceRange::getTokenRange(
- Tok.getLocation(), Tok.getLocation()));
+ Diag << FixItHint::CreateRemoval(CharSourceRange::getCharRange(
+ Tok.getLocation(), Tok.getEndLoc().getLocWithOffset(1)));
break;
}
}
diff --git a/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp b/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
index f302dcf5f09db0..76874ac9a2a4e7 100644
--- a/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
+++ b/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
@@ -898,6 +898,34 @@ TEST(DiagnosticTest, ClangTidySelfContainedDiags) {
withFix(equalToFix(ExpectedDFix))))));
}
+TEST(DiagnosticTest, ClangTidySelfContainedDiagsFormatting) {
+ Annotations Main(R"cpp(
+ class Interface {
+ public:
+ virtual void Reset() = 0;
+ };
+ class A : public Interface {
+ // This will be marked by clangd to use override instead of virtual
+ $virtual[[virtual ]] void $Reset[[Reset]]()$override[[]];
+ };
+ )cpp");
+ TestTU TU = TestTU::withCode(Main.code());
+ TU.ClangTidyProvider =
+ addTidyChecks("cppcoreguidelines-explicit-virtual-functions,");
+ clangd::Fix const ExpectedFix{"prefer using 'override' or (rarely) 'final' "
+ "instead of 'virtual'",
+ {TextEdit{Main.range("override"), " override"},
+ TextEdit{Main.range("virtual"), ""}}};
+ // Note that in the Fix we expect the "virtual" keyword and the following
+ // whitespace to be deleted
+ EXPECT_THAT(TU.build().getDiagnostics(),
+ ifTidyChecks(UnorderedElementsAre(
+ AllOf(Diag(Main.range("Reset"),
+ "prefer using 'override' or (rarely) 'final' "
+ "instead of 'virtual'"),
+ withFix(equalToFix(ExpectedFix))))));
+}
+
TEST(DiagnosticsTest, Preprocessor) {
// This looks like a preamble, but there's an #else in the middle!
// Check that:
>From 7b868a4ede5f49f2c4471b85ac389b36aaf1c6e1 Mon Sep 17 00:00:00 2001
From: Kevin Joseph <kevinjoseph1995 at gmail.com>
Date: Sun, 11 Feb 2024 19:35:29 -0800
Subject: [PATCH 2/2] fixup! [clangd] Clean formatting modernize-use-override
---
.../clang-tidy/modernize/UseOverrideCheck.cpp | 12 ++++++--
.../clangd/unittests/DiagnosticsTests.cpp | 28 +++++++++++++------
clang-tools-extra/docs/ReleaseNotes.rst | 4 +++
.../checkers/modernize/use-override.cpp | 5 ++++
4 files changed, 38 insertions(+), 11 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/modernize/UseOverrideCheck.cpp b/clang-tools-extra/clang-tidy/modernize/UseOverrideCheck.cpp
index 4db32b02ee5a0c..fd5bd9f0b181b1 100644
--- a/clang-tools-extra/clang-tidy/modernize/UseOverrideCheck.cpp
+++ b/clang-tools-extra/clang-tidy/modernize/UseOverrideCheck.cpp
@@ -7,6 +7,7 @@
//===----------------------------------------------------------------------===//
#include "UseOverrideCheck.h"
+#include "../utils/LexerUtils.h"
#include "clang/AST/ASTContext.h"
#include "clang/ASTMatchers/ASTMatchFinder.h"
#include "clang/Lex/Lexer.h"
@@ -228,9 +229,14 @@ void UseOverrideCheck::check(const MatchFinder::MatchResult &Result) {
if (HasVirtual) {
for (Token Tok : Tokens) {
if (Tok.is(tok::kw_virtual)) {
- Diag << FixItHint::CreateRemoval(CharSourceRange::getCharRange(
- Tok.getLocation(), Tok.getEndLoc().getLocWithOffset(1)));
- break;
+ std::optional<Token> NextToken =
+ utils::lexer::findNextTokenIncludingComments(
+ Tok.getEndLoc(), Sources, getLangOpts());
+ if (NextToken.has_value()) {
+ Diag << FixItHint::CreateRemoval(CharSourceRange::getCharRange(
+ Tok.getLocation(), NextToken->getLocation()));
+ break;
+ }
}
}
}
diff --git a/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp b/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
index 76874ac9a2a4e7..4839879e1b78c8 100644
--- a/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
+++ b/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
@@ -902,28 +902,40 @@ TEST(DiagnosticTest, ClangTidySelfContainedDiagsFormatting) {
Annotations Main(R"cpp(
class Interface {
public:
- virtual void Reset() = 0;
+ virtual void Reset1() = 0;
+ virtual void Reset2() = 0;
};
class A : public Interface {
// This will be marked by clangd to use override instead of virtual
- $virtual[[virtual ]] void $Reset[[Reset]]()$override[[]];
+ $virtual1[[virtual ]]void $Reset1[[Reset1]]()$override1[[]];
+ $virtual2[[virtual ]]/**/void $Reset2[[Reset2]]()$override2[[]];
};
)cpp");
TestTU TU = TestTU::withCode(Main.code());
TU.ClangTidyProvider =
addTidyChecks("cppcoreguidelines-explicit-virtual-functions,");
- clangd::Fix const ExpectedFix{"prefer using 'override' or (rarely) 'final' "
- "instead of 'virtual'",
- {TextEdit{Main.range("override"), " override"},
- TextEdit{Main.range("virtual"), ""}}};
+ clangd::Fix const ExpectedFix1{
+ "prefer using 'override' or (rarely) 'final' "
+ "instead of 'virtual'",
+ {TextEdit{Main.range("override1"), " override"},
+ TextEdit{Main.range("virtual1"), ""}}};
+ clangd::Fix const ExpectedFix2{
+ "prefer using 'override' or (rarely) 'final' "
+ "instead of 'virtual'",
+ {TextEdit{Main.range("override2"), " override"},
+ TextEdit{Main.range("virtual2"), ""}}};
// Note that in the Fix we expect the "virtual" keyword and the following
// whitespace to be deleted
EXPECT_THAT(TU.build().getDiagnostics(),
ifTidyChecks(UnorderedElementsAre(
- AllOf(Diag(Main.range("Reset"),
+ AllOf(Diag(Main.range("Reset1"),
"prefer using 'override' or (rarely) 'final' "
"instead of 'virtual'"),
- withFix(equalToFix(ExpectedFix))))));
+ withFix(equalToFix(ExpectedFix1))),
+ AllOf(Diag(Main.range("Reset2"),
+ "prefer using 'override' or (rarely) 'final' "
+ "instead of 'virtual'"),
+ withFix(equalToFix(ExpectedFix2))))));
}
TEST(DiagnosticsTest, Preprocessor) {
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index ee68c8f49b3df2..bd727fea7fd1e9 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -160,6 +160,10 @@ Changes in existing checks
`AllowStringArrays` option, enabling the exclusion of array types with deduced
length initialized from string literals.
+- Improved :doc:`
+ <clang-tidy/checks/modernize/use-override>` check to also remove any trailing
+ whitespace when deleting the ``virtual`` keyword.
+
Removed checks
^^^^^^^^^^^^^^
diff --git a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-override.cpp b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-override.cpp
index 55f226be70869e..89d1aa48c46a3c 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-override.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-override.cpp
@@ -27,6 +27,7 @@ struct Base {
virtual void f() = 0;
virtual void f2() const = 0;
virtual void g() = 0;
+ virtual void g2() = 0;
virtual void j() const;
virtual MustUseResultObject k();
@@ -126,6 +127,10 @@ struct SimpleCases : public Base {
virtual void t() throw();
// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: prefer using
// CHECK-FIXES: {{^}} void t() throw() override;
+
+ virtual /* */ void g2();
+ // CHECK-MESSAGES: :[[@LINE-1]]:33: warning: prefer using 'override' or (rarely) 'final' instead of 'virtual'
+ // CHECK-FIXES: {{^}} /* */ void g2() override;
};
// CHECK-MESSAGES-NOT: warning:
More information about the cfe-commits
mailing list