[PATCH] Add an argument comment checker to clang-tidy.

[Alexander Kornienko]

  That's nice! I guess, this check will be really useful, especially when refactoring interfaces. Did you try running the check on the whole LLVM project, for example?

  As for the right place for this check, neither LLVM nor Google C++ coding style say anything about /*Param=*/ constructs, so it doesn't seem right to put it into one of those. I'd create a separate directory ("misc"?) and a separate checks module for this check.


================
Comment at: clang-tidy/llvm/LLVMTidyModule.cpp:140
@@ +139,3 @@
+  const Expr *E = Result.Nodes.getNodeAs<Expr>("expr");
+  if (auto CE = dyn_cast<CallExpr>(E)) {
+    auto FD = CE->getDirectCallee();
----------------
Have we moved to the Almost Always Auto style? ;)

I'd say that the usages of auto in this method don't add much value (unlike the one on lines 121 and 123). Could you use concrete types here instead?

I would also expand CE, FD and CCE to a bit more meaningful names.

================
Comment at: clang-tidy/llvm/LLVMTidyModule.cpp:131
@@ +130,3 @@
+          diag(PVD->getLocation(), "%0 declared here", DiagnosticIDs::Note)
+              << II;
+        }
----------------
I think, adding a FixIt to correct the comment would be useful at least in some of the cases (say, a typo in the comment). As a safety measure you could try to find the argument with the closest name to the one used in the comment, and only suggest the fix if this is the same argument. What do you think?

================
Comment at: test/clang-tidy/llvm-arg-comments.cpp:7
@@ +6,3 @@
+void g() {
+  // CHECK: 11:5: warning: argument name 'y' in comment does not match parameter name 'x'
+  // CHECK: 4:12: note: 'x' declared here
----------------
You can use [[@LINE+4]] instead of "11" here to make the test easier to change.

================
Comment at: clang-tidy/llvm/LLVMTidyModule.cpp:80
@@ +79,3 @@
+
+  while (1) {
+    Token Tok;
----------------
Maybe while (true)? Or even:

  Token Tok;
  while (!TheLexer.LexFromRawLexer(Tok) && Tok.getLocation() != Range.getEnd() && Tok.getKind() != tok::eof()) {
    ...

?

================
Comment at: clang-tidy/llvm/LLVMTidyModule.h:48
@@ +47,3 @@
+public:
+  void registerMatchers(ast_matchers::MatchFinder *Finder) LLVM_OVERRIDE;
+  void
----------------
There's no LLVM_OVERRIDE already. Use "override". Things are going fast these days. ;)

================
Comment at: clang-tidy/llvm/LLVMTidyModule.h:39
@@ +38,3 @@
+class ArgumentCommentCheck : public ClangTidyCheck {
+  llvm::Regex IdentRE{ "^/\\* *([_A-Za-z][_A-Za-z0-9]*) *= *\\*/$" };
+
----------------
I'd move the private section to the end of the class.


http://llvm-reviews.chandlerc.com/D2914