Index: clang-tidy/readability/ElseAfterReturnCheck.cpp =================================================================== --- clang-tidy/readability/ElseAfterReturnCheck.cpp +++ clang-tidy/readability/ElseAfterReturnCheck.cpp @@ -19,11 +19,19 @@ namespace readability { void ElseAfterReturnCheck::registerMatchers(MatchFinder *Finder) { - // FIXME: Support continue, break and throw. Finder->addMatcher( - compoundStmt( - forEach(ifStmt(hasThen(stmt(anyOf(returnStmt(), - compoundStmt(has(returnStmt()))))), + stmt( + forEach(ifStmt(hasThen(stmt(anyOf( + returnStmt().bind("return"), + compoundStmt(has(returnStmt().bind("return"))), + continueStmt().bind("continue"), + compoundStmt(has(continueStmt().bind("continue"))), + breakStmt().bind("break"), + compoundStmt(has(breakStmt().bind("break"))), + gotoStmt().bind("goto"), + compoundStmt(has(gotoStmt().bind("goto"))), + cxxThrowExpr().bind("throw"), + compoundStmt(has(cxxThrowExpr().bind("throw")))))), hasElse(stmt().bind("else"))) .bind("if"))), this); @@ -32,7 +40,16 @@ void ElseAfterReturnCheck::check(const MatchFinder::MatchResult &Result) { const auto *If = Result.Nodes.getNodeAs("if"); SourceLocation ElseLoc = If->getElseLoc(); - DiagnosticBuilder Diag = diag(ElseLoc, "don't use else after return"); + std::string ControlFlowInterruptor; + for (const auto &BindingName : + {"return", "continue", "break", "goto", "throw"}) { + if (Result.Nodes.getNodeAs(BindingName)) { + ControlFlowInterruptor = BindingName; + } + } + + DiagnosticBuilder Diag = diag(ElseLoc, "do not use 'else' after '" + + ControlFlowInterruptor + '\''); Diag << tooling::fixit::createRemoval(ElseLoc); // FIXME: Removing the braces isn't always safe. Do a more careful analysis. Index: docs/clang-tidy/checks/readability-else-after-return.rst =================================================================== --- docs/clang-tidy/checks/readability-else-after-return.rst +++ docs/clang-tidy/checks/readability-else-after-return.rst @@ -3,7 +3,72 @@ readability-else-after-return ============================= +`LLVM Coding Standards `_ advises to +reduce indentation where possible and where it makes understanding code easier. +Early exit is one of the suggested enforcement of that. Please do not use `else` +or `else if` after something that interrupts control flow - like `return`, +`break`, `continue`, `throw`, `goto`, etc. -Flags the usages of ``else`` after ``return``. +Following piece of code illustrates how check would work. This piece of code: -http://llvm.org/docs/CodingStandards.html#don-t-use-else-after-a-return +``` c++ +void foo(int Value) { + int Local = 0; + for (int i = 0; i < 42; i++) { + if (Value == 1) { + return; + } else { + Local++; + } + + if (Value == 2) + continue; + else + Local++; + + if (Value == 3) { + throw 42; + } else { + Local++; + } + + if (Value == 4) { + goto label; + } else { + Local++; + } + } +} +``` + +Would be transformed into: + +``` c++ +void foo(int Value) { + int Local = 0; + for (int i = 0; i < 42; i++) { + if (Value == 1) { + return; + } + Local++; + + if (Value == 2) + continue; + Local++; + + if (Value == 3) { + throw 42; + } + Local++; + + if (Value == 4) { + goto label; + } + Local++; + } +} +``` + + +This checks helps to enforce this `Coding Standars recommendation +`_. Index: test/clang-tidy/readability-else-after-return.cpp =================================================================== --- test/clang-tidy/readability-else-after-return.cpp +++ test/clang-tidy/readability-else-after-return.cpp @@ -4,15 +4,15 @@ if (a > 0) return; else // comment -// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: don't use else after return -// CHECK-FIXES: {{^}} // comment + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not use 'else' after 'return' + // CHECK-FIXES: // comment return; if (a > 0) { return; } else { // comment -// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: don't use else after return -// CHECK-FIXES: } // comment + // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: do not use 'else' after 'return' + // CHECK-FIXES: // comment return; } @@ -28,7 +28,42 @@ f(0); else if (a > 10) return; - else + else // comment + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not use 'else' after 'return' + // CHECK-FIXES: // comment f(0); } +void foo() { +label: + for (unsigned x = 0; x < 42; ++x) { + if (x) { + continue; + } else { // comment + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: do not use 'else' after 'continue' + // CHECK-FIXES: // comment + x++; + } + if (x) { + break; + } else { // comment + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: do not use 'else' after 'break' + // CHECK-FIXES: // comment + x++; + } + if (x) { + throw 42; + } else { // comment + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: do not use 'else' after 'throw' + // CHECK-FIXES: // comment + x++; + } + if (x) { + goto label; + } else { // comment + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: do not use 'else' after 'goto' + // CHECK-FIXES: // comment + x++; + } + } +}