[PATCH] D12462: [PATCH] [clang-tidy] Add inconsistent declaration parameter name check

[Alexander Kornienko via cfe-commits]

alexfh added a comment.

Sorry for the long delay.

This version looks significantly better. Thank you for the updates!


================
Comment at: clang-tidy/readability/InconsistentDeclarationParameterNameCheck.cpp:1
@@ +1,2 @@
+//===--- InconsistentDeclarationParameterNameCheck.cpp -
+//clang-tidy---------------------------===//
----------------
Seems like clang-format (or something else) broke this comment.

================
Comment at: clang-tidy/readability/InconsistentDeclarationParameterNameCheck.cpp:17
@@ +16,3 @@
+
+using namespace clang::ast_matchers;
+
----------------
nit: If you move this below `namespace clang {`, you can omit the `clang::` part.

================
Comment at: clang-tidy/readability/InconsistentDeclarationParameterNameCheck.cpp:38
@@ +37,3 @@
+  DifferingParamInfo(int Number,
+                     llvm::StringRef MainName,
+                     llvm::StringRef OtherName,
----------------
I think, `llvm::` is not needed here, as `StringRef` should also be declared in namespace clang. 

================
Comment at: clang-tidy/readability/InconsistentDeclarationParameterNameCheck.cpp:86
@@ +85,3 @@
+
+      // FIXME: provide a way to extract commented out parameter name from comment
+      if (!MainParamName.empty() &&
----------------
Please use proper Capitalization and punctuation. http://llvm.org/docs/CodingStandards.html#commenting

Same in other comments.

================
Comment at: clang-tidy/readability/InconsistentDeclarationParameterNameCheck.cpp:180
@@ +179,3 @@
+    for (const DifferingParamInfo &ParamInfo : InconsistentDeclaration.DifferingParams) {
+      auto ParamDiag = diag(ParamInfo.OtherNameRange.getBegin(),
+           "parameter %0 is named '%1' here, but '%2' in compared declaration",
----------------
That can quickly become extremely chatty. Maybe cramp two lists of names in a single message? Something along the lines of: "parameter names here: (a, b, c, d), in the other declaration: (q, u, x)"

================
Comment at: clang-tidy/readability/InconsistentDeclarationParameterNameCheck.h:24
@@ +23,3 @@
+///
+/// Detailed documentation is provided in HTML files, see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/readability-inconsistent-declaration-parameter-name.html
----------------
nit: I'd say "For detailed documentation see:"


http://reviews.llvm.org/D12462