diff --git a/clang-tools-extra/clang-tidy/readability/ElseAfterReturnCheck.cpp b/clang-tools-extra/clang-tidy/readability/ElseAfterReturnCheck.cpp index 52554fe29151..f06059e0b196 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(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 a02eda85a7e5..c5d0c617a9fd 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 1bbbdbc2a568..85117b718de8 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 +}