diff --git a/clang-tools-extra/clang-tidy/readability/ElseAfterReturnCheck.cpp b/clang-tools-extra/clang-tidy/readability/ElseAfterReturnCheck.cpp --- a/clang-tools-extra/clang-tidy/readability/ElseAfterReturnCheck.cpp +++ b/clang-tools-extra/clang-tidy/readability/ElseAfterReturnCheck.cpp @@ -93,38 +93,18 @@ return false; } -static void removeElseAndBrackets(DiagnosticBuilder &Diag, ASTContext &Context, +void removeElseAndBrackets(DiagnosticBuilder &Diag, const SourceManager &SM, const Stmt *Else, SourceLocation ElseLoc) { - auto Remap = [&](SourceLocation Loc) { - return Context.getSourceManager().getExpansionLoc(Loc); - }; - auto TokLen = [&](SourceLocation Loc) { - return Lexer::MeasureTokenLength(Loc, Context.getSourceManager(), - Context.getLangOpts()); - }; + auto Remap = [&](SourceLocation Loc) { return SM.getExpansionLoc(Loc); }; if (const auto *CS = dyn_cast(Else)) { - Diag << tooling::fixit::createRemoval(ElseLoc); - SourceLocation LBrace = CS->getLBracLoc(); - SourceLocation RBrace = CS->getRBracLoc(); - SourceLocation RangeStart = - Remap(LBrace).getLocWithOffset(TokLen(LBrace) + 1); - SourceLocation RangeEnd = Remap(RBrace).getLocWithOffset(-1); - - llvm::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())) + << FixItHint::CreateReformatFixIt(CS->getSourceRange()); } else { - SourceLocation ElseExpandedLoc = Remap(ElseLoc); - SourceLocation EndLoc = Remap(Else->getEndLoc()); - - llvm::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)) + << FixItHint::CreateReformatFixIt(Else->getSourceRange()); } } @@ -208,7 +188,8 @@ Repl) << tooling::fixit::createReplacement(VDeclStmt->getSourceRange(), VDecl->getName()); - removeElseAndBrackets(Diag, *Result.Context, Else, ElseLoc); + removeElseAndBrackets(Diag, Result.Context->getSourceManager(), Else, + ElseLoc); } else if (WarnOnUnfixable) { // Warn, but don't attempt an autofix. diag(ElseLoc, WarningMessage) << ControlFlowInterruptor; @@ -231,7 +212,8 @@ tooling::fixit::getText(If->getIfLoc(), *Result.Context)) .str()) << tooling::fixit::createRemoval(If->getInit()->getSourceRange()); - removeElseAndBrackets(Diag, *Result.Context, Else, ElseLoc); + removeElseAndBrackets(Diag, Result.Context->getSourceManager(), Else, + ElseLoc); } else if (WarnOnUnfixable) { // Warn, but don't attempt an autofix. diag(ElseLoc, WarningMessage) << ControlFlowInterruptor; @@ -241,7 +223,8 @@ DiagnosticBuilder Diag = diag(ElseLoc, WarningMessage) << ControlFlowInterruptor; - removeElseAndBrackets(Diag, *Result.Context, Else, ElseLoc); + removeElseAndBrackets(Diag, Result.Context->getSourceManager(), Else, + ElseLoc); } } // namespace readability 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 --- 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 @@ -1,4 +1,4 @@ -// RUN: %check_clang_tidy %s readability-else-after-return %t -- -- -fexceptions -std=c++17 +// RUN: %check_clang_tidy %s readability-else-after-return -format-style=LLVM %t -- -- -fexceptions -std=c++17 namespace std { struct string { @@ -112,7 +112,7 @@ int declInConditionUsedInElse() { if (int X = g()) { // comment-11 // CHECK-FIXES: {{^}} int X = g(); - // CHECK-FIXES-NEXT: {{^}}if (X) { // comment-11 + // CHECK-FIXES-NEXT: {{^}} if (X) { // comment-11 return X; } else { // comment-11 // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: do not use 'else' after 'return' @@ -133,7 +133,7 @@ int varInitAndCondition() { if (int X = g(); X != 0) { // comment-13 // CHECK-FIXES: {{^}} int X = g(); - // CHECK-FIXES-NEXT: {{^}}if ( X != 0) { // comment-13 + // CHECK-FIXES-NEXT: {{^}} if (X != 0) { // comment-13 return X; } else { // comment-13 // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: do not use 'else' after 'return' @@ -179,8 +179,8 @@ int varInitAndCondVarUsedInElse() { if (int X = g(); int Y = g()) { // comment-17 // CHECK-FIXES: {{^}} int X = g(); - // CHECK-FIXES-NEXT: {{^}}int Y = g(); - // CHECK-FIXES-NEXT: {{^}}if ( Y) { // comment-17 + // CHECK-FIXES-NEXT: {{^}} int Y = g(); + // CHECK-FIXES-NEXT: {{^}} if (Y) { // comment-17 return X ? X : Y; } else { // comment-17 // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: do not use 'else' after 'return' @@ -205,7 +205,7 @@ } if (int b = a; b > 1) { // comment-18 // CHECK-FIXES: {{^}} int b = a; - // CHECK-FIXES-NEXT: {{^}}if ( b > 1) { // comment-18 + // CHECK-FIXES-NEXT: {{^}} if (b > 1) { // comment-18 return a; } else { // comment-18 // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: do not use 'else' after 'return' @@ -226,3 +226,40 @@ } return; } + +int complicatedFormat() { + // CHECK-MESSAGES: :[[@LINE+6]]:5: warning: do not use 'else' after 'return' [readability-else-after-return] + // CHECK-MESSAGES: :[[@LINE+8]]:7: warning: do not use 'else' after 'return' [readability-else-after-return] + // CHECK-MESSAGES: :[[@LINE+10]]:9: warning: do not use 'else' after 'return' [readability-else-after-return] + // CHECK-MESSAGES: :[[@LINE+12]]:11: warning: do not use 'else' after 'return' [readability-else-after-return] + if (true) { + return 1; + } else { + if (true) { + return 2; + } else { + if (true) { + return 3; + } else { + if (true) { + return 4; + } else { + return 5; + } + } + } + } + // CHECK-FIXES: {{^ }}if (true) { + // CHECK-FIXES-NEXT: {{^ }} return 1; + // CHECK-FIXES-NEXT: {{^ }}} + // CHECK-FIXES-NEXT: {{^ }}if (true) { + // CHECK-FIXES-NEXT: {{^ }} return 2; + // CHECK-FIXES-NEXT: {{^ }}} + // CHECK-FIXES-NEXT: {{^ }}if (true) { + // CHECK-FIXES-NEXT: {{^ }} return 3; + // CHECK-FIXES-NEXT: {{^ }}} + // CHECK-FIXES-NEXT: {{^ }}if (true) { + // CHECK-FIXES-NEXT: {{^ }} return 4; + // CHECK-FIXES-NEXT: {{^ }}} + // CHECK-FIXES-NEXT: {{^ }}return 5; +}