[clang-tidy] misc-argument-comment non-strict mode

Summary:
The misc-argument-comment check now ignores leading and trailing underscores and
case. The new `StrictMode` local/global option can be used to switch back to
strict checking.

Add getLocalOrGlobal version for integral types, minor cleanups.

Reviewers: hokein, aaron.ballman

Subscribers: aaron.ballman, Prazek, cfe-commits

Differential Revision: https://reviews.llvm.org/D23135

llvm-svn: 277729

1 files changed, 37 insertions, 22 deletions

diff --git a/clang-tools-extra/clang-tidy/misc/ArgumentCommentCheck.cpp b/clang-tools-extra/clang-tidy/misc/ArgumentCommentCheck.cpp
index a0bdd8a98ed..7bf602152de 100644
--- a/clang-tools-extra/clang-tidy/misc/ArgumentCommentCheck.cpp
+++ b/clang-tools-extra/clang-tidy/misc/ArgumentCommentCheck.cpp
@@ -22,16 +22,21 @@ namespace misc {
 ArgumentCommentCheck::ArgumentCommentCheck(StringRef Name,
                                            ClangTidyContext *Context)
     : ClangTidyCheck(Name, Context),
+      StrictMode(Options.getLocalOrGlobal("StrictMode", 0) != 0),
       IdentRE("^(/\\* *)([_A-Za-z][_A-Za-z0-9]*)( *= *\\*/)$") {}
 
+void ArgumentCommentCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
+  Options.store(Opts, "StrictMode", StrictMode);
+}
+
 void ArgumentCommentCheck::registerMatchers(MatchFinder *Finder) {
   Finder->addMatcher(
       callExpr(unless(cxxOperatorCallExpr()),
                // NewCallback's arguments relate to the pointed function, don't
                // check them against NewCallback's parameter names.
                // FIXME: Make this configurable.
-               unless(hasDeclaration(functionDecl(anyOf(
-                   hasName("NewCallback"), hasName("NewPermanentCallback"))))))
+               unless(hasDeclaration(functionDecl(
+                   hasAnyName("NewCallback", "NewPermanentCallback")))))
           .bind("expr"),
       this);
   Finder->addMatcher(cxxConstructExpr().bind("expr"), this);
@@ -78,12 +83,13 @@ getCommentsInRange(ASTContext *Ctx, CharSourceRange Range) {
   return Comments;
 }
 
-bool
-ArgumentCommentCheck::isLikelyTypo(llvm::ArrayRef<ParmVarDecl *> Params,
-                                   StringRef ArgName, unsigned ArgIndex) {
-  std::string ArgNameLower = ArgName.lower();
+bool ArgumentCommentCheck::isLikelyTypo(llvm::ArrayRef<ParmVarDecl *> Params,
+                                        StringRef ArgName, unsigned ArgIndex) {
+  std::string ArgNameLowerStr = ArgName.lower();
+  StringRef ArgNameLower = ArgNameLowerStr;
+  // The threshold is arbitrary.
   unsigned UpperBound = (ArgName.size() + 2) / 3 + 1;
-  unsigned ThisED = StringRef(ArgNameLower).edit_distance(
+  unsigned ThisED = ArgNameLower.edit_distance(
       Params[ArgIndex]->getIdentifier()->getName().lower(),
       /*AllowReplacements=*/true, UpperBound);
   if (ThisED >= UpperBound)
@@ -100,9 +106,9 @@ ArgumentCommentCheck::isLikelyTypo(llvm::ArrayRef<ParmVarDecl *> Params,
     // Other parameters must be an edit distance at least Threshold more away
     // from this parameter. This gives us greater confidence that this is a typo
     // of this parameter and not one with a similar name.
-    unsigned OtherED = StringRef(ArgNameLower).edit_distance(
-        II->getName().lower(),
-        /*AllowReplacements=*/true, ThisED + Threshold);
+    unsigned OtherED = ArgNameLower.edit_distance(II->getName().lower(),
+                                                  /*AllowReplacements=*/true,
+                                                  ThisED + Threshold);
     if (OtherED < ThisED + Threshold)
       return false;
   }
@@ -110,15 +116,24 @@ ArgumentCommentCheck::isLikelyTypo(llvm::ArrayRef<ParmVarDecl *> Params,
   return true;
 }
 
+static bool sameName(StringRef InComment, StringRef InDecl, bool StrictMode) {
+  if (StrictMode)
+    return InComment == InDecl;
+  InComment = InComment.trim('_');
+  InDecl = InDecl.trim('_');
+  // FIXME: compare_lower only works for ASCII.
+  return InComment.compare_lower(InDecl) == 0;
+}
+
 void ArgumentCommentCheck::checkCallArgs(ASTContext *Ctx,
                                          const FunctionDecl *Callee,
                                          SourceLocation ArgBeginLoc,
                                          llvm::ArrayRef<const Expr *> Args) {
   Callee = Callee->getFirstDecl();
-  for (unsigned i = 0,
-                e = std::min<unsigned>(Args.size(), Callee->getNumParams());
-       i != e; ++i) {
-    const ParmVarDecl *PVD = Callee->getParamDecl(i);
+  for (unsigned I = 0,
+                E = std::min<unsigned>(Args.size(), Callee->getNumParams());
+       I != E; ++I) {
+    const ParmVarDecl *PVD = Callee->getParamDecl(I);
     IdentifierInfo *II = PVD->getIdentifier();
     if (!II)
       continue;
@@ -126,28 +141,28 @@ void ArgumentCommentCheck::checkCallArgs(ASTContext *Ctx,
       // Don't warn on arguments for parameters instantiated from template
       // parameter packs. If we find more arguments than the template definition
       // has, it also means that they correspond to a parameter pack.
-      if (Template->getNumParams() <= i ||
-          Template->getParamDecl(i)->isParameterPack()) {
+      if (Template->getNumParams() <= I ||
+          Template->getParamDecl(I)->isParameterPack()) {
         continue;
       }
     }
 
     CharSourceRange BeforeArgument = CharSourceRange::getCharRange(
-        i == 0 ? ArgBeginLoc : Args[i - 1]->getLocEnd(),
-        Args[i]->getLocStart());
+        I == 0 ? ArgBeginLoc : Args[I - 1]->getLocEnd(),
+        Args[I]->getLocStart());
     BeforeArgument = Lexer::makeFileCharRange(
         BeforeArgument, Ctx->getSourceManager(), Ctx->getLangOpts());
 
     for (auto Comment : getCommentsInRange(Ctx, BeforeArgument)) {
       llvm::SmallVector<StringRef, 2> Matches;
       if (IdentRE.match(Comment.second, &Matches)) {
-        if (Matches[2] != II->getName()) {
+        if (!sameName(Matches[2], II->getName(), StrictMode)) {
           {
             DiagnosticBuilder Diag =
                 diag(Comment.first, "argument name '%0' in comment does not "
                                     "match parameter name %1")
                 << Matches[2] << II;
-            if (isLikelyTypo(Callee->parameters(), Matches[2], i)) {
+            if (isLikelyTypo(Callee->parameters(), Matches[2], I)) {
               Diag << FixItHint::CreateReplacement(
                           Comment.first,
                           (Matches[1] + II->getName() + Matches[3]).str());
@@ -163,7 +178,7 @@ void ArgumentCommentCheck::checkCallArgs(ASTContext *Ctx,
 
 void ArgumentCommentCheck::check(const MatchFinder::MatchResult &Result) {
   const Expr *E = Result.Nodes.getNodeAs<Expr>("expr");
-  if (auto Call = dyn_cast<CallExpr>(E)) {
+  if (const auto *Call = dyn_cast<CallExpr>(E)) {
     const FunctionDecl *Callee = Call->getDirectCallee();
     if (!Callee)
       return;
@@ -171,7 +186,7 @@ void ArgumentCommentCheck::check(const MatchFinder::MatchResult &Result) {
     checkCallArgs(Result.Context, Callee, Call->getCallee()->getLocEnd(),
                   llvm::makeArrayRef(Call->getArgs(), Call->getNumArgs()));
   } else {
-    auto Construct = cast<CXXConstructExpr>(E);
+    const auto *Construct = cast<CXXConstructExpr>(E);
     checkCallArgs(
         Result.Context, Construct->getConstructor(),
         Construct->getParenOrBraceRange().getBegin(),