Skip to content

Commit 5992b32

Browse files
[clangd] Clean formatting modernize-use-override (llvm#81435)
When applying the recommended fix for the "modernize-use-override" clang-tidy diagnostic there was a stray whitespace. This PR fixes that. Resolves clangd/clangd#1704
1 parent 1301bc4 commit 5992b32

File tree

4 files changed

+58
-3
lines changed

4 files changed

+58
-3
lines changed

clang-tools-extra/clang-tidy/modernize/UseOverrideCheck.cpp

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
//===----------------------------------------------------------------------===//
88

99
#include "UseOverrideCheck.h"
10+
#include "../utils/LexerUtils.h"
1011
#include "clang/AST/ASTContext.h"
1112
#include "clang/ASTMatchers/ASTMatchFinder.h"
1213
#include "clang/Lex/Lexer.h"
@@ -228,9 +229,14 @@ void UseOverrideCheck::check(const MatchFinder::MatchResult &Result) {
228229
if (HasVirtual) {
229230
for (Token Tok : Tokens) {
230231
if (Tok.is(tok::kw_virtual)) {
231-
Diag << FixItHint::CreateRemoval(CharSourceRange::getTokenRange(
232-
Tok.getLocation(), Tok.getLocation()));
233-
break;
232+
std::optional<Token> NextToken =
233+
utils::lexer::findNextTokenIncludingComments(
234+
Tok.getEndLoc(), Sources, getLangOpts());
235+
if (NextToken.has_value()) {
236+
Diag << FixItHint::CreateRemoval(CharSourceRange::getCharRange(
237+
Tok.getLocation(), NextToken->getLocation()));
238+
break;
239+
}
234240
}
235241
}
236242
}

clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -898,6 +898,46 @@ TEST(DiagnosticTest, ClangTidySelfContainedDiags) {
898898
withFix(equalToFix(ExpectedDFix))))));
899899
}
900900

901+
TEST(DiagnosticTest, ClangTidySelfContainedDiagsFormatting) {
902+
Annotations Main(R"cpp(
903+
class Interface {
904+
public:
905+
virtual void Reset1() = 0;
906+
virtual void Reset2() = 0;
907+
};
908+
class A : public Interface {
909+
// This will be marked by clangd to use override instead of virtual
910+
$virtual1[[virtual ]]void $Reset1[[Reset1]]()$override1[[]];
911+
$virtual2[[virtual ]]/**/void $Reset2[[Reset2]]()$override2[[]];
912+
};
913+
)cpp");
914+
TestTU TU = TestTU::withCode(Main.code());
915+
TU.ClangTidyProvider =
916+
addTidyChecks("cppcoreguidelines-explicit-virtual-functions,");
917+
clangd::Fix const ExpectedFix1{
918+
"prefer using 'override' or (rarely) 'final' "
919+
"instead of 'virtual'",
920+
{TextEdit{Main.range("override1"), " override"},
921+
TextEdit{Main.range("virtual1"), ""}}};
922+
clangd::Fix const ExpectedFix2{
923+
"prefer using 'override' or (rarely) 'final' "
924+
"instead of 'virtual'",
925+
{TextEdit{Main.range("override2"), " override"},
926+
TextEdit{Main.range("virtual2"), ""}}};
927+
// Note that in the Fix we expect the "virtual" keyword and the following
928+
// whitespace to be deleted
929+
EXPECT_THAT(TU.build().getDiagnostics(),
930+
ifTidyChecks(UnorderedElementsAre(
931+
AllOf(Diag(Main.range("Reset1"),
932+
"prefer using 'override' or (rarely) 'final' "
933+
"instead of 'virtual'"),
934+
withFix(equalToFix(ExpectedFix1))),
935+
AllOf(Diag(Main.range("Reset2"),
936+
"prefer using 'override' or (rarely) 'final' "
937+
"instead of 'virtual'"),
938+
withFix(equalToFix(ExpectedFix2))))));
939+
}
940+
901941
TEST(DiagnosticsTest, Preprocessor) {
902942
// This looks like a preamble, but there's an #else in the middle!
903943
// Check that:

clang-tools-extra/docs/ReleaseNotes.rst

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -164,6 +164,10 @@ Changes in existing checks
164164
`AllowStringArrays` option, enabling the exclusion of array types with deduced
165165
length initialized from string literals.
166166

167+
- Improved :doc:`modernize-use-override
168+
<clang-tidy/checks/modernize/use-override>` check to also remove any trailing
169+
whitespace when deleting the ``virtual`` keyword.
170+
167171
- Improved :doc:`readability-redundant-inline-specifier
168172
<clang-tidy/checks/readability/redundant-inline-specifier>` check to properly
169173
emit warnings for static data member with an in-class initializer.

clang-tools-extra/test/clang-tidy/checkers/modernize/use-override.cpp

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ struct Base {
2727
virtual void f() = 0;
2828
virtual void f2() const = 0;
2929
virtual void g() = 0;
30+
virtual void g2() = 0;
3031

3132
virtual void j() const;
3233
virtual MustUseResultObject k();
@@ -126,6 +127,10 @@ struct SimpleCases : public Base {
126127
virtual void t() throw();
127128
// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: prefer using
128129
// CHECK-FIXES: {{^}} void t() throw() override;
130+
131+
virtual /* */ void g2();
132+
// CHECK-MESSAGES: :[[@LINE-1]]:33: warning: prefer using 'override' or (rarely) 'final' instead of 'virtual'
133+
// CHECK-FIXES: {{^}} /* */ void g2() override;
129134
};
130135

131136
// CHECK-MESSAGES-NOT: warning:

0 commit comments

Comments
 (0)