[clang-tidy] Fix readability-else-after-return breaking code by deleting too many characters (#187437)

Fixes #57258.
This commit is contained in:
Victor Chernyakin 2026-03-19 12:09:48 -05:00 committed by GitHub
parent ffa8ba8ce2
commit 555caa1876
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 27 additions and 24 deletions

View File

@ -129,33 +129,13 @@ static void removeElseAndBrackets(DiagnosticBuilder &Diag, ASTContext &Context,
auto Remap = [&](SourceLocation Loc) {
return Context.getSourceManager().getExpansionLoc(Loc);
};
auto TokLen = [&](SourceLocation Loc) {
return Lexer::MeasureTokenLength(Loc, Context.getSourceManager(),
Context.getLangOpts());
};
if (const auto *CS = dyn_cast<CompoundStmt>(Else)) {
Diag << tooling::fixit::createRemoval(ElseLoc);
const SourceLocation LBrace = CS->getLBracLoc();
const SourceLocation RBrace = CS->getRBracLoc();
const SourceLocation RangeStart =
Remap(LBrace).getLocWithOffset(TokLen(LBrace) + 1);
const SourceLocation RangeEnd = Remap(RBrace).getLocWithOffset(-1);
const StringRef Repl = Lexer::getSourceText(
CharSourceRange::getTokenRange(RangeStart, RangeEnd),
Context.getSourceManager(), Context.getLangOpts());
Diag << tooling::fixit::createReplacement(CS->getSourceRange(), Repl);
Diag << tooling::fixit::createRemoval(ElseLoc)
<< tooling::fixit::createRemoval(Remap(CS->getLBracLoc()))
<< tooling::fixit::createRemoval(Remap(CS->getRBracLoc()));
} else {
const SourceLocation ElseExpandedLoc = Remap(ElseLoc);
const SourceLocation EndLoc = Remap(Else->getEndLoc());
const StringRef Repl = Lexer::getSourceText(
CharSourceRange::getTokenRange(
ElseExpandedLoc.getLocWithOffset(TokLen(ElseLoc) + 1), EndLoc),
Context.getSourceManager(), Context.getLangOpts());
Diag << tooling::fixit::createReplacement(
SourceRange(ElseExpandedLoc, EndLoc), Repl);
Diag << tooling::fixit::createRemoval(Remap(ElseLoc));
}
}

View File

@ -357,6 +357,9 @@ Changes in existing checks
- Fixed a false positive involving ``if`` statements which contain
a ``return``, ``break``, etc., jumped over by a ``goto``.
- Fixed the check potentially breaking code by deleting one too many
characters following an ``else`` or a curly brace.
- Added support for handling attributed ``if`` then-branches such as
``[[likely]]`` and ``[[unlikely]]``.

View File

@ -502,3 +502,23 @@ void testNoReturn() {
f(0);
}
}
void testFixitsPreserveComments() {
if (true) {
return;
} else {// aaaa
// bbbb
}// cccc
// CHECK-MESSAGES: :[[@LINE-3]]:5: warning: do not use 'else' after 'return'
// CHECK-FIXES: } // aaaa
// CHECK-FIXES-NEXT: // bbbb
// CHECK-FIXES-NEXT: // cccc
if (true)
return;
else// dddd
;// eeee
// CHECK-MESSAGES: :[[@LINE-2]]:3: warning: do not use 'else' after 'return'
// CHECK-FIXES: // dddd
// CHECK-FIXES-NEXT: ;// eeee
}