[clang-tidy] Add options to bugprone-argument-comment to add missing argument comments to literals

bugprone-argument-comment only supports identifying those comments which do not match the function parameter name

This revision add 3 options to adding missing argument comments to literals (granularity on type is added to control verbosity of fixit)

```
CheckOptions:
  - key:             bugprone-argument-comment.CommentBoolLiterals
    value:           '1'
  - key:             bugprone-argument-comment.CommentFloatLiterals
    value:           '1'
  - key:             bugprone-argument-comment.CommentIntegerLiterals
    value:           '1'
  - key:             bugprone-argument-comment.CommentStringLiterals
    value:           '1'
  - key:             bugprone-argument-comment.CommentCharacterLiterals
    value:           '1'
  - key:             bugprone-argument-comment.CommentUserDefinedLiterals
    value:           '1'
  - key:             bugprone-argument-comment.CommentNullPtrs
    value:           '1'
```

After applying these options, literal arguments will be preceded with /*ParameterName=*/

Reviewers: JonasToth, Eugene.Zelenko, alexfh, hokein, aaron.ballman

Reviewed By: aaron.ballman, Eugene.Zelenko

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

git-svn-id: https://llvm.org/svn/llvm-project/clang-tools-extra/trunk@353535 91177308-0d34-0410-b5e6-96231b3b80d8

5 files changed, 354 insertions, 12 deletions

diff --git a/clang-tidy/bugprone/ArgumentCommentCheck.cpp b/clang-tidy/bugprone/ArgumentCommentCheck.cpp
index f939aa0e..5d6c7c9a 100644
--- a/clang-tidy/bugprone/ArgumentCommentCheck.cpp
+++ b/clang-tidy/bugprone/ArgumentCommentCheck.cpp
@@ -11,6 +11,7 @@
 #include "clang/ASTMatchers/ASTMatchFinder.h"
 #include "clang/Lex/Lexer.h"
 #include "clang/Lex/Token.h"
+
 #include "../utils/LexerUtils.h"
 
 using namespace clang::ast_matchers;
@@ -23,17 +24,37 @@ ArgumentCommentCheck::ArgumentCommentCheck(StringRef Name,
                                            ClangTidyContext *Context)
     : ClangTidyCheck(Name, Context),
       StrictMode(Options.getLocalOrGlobal("StrictMode", 0) != 0),
+      CommentBoolLiterals(Options.getLocalOrGlobal("CommentBoolLiterals", 0) !=
+                          0),
+      CommentIntegerLiterals(
+          Options.getLocalOrGlobal("CommentIntegerLiterals", 0) != 0),
+      CommentFloatLiterals(
+          Options.getLocalOrGlobal("CommentFloatLiterals", 0) != 0),
+      CommentStringLiterals(
+          Options.getLocalOrGlobal("CommentStringLiterals", 0) != 0),
+      CommentUserDefinedLiterals(
+          Options.getLocalOrGlobal("CommentUserDefinedLiterals", 0) != 0),
+      CommentCharacterLiterals(
+          Options.getLocalOrGlobal("CommentCharacterLiterals", 0) != 0),
+      CommentNullPtrs(Options.getLocalOrGlobal("CommentNullPtrs", 0) != 0),
       IdentRE("^(/\\* *)([_A-Za-z][_A-Za-z0-9]*)( *= *\\*/)$") {}
 
 void ArgumentCommentCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
   Options.store(Opts, "StrictMode", StrictMode);
+  Options.store(Opts, "CommentBoolLiterals", CommentBoolLiterals);
+  Options.store(Opts, "CommentIntegerLiterals", CommentIntegerLiterals);
+  Options.store(Opts, "CommentFloatLiterals", CommentFloatLiterals);
+  Options.store(Opts, "CommentStringLiterals", CommentStringLiterals);
+  Options.store(Opts, "CommentUserDefinedLiterals", CommentUserDefinedLiterals);
+  Options.store(Opts, "CommentCharacterLiterals", CommentCharacterLiterals);
+  Options.store(Opts, "CommentNullPtrs", CommentNullPtrs);
 }
 
 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.
+               // NewCallback's arguments relate to the pointed function,
+               // don't check them against NewCallback's parameter names.
                // FIXME: Make this configurable.
                unless(hasDeclaration(functionDecl(
                    hasAnyName("NewCallback", "NewPermanentCallback")))))
@@ -126,8 +147,8 @@ static bool isLikelyTypo(llvm::ArrayRef<ParmVarDecl *> Params,
 
     const unsigned Threshold = 2;
     // 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.
+    // 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 = ArgNameLower.edit_distance(II->getName().lower(),
                                                   /*AllowReplacements=*/true,
                                                   ThisED + Threshold);
@@ -180,8 +201,8 @@ static const CXXMethodDecl *findMockedMethod(const CXXMethodDecl *Method) {
     }
     return nullptr;
   }
-  if (const auto *Next = dyn_cast_or_null<CXXMethodDecl>(
-                 Method->getNextDeclInContext())) {
+  if (const auto *Next =
+          dyn_cast_or_null<CXXMethodDecl>(Method->getNextDeclInContext())) {
     if (looksLikeExpectMethod(Next) && areMockAndExpectMethods(Method, Next))
       return Method;
   }
@@ -206,6 +227,21 @@ static const FunctionDecl *resolveMocks(const FunctionDecl *Func) {
   return Func;
 }
 
+// Given the argument type and the options determine if we should
+// be adding an argument comment.
+bool ArgumentCommentCheck::shouldAddComment(const Expr *Arg) const {
+  if (Arg->getExprLoc().isMacroID())
+    return false;
+  Arg = Arg->IgnoreImpCasts();
+  return (CommentBoolLiterals && isa<CXXBoolLiteralExpr>(Arg)) ||
+         (CommentIntegerLiterals && isa<IntegerLiteral>(Arg)) ||
+         (CommentFloatLiterals && isa<FloatingLiteral>(Arg)) ||
+         (CommentUserDefinedLiterals && isa<UserDefinedLiteral>(Arg)) ||
+         (CommentCharacterLiterals && isa<CharacterLiteral>(Arg)) ||
+         (CommentStringLiterals && isa<StringLiteral>(Arg)) ||
+         (CommentNullPtrs && isa<CXXNullPtrLiteralExpr>(Arg));
+}
+
 void ArgumentCommentCheck::checkCallArgs(ASTContext *Ctx,
                                          const FunctionDecl *OriginalCallee,
                                          SourceLocation ArgBeginLoc,
@@ -219,7 +255,7 @@ void ArgumentCommentCheck::checkCallArgs(ASTContext *Ctx,
   if (NumArgs == 0)
     return;
 
-  auto makeFileCharRange = [Ctx](SourceLocation Begin, SourceLocation End) {
+  auto MakeFileCharRange = [Ctx](SourceLocation Begin, SourceLocation End) {
     return Lexer::makeFileCharRange(CharSourceRange::getCharRange(Begin, End),
                                     Ctx->getSourceManager(),
                                     Ctx->getLangOpts());
@@ -242,7 +278,7 @@ void ArgumentCommentCheck::checkCallArgs(ASTContext *Ctx,
     }
 
     CharSourceRange BeforeArgument =
-        makeFileCharRange(ArgBeginLoc, Args[I]->getBeginLoc());
+        MakeFileCharRange(ArgBeginLoc, Args[I]->getBeginLoc());
     ArgBeginLoc = Args[I]->getEndLoc();
 
     std::vector<std::pair<SourceLocation, StringRef>> Comments;
@@ -250,7 +286,7 @@ void ArgumentCommentCheck::checkCallArgs(ASTContext *Ctx,
       Comments = getCommentsInRange(Ctx, BeforeArgument);
     } else {
       // Fall back to parsing back from the start of the argument.
-      CharSourceRange ArgsRange = makeFileCharRange(
+      CharSourceRange ArgsRange = MakeFileCharRange(
           Args[I]->getBeginLoc(), Args[NumArgs - 1]->getEndLoc());
       Comments = getCommentsBeforeLoc(Ctx, ArgsRange.getBegin());
     }
@@ -277,8 +313,19 @@ void ArgumentCommentCheck::checkCallArgs(ASTContext *Ctx,
         }
       }
     }
+
+    // If the argument comments are missing for literals add them.
+    if (Comments.empty() && shouldAddComment(Args[I])) {
+      std::string ArgComment =
+          (llvm::Twine("/*") + II->getName() + "=*/").str();
+      DiagnosticBuilder Diag =
+          diag(Args[I]->getBeginLoc(),
+               "argument comment missing for literal argument %0")
+          << II
+          << FixItHint::CreateInsertion(Args[I]->getBeginLoc(), ArgComment);
+    }
   }
-}
+} // namespace bugprone
 
 void ArgumentCommentCheck::check(const MatchFinder::MatchResult &Result) {
   const auto *E = Result.Nodes.getNodeAs<Expr>("expr");
diff --git a/clang-tidy/bugprone/ArgumentCommentCheck.h b/clang-tidy/bugprone/ArgumentCommentCheck.h
index 2eee74d6..451b309c 100644
--- a/clang-tidy/bugprone/ArgumentCommentCheck.h
+++ b/clang-tidy/bugprone/ArgumentCommentCheck.h
@@ -26,7 +26,8 @@ namespace bugprone {
 ///
 ///   ...
 ///   f(/*bar=*/true);
-///   // warning: argument name 'bar' in comment does not match parameter name 'foo'
+///   // warning: argument name 'bar' in comment does not match parameter name
+///   'foo'
 /// \endcode
 ///
 /// The check tries to detect typos and suggest automated fixes for them.
@@ -39,12 +40,21 @@ public:
   void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
 
 private:
-  const bool StrictMode;
+  const unsigned StrictMode : 1;
+  const unsigned CommentBoolLiterals : 1;
+  const unsigned CommentIntegerLiterals : 1;
+  const unsigned CommentFloatLiterals : 1;
+  const unsigned CommentStringLiterals : 1;
+  const unsigned CommentUserDefinedLiterals : 1;
+  const unsigned CommentCharacterLiterals : 1;
+  const unsigned CommentNullPtrs : 1;
   llvm::Regex IdentRE;
 
   void checkCallArgs(ASTContext *Ctx, const FunctionDecl *Callee,
                      SourceLocation ArgBeginLoc,
                      llvm::ArrayRef<const Expr *> Args);
+
+  bool shouldAddComment(const Expr *Arg) const;
 };
 
 } // namespace bugprone
diff --git a/docs/ReleaseNotes.rst b/docs/ReleaseNotes.rst
index 9f833954..f932af42 100644
--- a/docs/ReleaseNotes.rst
+++ b/docs/ReleaseNotes.rst
@@ -92,6 +92,12 @@ Improvements to clang-tidy
   Checks whether there are underscores in googletest test and test case names in
   test macros, which is prohibited by the Googletest FAQ.
 
+- The :doc:`bugprone-argument-comment
+  <clang-tidy/checks/bugprone-argument-comment>` now supports
+  `CommentBoolLiterals`, `CommentIntegerLiterals`,  `CommentFloatLiterals`,
+  `CommentUserDefiniedLiterals`, `CommentStringLiterals`,
+  `CommentCharacterLiterals` & `CommentNullPtrs` options.
+
 Improvements to include-fixer
 -----------------------------
 
diff --git a/docs/clang-tidy/checks/bugprone-argument-comment.rst b/docs/clang-tidy/checks/bugprone-argument-comment.rst
index 5f7e5f0d..afc12186 100644
--- a/docs/clang-tidy/checks/bugprone-argument-comment.rst
+++ b/docs/clang-tidy/checks/bugprone-argument-comment.rst
@@ -27,3 +27,158 @@ Options
    When zero (default value), the check will ignore leading and trailing
    underscores and case when comparing names -- otherwise they are taken into
    account.
+
+.. option:: CommentBoolLiterals
+
+   When true, the check will add argument comments in the format
+   ``/*ParameterName=*/`` right before the boolean literal argument.
+
+Before:
+
+.. code-block:: c++
+
+  void foo(bool TurnKey, bool PressButton);
+
+  foo(true, false);
+
+After:
+
+.. code-block:: c++
+
+  void foo(bool TurnKey, bool PressButton);
+
+  foo(/*TurnKey=*/true, /*PressButton=*/false);
+
+.. option:: CommentIntegerLiterals
+
+   When true, the check will add argument comments in the format
+   ``/*ParameterName=*/`` right before the integer literal argument.
+
+Before:
+
+.. code-block:: c++
+
+  void foo(int MeaningOfLife);
+
+  foo(42);
+
+After:
+
+.. code-block:: c++
+
+  void foo(int MeaningOfLife);
+
+  foo(/*MeaningOfLife=*/42);
+
+.. option:: CommentFloatLiterals
+
+   When true, the check will add argument comments in the format
+   ``/*ParameterName=*/`` right before the float/double literal argument.
+
+Before:
+
+.. code-block:: c++
+
+  void foo(float Pi);
+
+  foo(3.14159);
+
+After:
+
+.. code-block:: c++
+
+  void foo(float Pi);
+
+  foo(/*Pi=*/3.14159);
+
+.. option:: CommentStringLiterals
+
+   When true, the check will add argument comments in the format
+   ``/*ParameterName=*/`` right before the string literal argument.
+
+Before:
+
+.. code-block:: c++
+
+  void foo(const char *String);
+  void foo(const wchar_t *WideString);
+
+  foo("Hello World");
+  foo(L"Hello World");
+
+After:
+
+.. code-block:: c++
+
+  void foo(const char *String);
+  void foo(const wchar_t *WideString);
+
+  foo(/*String=*/"Hello World");
+  foo(/*WideString=*/L"Hello World");
+
+.. option:: CommentCharacterLiterals
+
+   When true, the check will add argument comments in the format
+   ``/*ParameterName=*/`` right before the character literal argument.
+
+Before:
+
+.. code-block:: c++
+
+  void foo(char *Character);
+
+  foo('A');
+
+After:
+
+.. code-block:: c++
+
+  void foo(char *Character);
+
+  foo(/*Character=*/'A');
+
+.. option:: CommentUserDefinedLiterals
+
+   When true, the check will add argument comments in the format
+   ``/*ParameterName=*/`` right before the user defined literal argument.
+
+Before:
+
+.. code-block:: c++
+
+  void foo(double Distance);
+
+  double operator"" _km(long double);
+
+  foo(402.0_km);
+
+After:
+
+.. code-block:: c++
+
+  void foo(double Distance);
+
+  double operator"" _km(long double);
+
+  foo(/*Distance=*/402.0_km);
+
+.. option:: CommentNullPtrs
+
+   When true, the check will add argument comments in the format
+   ``/*ParameterName=*/`` right before the nullptr literal argument.
+
+Before:
+
+.. code-block:: c++
+
+  void foo(A* Value);
+
+  foo(nullptr);
+
+After:
+
+.. code-block:: c++
+
+  void foo(A* Value);
+
+  foo(/*Value=*/nullptr);
diff --git a/test/clang-tidy/bugprone-argument-comment-literals.cpp b/test/clang-tidy/bugprone-argument-comment-literals.cpp
new file mode 100644
index 00000000..739c9a59
--- /dev/null
+++ b/test/clang-tidy/bugprone-argument-comment-literals.cpp
@@ -0,0 +1,124 @@
+// RUN: %check_clang_tidy %s bugprone-argument-comment %t -- \
+// RUN:   -config="{CheckOptions: [{key: CommentBoolLiterals, value: 1},{key: CommentIntegerLiterals, value: 1}, {key: CommentFloatLiterals, value: 1}, {key: CommentUserDefinedLiterals, value: 1}, {key: CommentStringLiterals, value: 1}, {key: CommentNullPtrs, value: 1}, {key: CommentCharacterLiterals, value: 1}]}" --
+
+struct A {
+  void foo(bool abc);
+  void foo(bool abc, bool cde);
+  void foo(const char *, bool abc);
+  void foo(int iabc);
+  void foo(float fabc);
+  void foo(double dabc);
+  void foo(const char *strabc);
+  void fooW(const wchar_t *wstrabc);
+  void fooPtr(A *ptrabc);
+  void foo(char chabc);
+};
+
+#define FOO 1
+
+void g(int a);
+void h(double b);
+void i(const char *c);
+
+double operator"" _km(long double);
+
+void test() {
+  A a;
+
+  a.foo(true);
+  // CHECK-MESSAGES: [[@LINE-1]]:9: warning: argument comment missing for literal argument 'abc' [bugprone-argument-comment]
+  // CHECK-FIXES: a.foo(/*abc=*/true);
+
+  a.foo(false);
+  // CHECK-MESSAGES: [[@LINE-1]]:9: warning: argument comment missing for literal argument 'abc' [bugprone-argument-comment]
+  // CHECK-FIXES: a.foo(/*abc=*/false);
+
+  a.foo(true, false);
+  // CHECK-MESSAGES: [[@LINE-1]]:9: warning: argument comment missing for literal argument 'abc' [bugprone-argument-comment]
+  // CHECK-MESSAGES: [[@LINE-2]]:15: warning: argument comment missing for literal argument 'cde' [bugprone-argument-comment]
+  // CHECK-FIXES: a.foo(/*abc=*/true, /*cde=*/false);
+
+  a.foo(false, true);
+  // CHECK-MESSAGES: [[@LINE-1]]:9: warning: argument comment missing for literal argument 'abc' [bugprone-argument-comment]
+  // CHECK-MESSAGES: [[@LINE-2]]:16: warning: argument comment missing for literal argument 'cde' [bugprone-argument-comment]
+  // CHECK-FIXES: a.foo(/*abc=*/false, /*cde=*/true);
+
+  a.foo(/*abc=*/false, true);
+  // CHECK-MESSAGES: [[@LINE-1]]:24: warning: argument comment missing for literal argument 'cde' [bugprone-argument-comment]
+  // CHECK-FIXES: a.foo(/*abc=*/false, /*cde=*/true);
+
+  a.foo(false, /*cde=*/true);
+  // CHECK-MESSAGES: [[@LINE-1]]:9: warning: argument comment missing for literal argument 'abc' [bugprone-argument-comment]
+  // CHECK-FIXES: a.foo(/*abc=*/false, /*cde=*/true);
+
+  bool val1 = true;
+  bool val2 = false;
+  a.foo(val1, val2);
+
+  a.foo("", true);
+  // CHECK-MESSAGES: [[@LINE-1]]:13: warning: argument comment missing for literal argument 'abc' [bugprone-argument-comment]
+  // CHECK-FIXES: a.foo("", /*abc=*/true);
+
+  a.foo(0);
+  // CHECK-MESSAGES: [[@LINE-1]]:9: warning: argument comment missing for literal argument 'iabc' [bugprone-argument-comment]
+  // CHECK-FIXES: a.foo(/*iabc=*/0);
+
+  a.foo(1.0f);
+  // CHECK-MESSAGES: [[@LINE-1]]:9: warning: argument comment missing for literal argument 'fabc' [bugprone-argument-comment]
+  // CHECK-FIXES: a.foo(/*fabc=*/1.0f);
+
+  a.foo(1.0);
+  // CHECK-MESSAGES: [[@LINE-1]]:9: warning: argument comment missing for literal argument 'dabc' [bugprone-argument-comment]
+  // CHECK-FIXES: a.foo(/*dabc=*/1.0);
+
+  int val3 = 10;
+  a.foo(val3);
+
+  float val4 = 10.0;
+  a.foo(val4);
+
+  double val5 = 10.0;
+  a.foo(val5);
+
+  a.foo("Hello World");
+  // CHECK-MESSAGES: [[@LINE-1]]:9: warning: argument comment missing for literal argument 'strabc' [bugprone-argument-comment]
+  // CHECK-FIXES: a.foo(/*strabc=*/"Hello World");
+  //
+  a.fooW(L"Hello World");
+  // CHECK-MESSAGES: [[@LINE-1]]:10: warning: argument comment missing for literal argument 'wstrabc' [bugprone-argument-comment]
+  // CHECK-FIXES: a.fooW(/*wstrabc=*/L"Hello World");
+
+  a.fooPtr(nullptr);
+  // CHECK-MESSAGES: [[@LINE-1]]:12: warning: argument comment missing for literal argument 'ptrabc' [bugprone-argument-comment]
+  // CHECK-FIXES: a.fooPtr(/*ptrabc=*/nullptr);
+
+  a.foo(402.0_km);
+  // CHECK-MESSAGES: [[@LINE-1]]:9: warning: argument comment missing for literal argument 'dabc' [bugprone-argument-comment]
+  // CHECK-FIXES: a.foo(/*dabc=*/402.0_km);
+
+  a.foo('A');
+  // CHECK-MESSAGES: [[@LINE-1]]:9: warning: argument comment missing for literal argument 'chabc' [bugprone-argument-comment]
+  // CHECK-FIXES: a.foo(/*chabc=*/'A');
+
+  g(FOO);
+  h(1.0f);
+  // CHECK-MESSAGES: [[@LINE-1]]:5: warning: argument comment missing for literal argument 'b' [bugprone-argument-comment]
+  // CHECK-FIXES: h(/*b=*/1.0f);
+  i(__FILE__);
+
+  // FIXME Would like the below to add argument comments.
+  g((1));
+  // FIXME But we should not add argument comments here.
+  g(_Generic(0, int : 0));
+}
+
+void f(bool _with_underscores_);
+void ignores_underscores() {
+  f(false);
+  // CHECK-MESSAGES: [[@LINE-1]]:5: warning: argument comment missing for literal argument '_with_underscores_' [bugprone-argument-comment]
+  // CHECK-FIXES: f(/*_with_underscores_=*/false);
+
+  f(true);
+  // CHECK-MESSAGES: [[@LINE-1]]:5: warning: argument comment missing for literal argument
+  // CHECK-FIXES: f(/*_with_underscores_=*/true);
+}