[PATCH] D23265: [clang-tidy] enhance readability-else-after-return

[Kirill Bobyrev via cfe-commits]

omtcyfz created this revision.
omtcyfz added a reviewer: alexfh.
omtcyfz added a subscriber: cfe-commits.

`readability-else-after-return` only warns about `return` calls, but [[ 
http://llvm.org/docs/CodingStandards.html#don-t-use-else-after-a-return | 
Coding Stadnards ]] mention `throw`, `continue`, `goto`, etc.

https://reviews.llvm.org/D23265

Files:
  clang-tidy/readability/ElseAfterReturnCheck.cpp
  test/clang-tidy/readability-else-after-return.cpp


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' or 'else if' after something that interrupts control flow - like return, break, throw, continue, goto, etc
+  // 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' or 'else if' after something that interrupts control flow - like return, break, throw, continue, goto, etc
+  // CHECK-FIXES: // comment
     return;
   }
 
@@ -28,7 +28,43 @@
     f(0);
   else if (a > 10)
     return;
-  else
+  else // comment
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not use 'else' or 'else if' after something that interrupts control flow - like return, break, throw, continue, goto, etc
+  // 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' or 'else if' after something that interrupts control flow - like return, break, throw, continue, goto, etc
+    // CHECK-FIXES: // comment
+      x++;
+    }
+    if (x) {
+      break;
+    } else { // comment
+    // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: do not use 'else' or 'else if' after something that interrupts control flow - like return, break, throw, continue, goto, etc
+    // CHECK-FIXES: // comment
+      x++;
+    }
+    if (x) {
+      throw 42;
+    } else { // comment
+    // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: do not use 'else' or 'else if' after something that interrupts control flow - like return, break, throw, continue, goto, etc
+    // CHECK-FIXES: // comment
+      x++;
+    }
+    if (x) {
+      goto label;
+    } else { // comment
+    // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: do not use 'else' or 'else if' after something that interrupts control flow - like return, break, throw, continue, goto, etc
+    // CHECK-FIXES: // comment
+      x++;
+    }
+
+  }
+}
Index: clang-tidy/readability/ElseAfterReturnCheck.cpp
===================================================================
--- clang-tidy/readability/ElseAfterReturnCheck.cpp
+++ clang-tidy/readability/ElseAfterReturnCheck.cpp
@@ -19,20 +19,26 @@
 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()))))),
-                         hasElse(stmt().bind("else")))
-                      .bind("if"))),
+      stmt(forEach(
+          ifStmt(hasThen(stmt(
+                     anyOf(returnStmt(), compoundStmt(has(returnStmt())),
+                           continueStmt(), compoundStmt(has(continueStmt())),
+                           breakStmt(), compoundStmt(has(breakStmt())),
+                           gotoStmt(), compoundStmt(has(gotoStmt())),
+                           cxxThrowExpr(), compoundStmt(has(cxxThrowExpr()))))),
+                 hasElse(stmt().bind("else")))
+              .bind("if"))),
       this);
 }
 
 void ElseAfterReturnCheck::check(const MatchFinder::MatchResult &Result) {
   const auto *If = Result.Nodes.getNodeAs<IfStmt>("if");
   SourceLocation ElseLoc = If->getElseLoc();
-  DiagnosticBuilder Diag = diag(ElseLoc, "don't use else after return");
+  StringRef DiagMessage = "do not use 'else' or 'else if' after something that "
+                          "interrupts control flow - like return, break, "
+                          "throw, continue, goto, etc";
+  DiagnosticBuilder Diag = diag(ElseLoc, DiagMessage);
   Diag << tooling::fixit::createRemoval(ElseLoc);
 
   // FIXME: Removing the braces isn't always safe. Do a more careful analysis.

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits