summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorKevin Joseph <kevinjoseph1995@gmail.com>2024-02-14 11:59:21 -0800
committerGitHub <noreply@github.com>2024-02-14 20:59:21 +0100
commit5992b3272b29e071f6f5a4807a4e0c23e88c310d (patch)
treeb193478980cc735938cfafdae91bf0a172744411
parent1301bc46aea14297478bd13bcacff429e2a18c04 (diff)
[clangd] Clean formatting modernize-use-override (#81435)
When applying the recommended fix for the "modernize-use-override" clang-tidy diagnostic there was a stray whitespace. This PR fixes that. Resolves https://github.com/clangd/clangd/issues/1704
-rw-r--r--clang-tools-extra/clang-tidy/modernize/UseOverrideCheck.cpp12
-rw-r--r--clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp40
-rw-r--r--clang-tools-extra/docs/ReleaseNotes.rst4
-rw-r--r--clang-tools-extra/test/clang-tidy/checkers/modernize/use-override.cpp5
4 files changed, 58 insertions, 3 deletions
diff --git a/clang-tools-extra/clang-tidy/modernize/UseOverrideCheck.cpp b/clang-tools-extra/clang-tidy/modernize/UseOverrideCheck.cpp
index e348968b325a..fd5bd9f0b181 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::getTokenRange(
- Tok.getLocation(), Tok.getLocation()));
- 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 f302dcf5f09d..4839879e1b78 100644
--- a/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
+++ b/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
@@ -898,6 +898,46 @@ TEST(DiagnosticTest, ClangTidySelfContainedDiags) {
withFix(equalToFix(ExpectedDFix))))));
}
+TEST(DiagnosticTest, ClangTidySelfContainedDiagsFormatting) {
+ Annotations Main(R"cpp(
+ class Interface {
+ public:
+ virtual void Reset1() = 0;
+ virtual void Reset2() = 0;
+ };
+ class A : public Interface {
+ // This will be marked by clangd to use override instead of virtual
+ $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 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("Reset1"),
+ "prefer using 'override' or (rarely) 'final' "
+ "instead of 'virtual'"),
+ withFix(equalToFix(ExpectedFix1))),
+ AllOf(Diag(Main.range("Reset2"),
+ "prefer using 'override' or (rarely) 'final' "
+ "instead of 'virtual'"),
+ withFix(equalToFix(ExpectedFix2))))));
+}
+
TEST(DiagnosticsTest, Preprocessor) {
// This looks like a preamble, but there's an #else in the middle!
// Check that:
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 2f874d17da43..a1b95d2a2020 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -164,6 +164,10 @@ Changes in existing checks
`AllowStringArrays` option, enabling the exclusion of array types with deduced
length initialized from string literals.
+- Improved :doc:`modernize-use-override
+ <clang-tidy/checks/modernize/use-override>` check to also remove any trailing
+ whitespace when deleting the ``virtual`` keyword.
+
- Improved :doc:`readability-redundant-inline-specifier
<clang-tidy/checks/readability/redundant-inline-specifier>` check to properly
emit warnings for static data member with an in-class initializer.
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 55f226be7086..89d1aa48c46a 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 @@ public:
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: