Add a bugprone-argument-comment option: IgnoreSingleArgument.

[xyb]

Summary

Add bugprone-argument-comment option: IgnoreSingleArgument.
When true, the check will ignore the single argument.

Sometimes, it's not necessary to add comment to single argument.
For example:

std::string name("Yubo Xie");
pScreen->SetWidth(1920);
pScreen->SetHeight(1080);

This option can ignore such single argument in bugprone-argument-comment check.

[alexfh]

LG with a comment.

clang-tools-extra/clang-tidy/bugprone/ArgumentCommentCheck.cpp

28 ↗

(On Diff #218281)

Why is it a global option? Are there other checks that would need the same option? The one below also needs to be check-local.

[xyb]

Thanks. BTW, I can't commit the patch by myself.

[alexfh]

In D67056#1657540, @xyb wrote:

Thanks. BTW, I can't commit the patch by myself.

There's an outstanding comment. Please address it first, and I'll be happy to commit your patch.

[xyb]

clang-tools-extra/clang-tidy/bugprone/ArgumentCommentCheck.cpp

28 ↗

(On Diff #218281)

Sorry, I'm afraid I didn't get your point. Could you please explain more? This setting just follows the same pattern used for other settings. All other settings use "Options.getLocalOrGlobal(...)", I'm not sure why this setting need be different. Or do you mean we should change other settings ("StrictMode", "CommentBoolLiterals", "CommentIntegerLiterals", ...) to local options also?

[alexfh]

clang-tools-extra/clang-tidy/bugprone/ArgumentCommentCheck.cpp

28 ↗

(On Diff #218281)

Options are stored as a string -> string map. The key in this map is either the option name prepended with the check name (for check-local options) or just the option name (this way an option can be read by all checks). There are two ways to read options: OptionsView::get reads just the local name, OptionsView::getLocalOrGlobal tries the check-local name and then falls back to reading the option using its global name.

In this particular check only the StrictMode option makes sense to be global (a few other checks define an option with the same name and set of values, and it may make sense to configure a global default value). Other options are specific to this check and should be local.

[xyb]

clang-tools-extra/clang-tidy/bugprone/ArgumentCommentCheck.cpp

28 ↗

(On Diff #218281)

Thanks for your explanation! Updated.

[xyb]

Update patch.

clang-tools-extra/trunk/clang-tidy/bugprone/ArgumentCommentCheck.h

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 unsigned StrictMode : 1;
+  const unsigned IgnoreSingleArgument : 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;

clang-tools-extra/trunk/clang-tidy/bugprone/ArgumentCommentCheck.cpp

 namespace clang {
 namespace tidy {
 namespace bugprone {
 
 ArgumentCommentCheck::ArgumentCommentCheck(StringRef Name,
                                            ClangTidyContext *Context)
     : ClangTidyCheck(Name, Context),
       StrictMode(Options.getLocalOrGlobal("StrictMode", 0) != 0),
+      IgnoreSingleArgument(Options.get("IgnoreSingleArgument", 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, "IgnoreSingleArgument", IgnoreSingleArgument);
   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::checkCallArgs(ASTContext *Ctx,
                                          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)
+  if ((NumArgs == 0) || (IgnoreSingleArgument && NumArgs == 1))
     return;
 
   auto MakeFileCharRange = [Ctx](SourceLocation Begin, SourceLocation End) {
     return Lexer::makeFileCharRange(CharSourceRange::getCharRange(Begin, End),
                                     Ctx->getSourceManager(),
                                     Ctx->getLangOpts());
   };
 

clang-tools-extra/trunk/docs/clang-tidy/checks/bugprone-argument-comment.rst

 -------
 
 .. 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:: IgnoreSingleArgument
+   When true, the check will ignore the single argument.
+
 .. option:: CommentBoolLiterals
 
    When true, the check will add argument comments in the format
    ``/*ParameterName=*/`` right before the boolean literal argument.
 
 Before:
 
 .. code-block:: c++

clang-tools-extra/trunk/test/clang-tidy/bugprone-argument-comment-ignore-single-argument.cpp

+// RUN: %check_clang_tidy %s bugprone-argument-comment %t -- \
+// RUN:   -config="{CheckOptions: [{key: bugprone-argument-comment.IgnoreSingleArgument, value: 1}, {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);
+
+  a.foo(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);
+
+  a.foo(1.0f);
+
+  a.foo(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");
+
+  a.fooW(L"Hello World");
+
+  a.fooPtr(nullptr);
+
+  a.foo(402.0_km);
+
+  a.foo('A');
+
+  g(FOO);
+
+  h(1.0f);
+
+  i(__FILE__);
+
+  g((1));
+}
+
+void f(bool _with_underscores_);
+void ignores_underscores() {
+  f(false);
+
+  f(true);
+}