[PATCH] D19586: Misleading Indentation check

[Alexander Kornienko via cfe-commits]

alexfh added a comment.

Gergely, it seems that the last diff is missing clang-tidy/readability/MisleadingIndentationCheck.cpp. A few more comments below.


================
Comment at: docs/clang-tidy/checks/readability-misleading-indentation.rst:13
@@ +12,3 @@
+
+The way to avoid dangling else is to always check that an "else" belongs
+to the "if" that begins in the same column.
----------------
nit: Please use double backquotes to mark inline code fragments.

================
Comment at: test/clang-tidy/readability-misleading-indentation.cpp:16
@@ +15,3 @@
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: wrong indentation, else belongs to
+  // if(cond2) statement
+  // CHECK-FIXES: {{^}}  // comment
----------------
1. This is not a part of the previous CHECK-MESSAGES. Is it intended? It's fine to have long lines in tests.
2. Code snippets in the message should be enclosed in single quotes ('else', 'if(cond2)' ...).
3. Please specify each unique message once completely (including the [check-name] part).

================
Comment at: test/clang-tidy/readability-misleading-indentation.cpp:17
@@ +16,3 @@
+  // if(cond2) statement
+  // CHECK-FIXES: {{^}}  // comment
+
----------------
These `CHECK-FIXES:` are totally unreliable, since they are all matching the same pattern. FileCheck (which is used to verify the output against these CHECK lines) maintains no implicit relation between the check line location and the line number matched in the output. The two things that matter to FileCheck are the order of the matched lines and their content. So the patterns need to be unique to avoid matches to incorrect lines. Change them to, e.g. `// comment1`, `// comment2`, ...

================
Comment at: test/clang-tidy/readability-misleading-indentation.cpp:63
@@ +62,3 @@
+    foo2();
+  }
+
----------------
What about this comment?

================
Comment at: test/clang-tidy/readability-misleading-indentation.cpp:63
@@ +62,3 @@
+    foo2();
+  }
+
----------------
alexfh wrote:
> What about this comment?
What about this comment?


http://reviews.llvm.org/D19586