Index: clang-tools-extra/trunk/clang-tidy/ClangTidy.h =================================================================== --- clang-tools-extra/trunk/clang-tidy/ClangTidy.h +++ clang-tools-extra/trunk/clang-tidy/ClangTidy.h @@ -73,6 +73,23 @@ return Result; } + /// \brief Read a named option from the ``Context`` and parse it as an + /// integral type ``T``. + /// + /// Reads the option with the check-local name \p LocalName from local or + /// global ``CheckOptions``. Gets local option first. If local is not present, + /// falls back to get global option. If global option is not present either, + /// returns Default. + template + typename std::enable_if::value, T>::type + getLocalOrGlobal(StringRef LocalName, T Default) const { + std::string Value = getLocalOrGlobal(LocalName, ""); + T Result = Default; + if (!Value.empty()) + StringRef(Value).getAsInteger(10, Result); + return Result; + } + /// \brief Stores an option with the check-local name \p LocalName with string /// value \p Value to \p Options. void store(ClangTidyOptions::OptionMap &Options, StringRef LocalName, Index: clang-tools-extra/trunk/clang-tidy/misc/ArgumentCommentCheck.h =================================================================== --- clang-tools-extra/trunk/clang-tidy/misc/ArgumentCommentCheck.h +++ clang-tools-extra/trunk/clang-tidy/misc/ArgumentCommentCheck.h @@ -37,8 +37,10 @@ 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; llvm::Regex IdentRE; bool isLikelyTypo(llvm::ArrayRef Params, StringRef ArgName, Index: clang-tools-extra/trunk/clang-tidy/misc/ArgumentCommentCheck.cpp =================================================================== --- clang-tools-extra/trunk/clang-tidy/misc/ArgumentCommentCheck.cpp +++ clang-tools-extra/trunk/clang-tidy/misc/ArgumentCommentCheck.cpp @@ -22,16 +22,21 @@ 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 @@ return Comments; } -bool -ArgumentCommentCheck::isLikelyTypo(llvm::ArrayRef Params, - StringRef ArgName, unsigned ArgIndex) { - std::string ArgNameLower = ArgName.lower(); +bool ArgumentCommentCheck::isLikelyTypo(llvm::ArrayRef 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 @@ // 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 @@ 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 Args) { Callee = Callee->getFirstDecl(); - for (unsigned i = 0, - e = std::min(Args.size(), Callee->getNumParams()); - i != e; ++i) { - const ParmVarDecl *PVD = Callee->getParamDecl(i); + for (unsigned I = 0, + E = std::min(Args.size(), Callee->getNumParams()); + I != E; ++I) { + const ParmVarDecl *PVD = Callee->getParamDecl(I); IdentifierInfo *II = PVD->getIdentifier(); if (!II) continue; @@ -126,28 +141,28 @@ // 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 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::check(const MatchFinder::MatchResult &Result) { const Expr *E = Result.Nodes.getNodeAs("expr"); - if (auto Call = dyn_cast(E)) { + if (const auto *Call = dyn_cast(E)) { const FunctionDecl *Callee = Call->getDirectCallee(); if (!Callee) return; @@ -171,7 +186,7 @@ checkCallArgs(Result.Context, Callee, Call->getCallee()->getLocEnd(), llvm::makeArrayRef(Call->getArgs(), Call->getNumArgs())); } else { - auto Construct = cast(E); + const auto *Construct = cast(E); checkCallArgs( Result.Context, Construct->getConstructor(), Construct->getParenOrBraceRange().getBegin(), Index: clang-tools-extra/trunk/docs/clang-tidy/checks/misc-argument-comment.rst =================================================================== --- clang-tools-extra/trunk/docs/clang-tidy/checks/misc-argument-comment.rst +++ clang-tools-extra/trunk/docs/clang-tidy/checks/misc-argument-comment.rst @@ -18,3 +18,7 @@ // warning: argument name 'bar' in comment does not match parameter name 'foo' The check tries to detect typos and suggest automated fixes for them. + +Supported options: + - `StrictMode` (local or global): when non-zero, the check will ignore leading + and trailing underscores and case when comparing parameter names. Index: clang-tools-extra/trunk/test/clang-tidy/misc-argument-comment-strict.cpp =================================================================== --- clang-tools-extra/trunk/test/clang-tidy/misc-argument-comment-strict.cpp +++ clang-tools-extra/trunk/test/clang-tidy/misc-argument-comment-strict.cpp @@ -0,0 +1,19 @@ +// RUN: %check_clang_tidy %s misc-argument-comment %t -- \ +// RUN: -config="{CheckOptions: [{key: StrictMode, value: 1}]}" -- + +void f(int _with_underscores_); +void g(int x_); +void ignores_underscores() { + f(/*With_Underscores=*/0); +// CHECK-MESSAGES: [[@LINE-1]]:5: warning: argument name 'With_Underscores' in comment does not match parameter name '_with_underscores_' +// CHECK-FIXES: f(/*_with_underscores_=*/0); + f(/*with_underscores=*/1); +// CHECK-MESSAGES: [[@LINE-1]]:5: warning: argument name 'with_underscores' in comment does not match parameter name '_with_underscores_' +// CHECK-FIXES: f(/*_with_underscores_=*/1); + f(/*_With_Underscores_=*/2); +// CHECK-MESSAGES: [[@LINE-1]]:5: warning: argument name '_With_Underscores_' in comment does not match parameter name '_with_underscores_' +// CHECK-FIXES: f(/*_with_underscores_=*/2); + g(/*X=*/3); +// CHECK-MESSAGES: [[@LINE-1]]:5: warning: argument name 'X' in comment does not match parameter name 'x_' +// CHECK-FIXES: g(/*x_=*/3); +} Index: clang-tools-extra/trunk/test/clang-tidy/misc-argument-comment.cpp =================================================================== --- clang-tools-extra/trunk/test/clang-tidy/misc-argument-comment.cpp +++ clang-tools-extra/trunk/test/clang-tidy/misc-argument-comment.cpp @@ -36,8 +36,8 @@ void templates() { variadic(/*xxx=*/0, /*yyy=*/1); - variadic2(/*zzZ=*/0, /*xxx=*/1, /*yyy=*/2); - // CHECK-MESSAGES: [[@LINE-1]]:13: warning: argument name 'zzZ' in comment does not match parameter name 'zzz' + variadic2(/*zzU=*/0, /*xxx=*/1, /*yyy=*/2); + // CHECK-MESSAGES: [[@LINE-1]]:13: warning: argument name 'zzU' in comment does not match parameter name 'zzz' // CHECK-FIXES: variadic2(/*zzz=*/0, /*xxx=*/1, /*yyy=*/2); } @@ -46,3 +46,8 @@ void f() { qqq(/*bbb=*/FALSE); } // CHECK-MESSAGES: [[@LINE-1]]:16: warning: argument name 'bbb' in comment does not match parameter name 'aaa' // CHECK-FIXES: void f() { qqq(/*bbb=*/FALSE); } + +void f(bool _with_underscores_); +void ignores_underscores() { + f(/*With_Underscores=*/false); +} Index: clang-tools-extra/trunk/unittests/clang-tidy/MiscModuleTest.cpp =================================================================== --- clang-tools-extra/trunk/unittests/clang-tidy/MiscModuleTest.cpp +++ clang-tools-extra/trunk/unittests/clang-tidy/MiscModuleTest.cpp @@ -23,10 +23,10 @@ TEST(ArgumentCommentCheckTest, OtherEditDistanceAboveThreshold) { EXPECT_EQ("void f(int xxx, int yyy); void g() { f(/*xxx=*/0, 0); }", runCheckOnCode( - "void f(int xxx, int yyy); void g() { f(/*Xxx=*/0, 0); }")); + "void f(int xxx, int yyy); void g() { f(/*Zxx=*/0, 0); }")); EXPECT_EQ("struct C { C(int xxx, int yyy); }; C c(/*xxx=*/0, 0);", runCheckOnCode( - "struct C { C(int xxx, int yyy); }; C c(/*Xxx=*/0, 0);")); + "struct C { C(int xxx, int yyy); }; C c(/*Zxx=*/0, 0);")); } TEST(ArgumentCommentCheckTest, OtherEditDistanceBelowThreshold) {