njames93 created this revision.
Herald added subscribers: cfe-commits, xazax.hun.
Herald added a project: clang.
njames93 requested review of this revision.
Simplifies the code in the fix-it generation.
Addresses https://bugs.llvm.org/show_bug.cgi?id=47809 (hopefully)
By not changing the whole else block it also means other checks with fix-its
can be applied in the else block.
Depends on D91103 <https://reviews.llvm.org/D91103>
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D91149
Files:
clang-tools-extra/clang-tidy/readability/ElseAfterReturnCheck.cpp
clang-tools-extra/test/clang-tidy/checkers/readability-else-after-return.cpp
Index: clang-tools-extra/test/clang-tidy/checkers/readability-else-after-return.cpp
===================================================================
--- clang-tools-extra/test/clang-tidy/checkers/readability-else-after-return.cpp
+++ 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;
+}
Index: clang-tools-extra/clang-tidy/readability/ElseAfterReturnCheck.cpp
===================================================================
--- clang-tools-extra/clang-tidy/readability/ElseAfterReturnCheck.cpp
+++ 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<CompoundStmt>(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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
