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

[MyDeveloperDay]

Summary

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=*/

[MyDeveloperDay]

An example of this checker run over clang-tidy/modernize is given here D57675: [clang-tidy] DO NOT SUBMIT.. example diff of applying bugprone-argument-comments with AddCommentsToXXXLiterals options

[MyDeveloperDay]

docs/ReleaseNotes.rst

95

note to self, I will reorganize this alphabetically, on next revision

[MyDeveloperDay]

Fix release note alphabetic order
Add examples of Fixit into the documentation

[Eugene.Zelenko]

docs/ReleaseNotes.rst

82

Please move changes in existing checks after list of new checks.

[aaron.ballman]

Can you drop the file mode changes that are in this review?

clang-tidy/bugprone/ArgumentCommentCheck.cpp

318–319

These can be combined into a single if statement.

326–327

Why is this split into two lines?

330

This continue can be dropped without changing the semantics, correct?

clang-tidy/bugprone/ArgumentCommentCheck.h

44–46

Why not character or string literals?
What about nullptr literals or UDLs?

docs/clang-tidy/checks/bugprone-argument-comment.rst

40

Format the code examples from our style guide as well (same below).

[MyDeveloperDay]

Address review comments
Add support for additional StringLiterals,CharacterLiterals,UserDefinedLiterals and NullPtrs
Simplify the Options names from "AddCommentsToXXX" to "CommentXXX"
Add extra unit tests to support additional supported literal types

[aaron.ballman]

clang-tidy/bugprone/ArgumentCommentCheck.cpp

230–238

What's going on with these? Why not return isa<Blah>(Arg->IgnoreImpCasts()); (at which point, no need for the separate functions).

319–325

This is a bit... much. I'm not certain what would be a better approach, but at the very least, factoring this out into a simple function call should make it a bit less... much. :-)

clang-tidy/bugprone/ArgumentCommentCheck.h

43–51

At this point, I think it would be better to turn these into: const unsigned Blah : 1;

docs/clang-tidy/checks/bugprone-argument-comment.rst

40

This still seems to be happening in the current revision?

[MyDeveloperDay]

Address review comments

clang-tidy/bugprone/ArgumentCommentCheck.cpp

230–238

OK, my bad, I was just looking at the ast-dump on godbolt.org thinking... how do I get past that ImplicitCasrExpr, learning these tricks in the AST isn't always obvious despite me looking in doxygen, when you don't know what to look for its hard to know..but this is a neat trick and I'm happy to learn.

docs/clang-tidy/checks/bugprone-argument-comment.rst

40

Sorry didn't catch your meaning but I assume it was the space between the arguments...

[MyDeveloperDay]

clang-tidy/bugprone/ArgumentCommentCheck.cpp

262

@aaron.ballman, slight aside (and not in the code I introduced), when I run clang-tidy on the code I'm sending for review, I get the following...

C:\clang\llvm\tools\clang\tools\extra\clang-tidy\bugprone\ArgumentCommentCheck.cpp:253:8: warning: invalid case style for variable 'makeFileCharRange' [readability-identifier-naming]
  auto makeFileCharRange = [Ctx](SourceLocation Begin, SourceLocation End) {
       ^~~~~~~~~~~~~~~~~
       MakeFileCharRange

according to the .clang-tidy, a variable should be CamelCase, and a function camelBack, as its a Lambda what do we consider it should be? and does this really require an improvement in readability-identifier-naming? (might be something I'd be happy to look at next)

Checks: '-*,clang-diagnostic-*,llvm-*,misc-*,-misc-unused-parameters,readability-identifier-naming'
CheckOptions:
  - key:             readability-identifier-naming.ClassCase
    value:           CamelCase
  - key:             readability-identifier-naming.EnumCase
    value:           CamelCase
  - key:             readability-identifier-naming.FunctionCase
    value:           camelBack
  - key:             readability-identifier-naming.MemberCase
    value:           CamelCase
  - key:             readability-identifier-naming.ParameterCase
    value:           CamelCase
  - key:             readability-identifier-naming.UnionCase
    value:           CamelCase
  - key:             readability-identifier-naming.VariableCase
    value:           CamelCase

[aaron.ballman]

clang-tidy/bugprone/ArgumentCommentCheck.cpp

1

Why did this get removed?

230–238

No worries, I was just wondering if there was something special about a FullExpr that you didn't want to look though it for some reason. Glad to show you a new trick!

233

I think we may want to do some additional work here. Consider:

#define FOO 1

void g(int);
void h(double);
void i(const char *);

void f() {
  g(FOO); // Probably don't want to suggest comments here
  g((1)); // Probably do want to suggest comments here
  h(1.0f); // Probably do want to suggest comments here
  i(__FILE__); // Probably don't want to suggest comments here
}

I think this code should do two things: 1) check whether the location of the arg is a macro expansion (if so, return false), 2) do Arg = Arg->IgnoreParenImpCasts(); at the start of the function and drop the Arg->IgnoreImpCasts() for the string and pointer literal cases. However, that may be a bridge too far, so you should add a test case like this:

g(_Generic(0, int : 0)); // Definitely do not want to see comments here

If it turns out the above case gives the comment suggestions, then I'd go with IgnoreImpCasts() rather than IgnoreParenImpCasts().

262

according to the .clang-tidy, a variable should be CamelCase, and a function camelBack, as its a Lambda what do we consider it should be? and does this really require an improvement in readability-identifier-naming? (might be something I'd be happy to look at next)

Lambdas are variables, so I would expect those to follow the variable naming conventions. This is consistent with how we treat variables of function pointer type as well.

clang-tidy/bugprone/ArgumentCommentCheck.h

43–50

Can you include this in the bit-field packing as well (with type unsigned)?

docs/clang-tidy/checks/bugprone-argument-comment.rst

40

Yup, it was!

[MyDeveloperDay]

-Address review comments
-Ignore argument comment addition for macro inputs
-Add unit tests to cover the above
-Fix clang-tidy suggestion (from readability-identifier-naming)
-Ensure file is clang-formatted

[MyDeveloperDay]

clang-tidy/bugprone/ArgumentCommentCheck.cpp

1

Sorry I suspect poor vim skills

233

I agree, tried all combinations but g((1); and g(_Generic(0, int : 0)); would otherwise get comments

[aaron.ballman]

LGTM aside from a testing nit.

clang-tidy/bugprone/ArgumentCommentCheck.cpp

233

Yeah, I kind of figured that would be the case. Thank you for checking!

test/clang-tidy/bugprone-argument-comment-literals.cpp

105

Can you add a FIXME comment that says we'd like to handle this case at some point? Maybe move the test closer to the _Generic example so you can mention that case as being a problem when handling paren expressions.

[MyDeveloperDay]

Pre-Commit Changes

• add FIXME wording to test
• move local header include after libraries
• rebase

clang-tidy/bugprone/ArgumentCommentCheck.h

 /// The check understands argument comments in the form `/*parameter_name=*/`
 /// that are placed right before the argument.
 ///
 /// \code
 ///   void f(bool foo);
 ///
 ///   ...
 ///   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.
 class ArgumentCommentCheck : public ClangTidyCheck {
 public:
   ArgumentCommentCheck(StringRef Name, ClangTidyContext *Context);
 
   void registerMatchers(ast_matchers::MatchFinder *Finder) override;
   void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
   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
 } // namespace tidy
 } // namespace clang
 
 #endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_ARGUMENTCOMMENTCHECK_H

clang-tidy/bugprone/ArgumentCommentCheck.cpp

 //===--- ArgumentCommentCheck.cpp - clang-tidy ----------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "ArgumentCommentCheck.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
 #include "clang/Lex/Lexer.h"
 #include "clang/Lex/Token.h"
+
 #include "../utils/LexerUtils.h"
 
 using namespace clang::ast_matchers;
 
 namespace clang {
 namespace tidy {
 namespace bugprone {
 
 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")))))
           .bind("expr"),
       this);
   Finder->addMatcher(cxxConstructExpr().bind("expr"), this);
 }
 
for (unsigned I = 0, E = Params.size(); I != E; ++I) {
     if (I == ArgIndex)
       continue;
     IdentifierInfo *II = Params[I]->getIdentifier();
     if (!II)
       continue;
 
     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);
     if (OtherED < ThisED + Threshold)
       return false;
   }
 
   return true;
if (looksLikeExpectMethod(Method)) {
     for (const auto *D : Ctx->decls()) {
       if (D->getNextDeclInContext() == Method) {
         const auto *Previous = dyn_cast<CXXMethodDecl>(D);
         return areMockAndExpectMethods(Previous, Method) ? Previous : nullptr;
       }
     }
     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;
   }
   return nullptr;
 }
 
 // For gmock expectation builder method (the target of the call generated by
 // `EXPECT_CALL(obj, Method(...))`) tries to find the real method being mocked
 // (returns nullptr, if the mock method doesn't override anything). For other
 // functions returns the function itself.
 static const FunctionDecl *resolveMocks(const FunctionDecl *Func) {
   if (const auto *Method = dyn_cast<CXXMethodDecl>(Func)) {
     if (const auto *MockedMethod = findMockedMethod(Method)) {
       // If mocked method overrides the real one, we can use its parameter
       // names, otherwise we're out of luck.
       if (MockedMethod->size_overridden_methods() > 0) {
         return *MockedMethod->begin_overridden_methods();
       }
       return nullptr;
     }
   }
   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,
                                          llvm::ArrayRef<const Expr *> Args) {
   const FunctionDecl *Callee = resolveMocks(OriginalCallee);
   if (!Callee)
     return;
 
   Callee = Callee->getFirstDecl();
   unsigned NumArgs = std::min<unsigned>(Args.size(), Callee->getNumParams());
   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());
   };
 
   for (unsigned I = 0; I < NumArgs; ++I) {
     const ParmVarDecl *PVD = Callee->getParamDecl(I);
     IdentifierInfo *II = PVD->getIdentifier();
     if (!II)
       continue;
     if (auto Template = Callee->getTemplateInstantiationPattern()) {
       // 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()) {
         continue;
       }
     }
 
     CharSourceRange BeforeArgument =
-        makeFileCharRange(ArgBeginLoc, Args[I]->getBeginLoc());
+        MakeFileCharRange(ArgBeginLoc, Args[I]->getBeginLoc());
     ArgBeginLoc = Args[I]->getEndLoc();
 
     std::vector<std::pair<SourceLocation, StringRef>> Comments;
     if (BeforeArgument.isValid()) {
       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());
     }
 
     for (auto Comment : Comments) {
       llvm::SmallVector<StringRef, 2> Matches;
       if (IdentRE.match(Comment.second, &Matches) &&
           !sameName(Matches[2], II->getName(), StrictMode)) {
for (auto Comment : Comments) {
         diag(PVD->getLocation(), "%0 declared here", DiagnosticIDs::Note) << II;
         if (OriginalCallee != Callee) {
           diag(OriginalCallee->getLocation(),
                "actual callee (%0) is declared here", DiagnosticIDs::Note)
               << OriginalCallee;
         }
       }
     }
+
+    // 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");
   if (const auto *Call = dyn_cast<CallExpr>(E)) {
     const FunctionDecl *Callee = Call->getDirectCallee();
     if (!Callee)
       return;
 
     checkCallArgs(Result.Context, Callee, Call->getCallee()->getEndLoc(),
                   llvm::makeArrayRef(Call->getArgs(), Call->getNumArgs()));

docs/ReleaseNotes.rst

- New :doc:`abseil-duration-addition
   domain.
 
 - New :doc:`abseil-duration-conversion-cast
   <clang-tidy/checks/abseil-duration-conversion-cast>` check.
 
   Checks for casts of ``absl::Duration`` conversion functions, and recommends
   the right conversion function instead.
 
 - New :doc:`abseil-duration-unnecessary-conversion
   <clang-tidy/checks/abseil-duration-unnecessary-conversion>` check.
 
   Finds and fixes cases where ``absl::Duration`` values are being converted to
   numeric types and back again.
 
 - New :doc:`google-readability-avoid-underscore-in-googletest-name
   <clang-tidy/checks/google-readability-avoid-underscore-in-googletest-name>`
   check.
 
   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
 -----------------------------
 
 The improvements are...
 
 Improvements to modularize
 --------------------------
 
 The improvements are...

docs/clang-tidy/checks/bugprone-argument-comment.rst

 Options
 -------
 
 .. option:: StrictMode
 
    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);

test/clang-tidy/bugprone-argument-comment-literals.cpp

+// 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);
+}