llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang-tidy Author: Daan De Meyer (daandemeyer) <details> <summary>Changes</summary> C code often names parameters with a single letter or a generic name such as arg1. A comment like /*p=*/NULL tells the reader nothing, but the Comment* options of bugprone-argument-comment still ask for one. When the caller writes /*path=*/NULL for a parameter named p instead, the check reports a mismatch. Let's add an option with a semicolon-separated list of regular expressions for the parameter names that the check skips, the format most clang-tidy options with regular expressions use. A regular expression that doesn't compile is a configuration error, like in misc-include-cleaner. Split out of https://github.com/llvm/llvm-project/pull/237623. --- Full diff: https://github.com/llvm/llvm-project/pull/237891.diff 5 Files Affected: - (modified) clang-tools-extra/clang-tidy/bugprone/ArgumentCommentCheck.cpp (+21-2) - (modified) clang-tools-extra/clang-tidy/bugprone/ArgumentCommentCheck.h (+2) - (modified) clang-tools-extra/docs/ReleaseNotes.md (+4) - (modified) clang-tools-extra/docs/clang-tidy/checks/bugprone/argument-comment.md (+9) - (added) clang-tools-extra/test/clang-tidy/checkers/bugprone/argument-comment-ignored-parameter-names.cpp (+24) ``````````diff diff --git a/clang-tools-extra/clang-tidy/bugprone/ArgumentCommentCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/ArgumentCommentCheck.cpp index 783944cd1a5a17..84ee08b334c9fb 100644 --- a/clang-tools-extra/clang-tidy/bugprone/ArgumentCommentCheck.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/ArgumentCommentCheck.cpp @@ -13,6 +13,7 @@ #include "clang/Lex/Token.h" #include "../utils/LexerUtils.h" +#include "../utils/OptionsUtils.h" using namespace clang::ast_matchers; @@ -49,7 +50,20 @@ ArgumentCommentCheck::ArgumentCommentCheck(StringRef Name, CommentTypedInitLists(Options.get("CommentTypedInitLists", false)), CommentUserDefinedLiterals( Options.get("CommentUserDefinedLiterals", false)), - IdentRE("^(/\\* *)([_A-Za-z][_A-Za-z0-9]*)( *= *\\*/)$") {} + IgnoredParameterNames(utils::options::parseStringList( + Options.get("IgnoredParameterNames", ""))), + IdentRE("^(/\\* *)([_A-Za-z][_A-Za-z0-9]*)( *= *\\*/)$") { + for (const StringRef Name : IgnoredParameterNames) { + llvm::Regex Regex(Name); + if (!Regex.isValid()) { + configurationDiag("Invalid regular expression '%0' in " + "IgnoredParameterNames") + << Name; + continue; + } + IgnoredParameterNameRegexes.push_back(std::move(Regex)); + } +} void ArgumentCommentCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) { Options.store(Opts, "StrictMode", StrictMode); @@ -65,6 +79,8 @@ void ArgumentCommentCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) { Options.store(Opts, "CommentStringLiterals", CommentStringLiterals); Options.store(Opts, "CommentTypedInitLists", CommentTypedInitLists); Options.store(Opts, "CommentUserDefinedLiterals", CommentUserDefinedLiterals); + Options.store(Opts, "IgnoredParameterNames", + utils::options::serializeStringList(IgnoredParameterNames)); } void ArgumentCommentCheck::registerMatchers(MatchFinder *Finder) { @@ -325,7 +341,10 @@ void ArgumentCommentCheck::checkCallArgs(ASTContext *Ctx, for (unsigned I = 0; I < NumArgs; ++I) { const ParmVarDecl *PVD = Callee->getParamDecl(I); const IdentifierInfo *II = PVD->getIdentifier(); - if (!II) + if (!II || llvm::any_of(IgnoredParameterNameRegexes, + [II](const llvm::Regex &Regex) { + return Regex.match(II->getName()); + })) continue; // Don't warn on arguments for parameters instantiated from template // parameter packs. If we find more arguments than the template diff --git a/clang-tools-extra/clang-tidy/bugprone/ArgumentCommentCheck.h b/clang-tools-extra/clang-tidy/bugprone/ArgumentCommentCheck.h index 3bed1ad4247e5b..758d7f3725b035 100644 --- a/clang-tools-extra/clang-tidy/bugprone/ArgumentCommentCheck.h +++ b/clang-tools-extra/clang-tidy/bugprone/ArgumentCommentCheck.h @@ -58,6 +58,8 @@ class ArgumentCommentCheck : public ClangTidyCheck { const unsigned CommentStringLiterals : 1; const unsigned CommentTypedInitLists : 1; const unsigned CommentUserDefinedLiterals : 1; + const std::vector<StringRef> IgnoredParameterNames; + std::vector<llvm::Regex> IgnoredParameterNameRegexes; llvm::Regex IdentRE; void checkCallArgs(ASTContext *Ctx, const FunctionDecl *Callee, diff --git a/clang-tools-extra/docs/ReleaseNotes.md b/clang-tools-extra/docs/ReleaseNotes.md index 679ca9ecbb4a39..e5ba36e01d2675 100644 --- a/clang-tools-extra/docs/ReleaseNotes.md +++ b/clang-tools-extra/docs/ReleaseNotes.md @@ -184,6 +184,10 @@ infrastructure are described first, followed by tool-specific sections. #### Changes in existing checks +- Improved {doc}`bugprone-argument-comment + <clang-tidy/checks/bugprone/argument-comment>` check by adding the + {option}`IgnoredParameterNames` option to skip parameters by name. + - Improved {doc}`bugprone-assignment-in-if-condition <clang-tidy/checks/bugprone/assignment-in-if-condition>` check by ignoring assignments inside GNU statement expressions unless they form the value of diff --git a/clang-tools-extra/docs/clang-tidy/checks/bugprone/argument-comment.md b/clang-tools-extra/docs/clang-tidy/checks/bugprone/argument-comment.md index 68f7a6eb7a260f..34dfc7029cbc43 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/bugprone/argument-comment.md +++ b/clang-tools-extra/docs/clang-tidy/checks/bugprone/argument-comment.md @@ -33,6 +33,15 @@ account. Default is `false`. When `true`, the check will ignore the single argument. Default is `false`. ``` +```{option} IgnoredParameterNames +A semicolon-separated list of regular expressions for the parameter names that +the check skips. The check neither compares the argument comment of a matching +parameter nor adds a missing comment for it. A regular expression matches +anywhere in the name unless you anchor it. Use the option for names that say +nothing about the parameter, such as `^[a-z]$;^arg[0-9]*$`. Default is an +empty string. +``` + ```{option} CommentAnonymousInitLists When `true`, the check will add argument comments in the format `/*ParameterName=*/` right before anonymous braced-init list arguments diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/argument-comment-ignored-parameter-names.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/argument-comment-ignored-parameter-names.cpp new file mode 100644 index 00000000000000..2d24371ab3b7c8 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/argument-comment-ignored-parameter-names.cpp @@ -0,0 +1,24 @@ +// RUN: %check_clang_tidy %s bugprone-argument-comment %t -- \ +// RUN: -config="{CheckOptions: { \ +// RUN: bugprone-argument-comment.CommentBoolLiterals: true, \ +// RUN: bugprone-argument-comment.IgnoredParameterNames: '^[a-z]$;^arg[0-9]*$'}}" -- +// RUN: %check_clang_tidy -check-suffix=INVALID %s bugprone-argument-comment %t -- \ +// RUN: -config="{CheckOptions: { \ +// RUN: bugprone-argument-comment.CommentBoolLiterals: true, \ +// RUN: bugprone-argument-comment.IgnoredParameterNames: '[;^arg[0-9]*$'}}" -- + +// CHECK-MESSAGES-INVALID: warning: Invalid regular expression '[' in IgnoredParameterNames [clang-tidy-config] + +void f(bool b, bool arg1, bool force); + +void test() { + f(true, false, true); + // CHECK-MESSAGES: [[@LINE-1]]:18: warning: argument comment missing for literal argument 'force' [bugprone-argument-comment] + // CHECK-MESSAGES-INVALID: [[@LINE-2]]:5: warning: argument comment missing for literal argument 'b' [bugprone-argument-comment] + // CHECK-MESSAGES-INVALID: [[@LINE-3]]:18: warning: argument comment missing for literal argument 'force' [bugprone-argument-comment] + // CHECK-FIXES: f(true, false, /*force=*/true); + // CHECK-FIXES-INVALID: f(/*b=*/true, false, /*force=*/true); + + f(/*enable=*/true, /*verbose=*/false, /*force=*/true); + // CHECK-MESSAGES-INVALID: [[@LINE-1]]:5: warning: argument name 'enable' in comment does not match parameter name 'b' [bugprone-argument-comment] +} `````````` </details> https://github.com/llvm/llvm-project/pull/237891 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
