Author: Victor Chernyakin Date: 2026-03-19T12:09:48-05:00 New Revision: 555caa18762fe068d00b61c62e13e6d04b2bfa9f
URL: https://github.com/llvm/llvm-project/commit/555caa18762fe068d00b61c62e13e6d04b2bfa9f DIFF: https://github.com/llvm/llvm-project/commit/555caa18762fe068d00b61c62e13e6d04b2bfa9f.diff LOG: [clang-tidy] Fix `readability-else-after-return` breaking code by deleting too many characters (#187437) Fixes #57258. Added: Modified: clang-tools-extra/clang-tidy/readability/ElseAfterReturnCheck.cpp clang-tools-extra/docs/ReleaseNotes.rst clang-tools-extra/test/clang-tidy/checkers/readability/else-after-return.cpp Removed: ################################################################################ diff --git a/clang-tools-extra/clang-tidy/readability/ElseAfterReturnCheck.cpp b/clang-tools-extra/clang-tidy/readability/ElseAfterReturnCheck.cpp index 52554fe29151f..f06059e0b196e 100644 --- a/clang-tools-extra/clang-tidy/readability/ElseAfterReturnCheck.cpp +++ b/clang-tools-extra/clang-tidy/readability/ElseAfterReturnCheck.cpp @@ -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)); } } diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index a02eda85a7e53..c5d0c617a9fd7 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -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]]``. diff --git a/clang-tools-extra/test/clang-tidy/checkers/readability/else-after-return.cpp b/clang-tools-extra/test/clang-tidy/checkers/readability/else-after-return.cpp index 1bbbdbc2a5683..85117b718de83 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/readability/else-after-return.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/readability/else-after-return.cpp @@ -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 +} _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
