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

[Alexander Kornienko via cfe-commits]

alexfh added inline comments.

================
Comment at: clang-tidy/readability/ElseAfterReturnCheck.cpp:45
@@ +44,3 @@
+  for (const auto &BindingName :
+       {"return", "continue", "break", "goto", "throw"}) {
+    if (Result.Nodes.getNodeAs<Stmt>(BindingName)) {
----------------
This won't work in MSVC2013, I think. Just add a `const char *Labels[] = {"return", ...` (add more consts, constexprs or statics, if you like ;)

================
Comment at: clang-tidy/readability/ElseAfterReturnCheck.cpp:51
@@ +50,3 @@
+
+  DiagnosticBuilder Diag = diag(ElseLoc, "do not use 'else' after '" +
+                                             ControlFlowInterruptor + '\'');
----------------
Please use diagnostic formatting facilities instead of string concatenation:

  diag(L, "xxx '%0' yyy") << ControlFlowInterruptor;

================
Comment at: docs/clang-tidy/checks/readability-else-after-return.rst:10
@@ -6,1 +9,3 @@
+or `else if` after something that interrupts control flow - like `return`,
+`break`, `continue`, `throw`, `goto`, etc.
 
----------------
I would omit `goto`, since it's unclear, where the target label is (and it's not a common construct anyway).

================
Comment at: test/clang-tidy/readability-else-after-return.cpp:33
@@ -32,1 +32,3 @@
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not use 'else' after 'return'
+  // CHECK-FIXES: // comment
     f(0);
----------------
CHECK-FIXES pattern should be sticter. Current pattern will match for any `// comment` occurrence anywhere in the test file, even if it's on a different line and still prefixed with `else`.

Two action items here: 1. make comments and patterns unique, 2. anchor patterns to the start of line (`{{ˆ}}`).


https://reviews.llvm.org/D23265