Index: clang-tools-extra/trunk/clang-tidy/bugprone/ArgumentCommentCheck.h =================================================================== --- clang-tools-extra/trunk/clang-tidy/bugprone/ArgumentCommentCheck.h +++ clang-tools-extra/trunk/clang-tidy/bugprone/ArgumentCommentCheck.h @@ -0,0 +1,55 @@ +//===--- ArgumentCommentCheck.h - clang-tidy --------------------*- C++ -*-===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// + +#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_ARGUMENTCOMMENTCHECK_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_ARGUMENTCOMMENTCHECK_H + +#include "../ClangTidy.h" +#include "llvm/Support/Regex.h" + +namespace clang { +namespace tidy { +namespace bugprone { + +/// Checks that argument comments match parameter names. +/// +/// 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' +/// \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; + llvm::Regex IdentRE; + + void checkCallArgs(ASTContext *Ctx, const FunctionDecl *Callee, + SourceLocation ArgBeginLoc, + llvm::ArrayRef Args); +}; + +} // namespace bugprone +} // namespace tidy +} // namespace clang + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_ARGUMENTCOMMENTCHECK_H Index: clang-tools-extra/trunk/clang-tidy/bugprone/ArgumentCommentCheck.cpp =================================================================== --- clang-tools-extra/trunk/clang-tidy/bugprone/ArgumentCommentCheck.cpp +++ clang-tools-extra/trunk/clang-tidy/bugprone/ArgumentCommentCheck.cpp @@ -0,0 +1,308 @@ +//===--- ArgumentCommentCheck.cpp - clang-tidy ----------------------------===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// + +#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), + 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( + hasAnyName("NewCallback", "NewPermanentCallback"))))) + .bind("expr"), + this); + Finder->addMatcher(cxxConstructExpr().bind("expr"), this); +} + +static std::vector> +getCommentsInRange(ASTContext *Ctx, CharSourceRange Range) { + std::vector> Comments; + auto &SM = Ctx->getSourceManager(); + std::pair BeginLoc = SM.getDecomposedLoc(Range.getBegin()), + EndLoc = SM.getDecomposedLoc(Range.getEnd()); + + if (BeginLoc.first != EndLoc.first) + return Comments; + + bool Invalid = false; + StringRef Buffer = SM.getBufferData(BeginLoc.first, &Invalid); + if (Invalid) + return Comments; + + const char *StrData = Buffer.data() + BeginLoc.second; + + Lexer TheLexer(SM.getLocForStartOfFile(BeginLoc.first), Ctx->getLangOpts(), + Buffer.begin(), StrData, Buffer.end()); + TheLexer.SetCommentRetentionState(true); + + while (true) { + Token Tok; + if (TheLexer.LexFromRawLexer(Tok)) + break; + if (Tok.getLocation() == Range.getEnd() || Tok.is(tok::eof)) + break; + + if (Tok.is(tok::comment)) { + std::pair CommentLoc = + SM.getDecomposedLoc(Tok.getLocation()); + assert(CommentLoc.first == BeginLoc.first); + Comments.emplace_back( + Tok.getLocation(), + StringRef(Buffer.begin() + CommentLoc.second, Tok.getLength())); + } else { + // Clear comments found before the different token, e.g. comma. + Comments.clear(); + } + } + + return Comments; +} + +static std::vector> +getCommentsBeforeLoc(ASTContext *Ctx, SourceLocation Loc) { + std::vector> Comments; + while (Loc.isValid()) { + clang::Token Tok = + utils::lexer::getPreviousToken(*Ctx, Loc, /*SkipComments=*/false); + if (Tok.isNot(tok::comment)) + break; + Loc = Tok.getLocation(); + Comments.emplace_back( + Loc, + Lexer::getSourceText(CharSourceRange::getCharRange( + Loc, Loc.getLocWithOffset(Tok.getLength())), + Ctx->getSourceManager(), Ctx->getLangOpts())); + } + return Comments; +} + +static bool 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 = ArgNameLower.edit_distance( + Params[ArgIndex]->getIdentifier()->getName().lower(), + /*AllowReplacements=*/true, UpperBound); + if (ThisED >= UpperBound) + return false; + + 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. + unsigned OtherED = ArgNameLower.edit_distance(II->getName().lower(), + /*AllowReplacements=*/true, + ThisED + Threshold); + if (OtherED < ThisED + Threshold) + return false; + } + + 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; +} + +static bool looksLikeExpectMethod(const CXXMethodDecl *Expect) { + return Expect != nullptr && Expect->getLocation().isMacroID() && + Expect->getNameInfo().getName().isIdentifier() && + Expect->getName().startswith("gmock_"); +} +static bool areMockAndExpectMethods(const CXXMethodDecl *Mock, + const CXXMethodDecl *Expect) { + assert(looksLikeExpectMethod(Expect)); + return Mock != nullptr && Mock->getNextDeclInContext() == Expect && + Mock->getNumParams() == Expect->getNumParams() && + Mock->getLocation().isMacroID() && + Mock->getNameInfo().getName().isIdentifier() && + Mock->getName() == Expect->getName().substr(strlen("gmock_")); +} + +// This uses implementation details of MOCK_METHODx_ macros: for each mocked +// method M it defines M() with appropriate signature and a method used to set +// up expectations - gmock_M() - with each argument's type changed the +// corresponding matcher. This function returns M when given either M or +// gmock_M. +static const CXXMethodDecl *findMockedMethod(const CXXMethodDecl *Method) { + if (looksLikeExpectMethod(Method)) { + const DeclContext *Ctx = Method->getDeclContext(); + if (Ctx == nullptr || !Ctx->isRecord()) + return nullptr; + for (const auto *D : Ctx->decls()) { + if (D->getNextDeclInContext() == Method) { + const auto *Previous = dyn_cast(D); + return areMockAndExpectMethods(Previous, Method) ? Previous : nullptr; + } + } + return nullptr; + } + if (const auto *Next = dyn_cast_or_null( + 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(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; +} + +void ArgumentCommentCheck::checkCallArgs(ASTContext *Ctx, + const FunctionDecl *OriginalCallee, + SourceLocation ArgBeginLoc, + llvm::ArrayRef Args) { + const FunctionDecl *Callee = resolveMocks(OriginalCallee); + if (!Callee) + return; + + Callee = Callee->getFirstDecl(); + unsigned NumArgs = std::min(Args.size(), Callee->getNumParams()); + if (NumArgs == 0) + return; + + 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]->getLocStart()); + ArgBeginLoc = Args[I]->getLocEnd(); + + std::vector> Comments; + if (BeforeArgument.isValid()) { + Comments = getCommentsInRange(Ctx, BeforeArgument); + } else { + // Fall back to parsing back from the start of the argument. + CharSourceRange ArgsRange = makeFileCharRange( + Args[I]->getLocStart(), Args[NumArgs - 1]->getLocEnd()); + Comments = getCommentsBeforeLoc(Ctx, ArgsRange.getBegin()); + } + + for (auto Comment : Comments) { + llvm::SmallVector Matches; + if (IdentRE.match(Comment.second, &Matches) && + !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)) { + Diag << FixItHint::CreateReplacement( + Comment.first, (Matches[1] + II->getName() + Matches[3]).str()); + } + } + diag(PVD->getLocation(), "%0 declared here", DiagnosticIDs::Note) << II; + if (OriginalCallee != Callee) { + diag(OriginalCallee->getLocation(), + "actual callee (%0) is declared here", DiagnosticIDs::Note) + << OriginalCallee; + } + } + } + } +} + +void ArgumentCommentCheck::check(const MatchFinder::MatchResult &Result) { + const auto *E = Result.Nodes.getNodeAs("expr"); + if (const auto *Call = dyn_cast(E)) { + const FunctionDecl *Callee = Call->getDirectCallee(); + if (!Callee) + return; + + checkCallArgs(Result.Context, Callee, Call->getCallee()->getLocEnd(), + llvm::makeArrayRef(Call->getArgs(), Call->getNumArgs())); + } else { + const auto *Construct = cast(E); + if (Construct->getNumArgs() == 1 && + Construct->getArg(0)->getSourceRange() == Construct->getSourceRange()) { + // Ignore implicit construction. + return; + } + checkCallArgs( + Result.Context, Construct->getConstructor(), + Construct->getParenOrBraceRange().getBegin(), + llvm::makeArrayRef(Construct->getArgs(), Construct->getNumArgs())); + } +} + +} // namespace bugprone +} // namespace tidy +} // namespace clang Index: clang-tools-extra/trunk/clang-tidy/bugprone/BugproneTidyModule.cpp =================================================================== --- clang-tools-extra/trunk/clang-tidy/bugprone/BugproneTidyModule.cpp +++ clang-tools-extra/trunk/clang-tidy/bugprone/BugproneTidyModule.cpp @@ -10,6 +10,7 @@ #include "../ClangTidy.h" #include "../ClangTidyModule.h" #include "../ClangTidyModuleRegistry.h" +#include "ArgumentCommentCheck.h" #include "CopyConstructorInitCheck.h" #include "IntegerDivisionCheck.h" #include "MisplacedOperatorInStrlenInAllocCheck.h" @@ -24,6 +25,8 @@ class BugproneModule : public ClangTidyModule { public: void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override { + CheckFactories.registerCheck( + "bugprone-argument-comment"); CheckFactories.registerCheck( "bugprone-copy-constructor-init"); CheckFactories.registerCheck( Index: clang-tools-extra/trunk/clang-tidy/bugprone/CMakeLists.txt =================================================================== --- clang-tools-extra/trunk/clang-tidy/bugprone/CMakeLists.txt +++ clang-tools-extra/trunk/clang-tidy/bugprone/CMakeLists.txt @@ -1,6 +1,7 @@ set(LLVM_LINK_COMPONENTS support) add_clang_library(clangTidyBugproneModule + ArgumentCommentCheck.cpp BugproneTidyModule.cpp CopyConstructorInitCheck.cpp IntegerDivisionCheck.cpp 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 @@ -1,55 +0,0 @@ -//===--- ArgumentCommentCheck.h - clang-tidy --------------------*- C++ -*-===// -// -// The LLVM Compiler Infrastructure -// -// This file is distributed under the University of Illinois Open Source -// License. See LICENSE.TXT for details. -// -//===----------------------------------------------------------------------===// - -#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_ARGUMENTCOMMENTCHECK_H -#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_ARGUMENTCOMMENTCHECK_H - -#include "../ClangTidy.h" -#include "llvm/Support/Regex.h" - -namespace clang { -namespace tidy { -namespace misc { - -/// Checks that argument comments match parameter names. -/// -/// 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' -/// \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; - llvm::Regex IdentRE; - - void checkCallArgs(ASTContext *Ctx, const FunctionDecl *Callee, - SourceLocation ArgBeginLoc, - llvm::ArrayRef Args); -}; - -} // namespace misc -} // namespace tidy -} // namespace clang - -#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_ARGUMENTCOMMENTCHECK_H 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 @@ -1,308 +0,0 @@ -//===--- ArgumentCommentCheck.cpp - clang-tidy ----------------------------===// -// -// The LLVM Compiler Infrastructure -// -// This file is distributed under the University of Illinois Open Source -// License. See LICENSE.TXT for details. -// -//===----------------------------------------------------------------------===// - -#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 misc { - -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( - hasAnyName("NewCallback", "NewPermanentCallback"))))) - .bind("expr"), - this); - Finder->addMatcher(cxxConstructExpr().bind("expr"), this); -} - -static std::vector> -getCommentsInRange(ASTContext *Ctx, CharSourceRange Range) { - std::vector> Comments; - auto &SM = Ctx->getSourceManager(); - std::pair BeginLoc = SM.getDecomposedLoc(Range.getBegin()), - EndLoc = SM.getDecomposedLoc(Range.getEnd()); - - if (BeginLoc.first != EndLoc.first) - return Comments; - - bool Invalid = false; - StringRef Buffer = SM.getBufferData(BeginLoc.first, &Invalid); - if (Invalid) - return Comments; - - const char *StrData = Buffer.data() + BeginLoc.second; - - Lexer TheLexer(SM.getLocForStartOfFile(BeginLoc.first), Ctx->getLangOpts(), - Buffer.begin(), StrData, Buffer.end()); - TheLexer.SetCommentRetentionState(true); - - while (true) { - Token Tok; - if (TheLexer.LexFromRawLexer(Tok)) - break; - if (Tok.getLocation() == Range.getEnd() || Tok.is(tok::eof)) - break; - - if (Tok.is(tok::comment)) { - std::pair CommentLoc = - SM.getDecomposedLoc(Tok.getLocation()); - assert(CommentLoc.first == BeginLoc.first); - Comments.emplace_back( - Tok.getLocation(), - StringRef(Buffer.begin() + CommentLoc.second, Tok.getLength())); - } else { - // Clear comments found before the different token, e.g. comma. - Comments.clear(); - } - } - - return Comments; -} - -static std::vector> -getCommentsBeforeLoc(ASTContext *Ctx, SourceLocation Loc) { - std::vector> Comments; - while (Loc.isValid()) { - clang::Token Tok = - utils::lexer::getPreviousToken(*Ctx, Loc, /*SkipComments=*/false); - if (Tok.isNot(tok::comment)) - break; - Loc = Tok.getLocation(); - Comments.emplace_back( - Loc, - Lexer::getSourceText(CharSourceRange::getCharRange( - Loc, Loc.getLocWithOffset(Tok.getLength())), - Ctx->getSourceManager(), Ctx->getLangOpts())); - } - return Comments; -} - -static bool 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 = ArgNameLower.edit_distance( - Params[ArgIndex]->getIdentifier()->getName().lower(), - /*AllowReplacements=*/true, UpperBound); - if (ThisED >= UpperBound) - return false; - - 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. - unsigned OtherED = ArgNameLower.edit_distance(II->getName().lower(), - /*AllowReplacements=*/true, - ThisED + Threshold); - if (OtherED < ThisED + Threshold) - return false; - } - - 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; -} - -static bool looksLikeExpectMethod(const CXXMethodDecl *Expect) { - return Expect != nullptr && Expect->getLocation().isMacroID() && - Expect->getNameInfo().getName().isIdentifier() && - Expect->getName().startswith("gmock_"); -} -static bool areMockAndExpectMethods(const CXXMethodDecl *Mock, - const CXXMethodDecl *Expect) { - assert(looksLikeExpectMethod(Expect)); - return Mock != nullptr && Mock->getNextDeclInContext() == Expect && - Mock->getNumParams() == Expect->getNumParams() && - Mock->getLocation().isMacroID() && - Mock->getNameInfo().getName().isIdentifier() && - Mock->getName() == Expect->getName().substr(strlen("gmock_")); -} - -// This uses implementation details of MOCK_METHODx_ macros: for each mocked -// method M it defines M() with appropriate signature and a method used to set -// up expectations - gmock_M() - with each argument's type changed the -// corresponding matcher. This function returns M when given either M or -// gmock_M. -static const CXXMethodDecl *findMockedMethod(const CXXMethodDecl *Method) { - if (looksLikeExpectMethod(Method)) { - const DeclContext *Ctx = Method->getDeclContext(); - if (Ctx == nullptr || !Ctx->isRecord()) - return nullptr; - for (const auto *D : Ctx->decls()) { - if (D->getNextDeclInContext() == Method) { - const auto *Previous = dyn_cast(D); - return areMockAndExpectMethods(Previous, Method) ? Previous : nullptr; - } - } - return nullptr; - } - if (const auto *Next = dyn_cast_or_null( - 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(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; -} - -void ArgumentCommentCheck::checkCallArgs(ASTContext *Ctx, - const FunctionDecl *OriginalCallee, - SourceLocation ArgBeginLoc, - llvm::ArrayRef Args) { - const FunctionDecl *Callee = resolveMocks(OriginalCallee); - if (!Callee) - return; - - Callee = Callee->getFirstDecl(); - unsigned NumArgs = std::min(Args.size(), Callee->getNumParams()); - if (NumArgs == 0) - return; - - 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]->getLocStart()); - ArgBeginLoc = Args[I]->getLocEnd(); - - std::vector> Comments; - if (BeforeArgument.isValid()) { - Comments = getCommentsInRange(Ctx, BeforeArgument); - } else { - // Fall back to parsing back from the start of the argument. - CharSourceRange ArgsRange = makeFileCharRange( - Args[I]->getLocStart(), Args[NumArgs - 1]->getLocEnd()); - Comments = getCommentsBeforeLoc(Ctx, ArgsRange.getBegin()); - } - - for (auto Comment : Comments) { - llvm::SmallVector Matches; - if (IdentRE.match(Comment.second, &Matches) && - !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)) { - Diag << FixItHint::CreateReplacement( - Comment.first, (Matches[1] + II->getName() + Matches[3]).str()); - } - } - diag(PVD->getLocation(), "%0 declared here", DiagnosticIDs::Note) << II; - if (OriginalCallee != Callee) { - diag(OriginalCallee->getLocation(), - "actual callee (%0) is declared here", DiagnosticIDs::Note) - << OriginalCallee; - } - } - } - } -} - -void ArgumentCommentCheck::check(const MatchFinder::MatchResult &Result) { - const auto *E = Result.Nodes.getNodeAs("expr"); - if (const auto *Call = dyn_cast(E)) { - const FunctionDecl *Callee = Call->getDirectCallee(); - if (!Callee) - return; - - checkCallArgs(Result.Context, Callee, Call->getCallee()->getLocEnd(), - llvm::makeArrayRef(Call->getArgs(), Call->getNumArgs())); - } else { - const auto *Construct = cast(E); - if (Construct->getNumArgs() == 1 && - Construct->getArg(0)->getSourceRange() == Construct->getSourceRange()) { - // Ignore implicit construction. - return; - } - checkCallArgs( - Result.Context, Construct->getConstructor(), - Construct->getParenOrBraceRange().getBegin(), - llvm::makeArrayRef(Construct->getArgs(), Construct->getNumArgs())); - } -} - -} // namespace misc -} // namespace tidy -} // namespace clang Index: clang-tools-extra/trunk/clang-tidy/misc/CMakeLists.txt =================================================================== --- clang-tools-extra/trunk/clang-tidy/misc/CMakeLists.txt +++ clang-tools-extra/trunk/clang-tidy/misc/CMakeLists.txt @@ -1,7 +1,6 @@ set(LLVM_LINK_COMPONENTS support) add_clang_library(clangTidyMiscModule - ArgumentCommentCheck.cpp AssertSideEffectCheck.cpp ForwardingReferenceOverloadCheck.cpp LambdaFunctionNameCheck.cpp Index: clang-tools-extra/trunk/clang-tidy/misc/MiscTidyModule.cpp =================================================================== --- clang-tools-extra/trunk/clang-tidy/misc/MiscTidyModule.cpp +++ clang-tools-extra/trunk/clang-tidy/misc/MiscTidyModule.cpp @@ -10,7 +10,6 @@ #include "../ClangTidy.h" #include "../ClangTidyModule.h" #include "../ClangTidyModuleRegistry.h" -#include "ArgumentCommentCheck.h" #include "AssertSideEffectCheck.h" #include "BoolPointerImplicitConversionCheck.h" #include "DanglingHandleCheck.h" @@ -63,7 +62,6 @@ class MiscModule : public ClangTidyModule { public: void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override { - CheckFactories.registerCheck("misc-argument-comment"); CheckFactories.registerCheck( "misc-assert-side-effect"); CheckFactories.registerCheck( Index: clang-tools-extra/trunk/docs/ReleaseNotes.rst =================================================================== --- clang-tools-extra/trunk/docs/ReleaseNotes.rst +++ clang-tools-extra/trunk/docs/ReleaseNotes.rst @@ -57,6 +57,9 @@ Improvements to clang-tidy -------------------------- +- The 'misc-argument-comment' check was renamed to `bugprone-argument-comment + `_ + - The 'misc-string-constructor' check was renamed to `bugprone-string-constructor `_ Index: clang-tools-extra/trunk/docs/clang-tidy/checks/bugprone-argument-comment.rst =================================================================== --- clang-tools-extra/trunk/docs/clang-tidy/checks/bugprone-argument-comment.rst +++ clang-tools-extra/trunk/docs/clang-tidy/checks/bugprone-argument-comment.rst @@ -0,0 +1,29 @@ +.. title:: clang-tidy - bugprone-argument-comment + +bugprone-argument-comment +========================= + +Checks that argument comments match parameter names. + +The check understands argument comments in the form ``/*parameter_name=*/`` +that are placed right before the argument. + +.. code-block:: c++ + + void f(bool foo); + + ... + + f(/*bar=*/true); + // warning: argument name 'bar' in comment does not match parameter name 'foo' + +The check tries to detect typos and suggest automated fixes for them. + +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. Index: clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst =================================================================== --- clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst +++ clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst @@ -17,6 +17,7 @@ android-cloexec-open android-cloexec-socket boost-use-to-string + bugprone-argument-comment bugprone-copy-constructor-init bugprone-integer-division bugprone-misplaced-operator-in-strlen-in-alloc @@ -105,7 +106,6 @@ llvm-include-order llvm-namespace-comment llvm-twine-local - misc-argument-comment misc-assert-side-effect misc-bool-pointer-implicit-conversion misc-dangling-handle 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 @@ -1,29 +0,0 @@ -.. title:: clang-tidy - misc-argument-comment - -misc-argument-comment -===================== - -Checks that argument comments match parameter names. - -The check understands argument comments in the form ``/*parameter_name=*/`` -that are placed right before the argument. - -.. code-block:: c++ - - void f(bool foo); - - ... - - f(/*bar=*/true); - // warning: argument name 'bar' in comment does not match parameter name 'foo' - -The check tries to detect typos and suggest automated fixes for them. - -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. Index: clang-tools-extra/trunk/test/clang-tidy/bugprone-argument-comment-gmock.cpp =================================================================== --- clang-tools-extra/trunk/test/clang-tidy/bugprone-argument-comment-gmock.cpp +++ clang-tools-extra/trunk/test/clang-tidy/bugprone-argument-comment-gmock.cpp @@ -0,0 +1,108 @@ +// RUN: %check_clang_tidy %s bugprone-argument-comment %t + +namespace testing { +namespace internal { + +template +struct Function; + +template +struct Function { + typedef R Result; +}; + +template +struct Function + : Function { + typedef A1 Argument1; +}; + +template +struct Function + : Function { + typedef A2 Argument2; +}; + +} // namespace internal + +template +class MockSpec { + public: + void f(); +}; + +template +class Matcher { + public: + explicit Matcher(); + Matcher(T value); +}; + +} // namespace testing + +#define GMOCK_RESULT_(tn, ...) \ + tn ::testing::internal::Function<__VA_ARGS__>::Result +#define GMOCK_ARG_(tn, N, ...) \ + tn ::testing::internal::Function<__VA_ARGS__>::Argument##N +#define GMOCK_MATCHER_(tn, N, ...) \ + const ::testing::Matcher& +#define GMOCK_METHOD2_(tn, constness, ct, Method, ...) \ + GMOCK_RESULT_(tn, __VA_ARGS__) \ + ct Method( \ + GMOCK_ARG_(tn, 1, __VA_ARGS__) gmock_a1, \ + GMOCK_ARG_(tn, 2, __VA_ARGS__) gmock_a2) constness; \ + ::testing::MockSpec<__VA_ARGS__> \ + gmock_##Method(GMOCK_MATCHER_(tn, 1, __VA_ARGS__) gmock_a1, \ + GMOCK_MATCHER_(tn, 2, __VA_ARGS__) gmock_a2) constness +#define MOCK_METHOD2(m, ...) GMOCK_METHOD2_(, , , m, __VA_ARGS__) +#define MOCK_CONST_METHOD2(m, ...) GMOCK_METHOD2_(, const, , m, __VA_ARGS__) +#define GMOCK_EXPECT_CALL_IMPL_(obj, call) \ + ((obj).gmock_##call).f() +#define EXPECT_CALL(obj, call) GMOCK_EXPECT_CALL_IMPL_(obj, call) + +class Base { + public: + virtual void Method(int param_one_base, int param_two_base); +}; +class Derived : public Base { + public: + virtual void Method(int param_one, int param_two); + virtual void Method2(int p_one, int p_two) const; +}; +class MockDerived : public Derived { + public: + MOCK_METHOD2(Method, void(int a, int b)); + MOCK_CONST_METHOD2(Method2, void(int c, int d)); +}; + +class MockStandalone { + public: + MOCK_METHOD2(Method, void(int aaa, int bbb)); +}; + +void test_gmock_expectations() { + MockDerived m; + EXPECT_CALL(m, Method(/*param_one=*/1, /*param_tw=*/2)); +// CHECK-MESSAGES: [[@LINE-1]]:42: warning: argument name 'param_tw' in comment does not match parameter name 'param_two' +// CHECK-FIXES: EXPECT_CALL(m, Method(/*param_one=*/1, /*param_two=*/2)); + EXPECT_CALL(m, Method2(/*p_on=*/3, /*p_two=*/4)); +// CHECK-MESSAGES: [[@LINE-1]]:26: warning: argument name 'p_on' in comment does not match parameter name 'p_one' +// CHECK-FIXES: EXPECT_CALL(m, Method2(/*p_one=*/3, /*p_two=*/4)); + + #define PARAM1 11 + #define PARAM2 22 + EXPECT_CALL(m, Method2(/*p_on1=*/PARAM1, /*p_tw2=*/PARAM2)); +// CHECK-MESSAGES: [[@LINE-1]]:26: warning: argument name 'p_on1' in comment does not match parameter name 'p_one' +// CHECK-MESSAGES: [[@LINE-2]]:44: warning: argument name 'p_tw2' in comment does not match parameter name 'p_two' +// CHECK-FIXES: EXPECT_CALL(m, Method2(/*p_one=*/PARAM1, /*p_two=*/PARAM2)); + + MockStandalone m2; + EXPECT_CALL(m2, Method(/*aaa=*/5, /*bbc=*/6)); +} + +void test_gmock_direct_calls() { + MockDerived m; + m.Method(/*param_one=*/1, /*param_tw=*/2); +// CHECK-MESSAGES: [[@LINE-1]]:29: warning: argument name 'param_tw' in comment does not match parameter name 'param_two' +// CHECK-FIXES: m.Method(/*param_one=*/1, /*param_two=*/2); +} Index: clang-tools-extra/trunk/test/clang-tidy/bugprone-argument-comment-strict.cpp =================================================================== --- clang-tools-extra/trunk/test/clang-tidy/bugprone-argument-comment-strict.cpp +++ clang-tools-extra/trunk/test/clang-tidy/bugprone-argument-comment-strict.cpp @@ -0,0 +1,19 @@ +// RUN: %check_clang_tidy %s bugprone-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/bugprone-argument-comment.cpp =================================================================== --- clang-tools-extra/trunk/test/clang-tidy/bugprone-argument-comment.cpp +++ clang-tools-extra/trunk/test/clang-tidy/bugprone-argument-comment.cpp @@ -0,0 +1,89 @@ +// RUN: %check_clang_tidy %s bugprone-argument-comment %t + +// FIXME: clang-tidy should provide a -verify mode to make writing these checks +// easier and more accurate. + +void ffff(int xxxx, int yyyy); + +void f(int x, int y); +void g() { + // CHECK-MESSAGES: [[@LINE+4]]:5: warning: argument name 'y' in comment does not match parameter name 'x' + // CHECK-MESSAGES: :[[@LINE-3]]:12: note: 'x' declared here + // CHECK-MESSAGES: [[@LINE+2]]:14: warning: argument name 'z' in comment does not match parameter name 'y' + // CHECK-MESSAGES: :[[@LINE-5]]:19: note: 'y' declared here + f(/*y=*/0, /*z=*/0); + // CHECK-FIXES: {{^}} f(/*y=*/0, /*z=*/0); + + f(/*x=*/1, /*y=*/1); + + ffff(0 /*aaaa=*/, /*bbbb*/ 0); // Unsupported formats. +} + +struct C { + C(int x, int y); +}; +C c(/*x=*/0, /*y=*/0); + +struct Closure {}; + +template +Closure *NewCallback(void (*f)(T1, T2), T1 arg1, T2 arg2) { return nullptr; } + +template +Closure *NewPermanentCallback(void (*f)(T1, T2), T1 arg1, T2 arg2) { return nullptr; } + +void h() { + (void)NewCallback(&ffff, /*xxxx=*/11, /*yyyy=*/22); + (void)NewPermanentCallback(&ffff, /*xxxx=*/11, /*yyyy=*/22); +} + +template +void variadic(Args&&... args); + +template +void variadic2(int zzz, Args&&... args); + +void templates() { + variadic(/*xxx=*/0, /*yyy=*/1); + 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); +} + +#define FALSE 0 +void qqq(bool aaa); +void f2() { qqq(/*bbb=*/FALSE); } +// CHECK-MESSAGES: [[@LINE-1]]:17: warning: argument name 'bbb' in comment does not match parameter name 'aaa' +// CHECK-FIXES: void f2() { qqq(/*bbb=*/FALSE); } + +void f3(bool _with_underscores_); +void ignores_underscores() { + f3(/*With_Underscores=*/false); +} + +namespace ThisEditDistanceAboveThreshold { +void f4(int xxx); +void g() { f4(/*xyz=*/0); } +// CHECK-MESSAGES: [[@LINE-1]]:15: warning: argument name 'xyz' in comment does not match parameter name 'xxx' +// CHECK-FIXES: void g() { f4(/*xyz=*/0); } +} + +namespace OtherEditDistanceAboveThreshold { +void f5(int xxx, int yyy); +void g() { f5(/*Zxx=*/0, 0); } +// CHECK-MESSAGES: [[@LINE-1]]:15: warning: argument name 'Zxx' in comment does not match parameter name 'xxx' +// CHECK-FIXES: void g() { f5(/*xxx=*/0, 0); } +struct C2 { + C2(int xxx, int yyy); +}; +C2 c2(/*Zxx=*/0, 0); +// CHECK-MESSAGES: [[@LINE-1]]:7: warning: argument name 'Zxx' in comment does not match parameter name 'xxx' +// CHECK-FIXES: C2 c2(/*xxx=*/0, 0); +} + +namespace OtherEditDistanceBelowThreshold { +void f6(int xxx, int yyy); +void g() { f6(/*xxy=*/0, 0); } +// CHECK-MESSAGES: [[@LINE-1]]:15: warning: argument name 'xxy' in comment does not match parameter name 'xxx' +// CHECK-FIXES: void g() { f6(/*xxy=*/0, 0); } +} Index: clang-tools-extra/trunk/test/clang-tidy/misc-argument-comment-gmock.cpp =================================================================== --- clang-tools-extra/trunk/test/clang-tidy/misc-argument-comment-gmock.cpp +++ clang-tools-extra/trunk/test/clang-tidy/misc-argument-comment-gmock.cpp @@ -1,108 +0,0 @@ -// RUN: %check_clang_tidy %s misc-argument-comment %t - -namespace testing { -namespace internal { - -template -struct Function; - -template -struct Function { - typedef R Result; -}; - -template -struct Function - : Function { - typedef A1 Argument1; -}; - -template -struct Function - : Function { - typedef A2 Argument2; -}; - -} // namespace internal - -template -class MockSpec { - public: - void f(); -}; - -template -class Matcher { - public: - explicit Matcher(); - Matcher(T value); -}; - -} // namespace testing - -#define GMOCK_RESULT_(tn, ...) \ - tn ::testing::internal::Function<__VA_ARGS__>::Result -#define GMOCK_ARG_(tn, N, ...) \ - tn ::testing::internal::Function<__VA_ARGS__>::Argument##N -#define GMOCK_MATCHER_(tn, N, ...) \ - const ::testing::Matcher& -#define GMOCK_METHOD2_(tn, constness, ct, Method, ...) \ - GMOCK_RESULT_(tn, __VA_ARGS__) \ - ct Method( \ - GMOCK_ARG_(tn, 1, __VA_ARGS__) gmock_a1, \ - GMOCK_ARG_(tn, 2, __VA_ARGS__) gmock_a2) constness; \ - ::testing::MockSpec<__VA_ARGS__> \ - gmock_##Method(GMOCK_MATCHER_(tn, 1, __VA_ARGS__) gmock_a1, \ - GMOCK_MATCHER_(tn, 2, __VA_ARGS__) gmock_a2) constness -#define MOCK_METHOD2(m, ...) GMOCK_METHOD2_(, , , m, __VA_ARGS__) -#define MOCK_CONST_METHOD2(m, ...) GMOCK_METHOD2_(, const, , m, __VA_ARGS__) -#define GMOCK_EXPECT_CALL_IMPL_(obj, call) \ - ((obj).gmock_##call).f() -#define EXPECT_CALL(obj, call) GMOCK_EXPECT_CALL_IMPL_(obj, call) - -class Base { - public: - virtual void Method(int param_one_base, int param_two_base); -}; -class Derived : public Base { - public: - virtual void Method(int param_one, int param_two); - virtual void Method2(int p_one, int p_two) const; -}; -class MockDerived : public Derived { - public: - MOCK_METHOD2(Method, void(int a, int b)); - MOCK_CONST_METHOD2(Method2, void(int c, int d)); -}; - -class MockStandalone { - public: - MOCK_METHOD2(Method, void(int aaa, int bbb)); -}; - -void test_gmock_expectations() { - MockDerived m; - EXPECT_CALL(m, Method(/*param_one=*/1, /*param_tw=*/2)); -// CHECK-MESSAGES: [[@LINE-1]]:42: warning: argument name 'param_tw' in comment does not match parameter name 'param_two' -// CHECK-FIXES: EXPECT_CALL(m, Method(/*param_one=*/1, /*param_two=*/2)); - EXPECT_CALL(m, Method2(/*p_on=*/3, /*p_two=*/4)); -// CHECK-MESSAGES: [[@LINE-1]]:26: warning: argument name 'p_on' in comment does not match parameter name 'p_one' -// CHECK-FIXES: EXPECT_CALL(m, Method2(/*p_one=*/3, /*p_two=*/4)); - - #define PARAM1 11 - #define PARAM2 22 - EXPECT_CALL(m, Method2(/*p_on1=*/PARAM1, /*p_tw2=*/PARAM2)); -// CHECK-MESSAGES: [[@LINE-1]]:26: warning: argument name 'p_on1' in comment does not match parameter name 'p_one' -// CHECK-MESSAGES: [[@LINE-2]]:44: warning: argument name 'p_tw2' in comment does not match parameter name 'p_two' -// CHECK-FIXES: EXPECT_CALL(m, Method2(/*p_one=*/PARAM1, /*p_two=*/PARAM2)); - - MockStandalone m2; - EXPECT_CALL(m2, Method(/*aaa=*/5, /*bbc=*/6)); -} - -void test_gmock_direct_calls() { - MockDerived m; - m.Method(/*param_one=*/1, /*param_tw=*/2); -// CHECK-MESSAGES: [[@LINE-1]]:29: warning: argument name 'param_tw' in comment does not match parameter name 'param_two' -// CHECK-FIXES: m.Method(/*param_one=*/1, /*param_two=*/2); -} 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 @@ -1,19 +0,0 @@ -// 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 @@ -1,55 +0,0 @@ -// RUN: %check_clang_tidy %s misc-argument-comment %t - -// FIXME: clang-tidy should provide a -verify mode to make writing these checks -// easier and more accurate. - -void ffff(int xxxx, int yyyy); - -void f(int x, int y); -void g() { - // CHECK-MESSAGES: [[@LINE+4]]:5: warning: argument name 'y' in comment does not match parameter name 'x' - // CHECK-MESSAGES: :[[@LINE-3]]:12: note: 'x' declared here - // CHECK-MESSAGES: [[@LINE+2]]:14: warning: argument name 'z' in comment does not match parameter name 'y' - // CHECK-MESSAGES: :[[@LINE-5]]:19: note: 'y' declared here - f(/*y=*/0, /*z=*/0); - // CHECK-FIXES: {{^}} f(/*y=*/0, /*z=*/0); - - ffff(0 /*aaaa=*/, /*bbbb*/ 0); // Unsupported formats. -} - -struct Closure {}; - -template -Closure *NewCallback(void (*f)(T1, T2), T1 arg1, T2 arg2) { return nullptr; } - -template -Closure *NewPermanentCallback(void (*f)(T1, T2), T1 arg1, T2 arg2) { return nullptr; } - -void h() { - (void)NewCallback(&ffff, /*xxxx=*/11, /*yyyy=*/22); - (void)NewPermanentCallback(&ffff, /*xxxx=*/11, /*yyyy=*/22); -} - -template -void variadic(Args&&... args); - -template -void variadic2(int zzz, Args&&... args); - -void templates() { - variadic(/*xxx=*/0, /*yyy=*/1); - 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); -} - -#define FALSE 0 -void qqq(bool aaa); -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/CMakeLists.txt =================================================================== --- clang-tools-extra/trunk/unittests/clang-tidy/CMakeLists.txt +++ clang-tools-extra/trunk/unittests/clang-tidy/CMakeLists.txt @@ -12,7 +12,6 @@ IncludeInserterTest.cpp GoogleModuleTest.cpp LLVMModuleTest.cpp - MiscModuleTest.cpp NamespaceAliaserTest.cpp ObjCModuleTest.cpp OverlappingReplacementsTest.cpp @@ -29,7 +28,6 @@ clangTidyAndroidModule clangTidyGoogleModule clangTidyLLVMModule - clangTidyMiscModule clangTidyObjCModule clangTidyReadabilityModule clangTidyUtils 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 @@ -1,39 +0,0 @@ -#include "ClangTidyTest.h" -#include "misc/ArgumentCommentCheck.h" -#include "gtest/gtest.h" - -namespace clang { -namespace tidy { -namespace test { - -using misc::ArgumentCommentCheck; - -TEST(ArgumentCommentCheckTest, CorrectComments) { - EXPECT_NO_CHANGES(ArgumentCommentCheck, - "void f(int x, int y); void g() { f(/*x=*/0, /*y=*/0); }"); - EXPECT_NO_CHANGES(ArgumentCommentCheck, - "struct C { C(int x, int y); }; C c(/*x=*/0, /*y=*/0);"); -} - -TEST(ArgumentCommentCheckTest, ThisEditDistanceAboveThreshold) { - EXPECT_NO_CHANGES(ArgumentCommentCheck, - "void f(int xxx); void g() { f(/*xyz=*/0); }"); -} - -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(/*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(/*Zxx=*/0, 0);")); -} - -TEST(ArgumentCommentCheckTest, OtherEditDistanceBelowThreshold) { - EXPECT_NO_CHANGES(ArgumentCommentCheck, - "void f(int xxx, int yyy); void g() { f(/*xxy=*/0, 0); }"); -} - -} // namespace test -} // namespace tidy -} // namespace clang