[clang-tools-extra] r294193 - [clang-tidy] misc-argument-comment support for gmock
Alexander Kornienko via cfe-commits
cfe-commits at lists.llvm.org
Mon Feb 6 07:47:17 PST 2017
Author: alexfh
Date: Mon Feb 6 09:47:17 2017
New Revision: 294193
URL: http://llvm.org/viewvc/llvm-project?rev=294193&view=rev
Log:
[clang-tidy] misc-argument-comment support for gmock
Now for real. The use case supported previously is used by approximately nobody.
What's needed is support for matching argument comments in EXPECT_xxx calls to
the names of parameters of the mocked methods.
Added:
clang-tools-extra/trunk/test/clang-tidy/misc-argument-comment-gmock.cpp
Modified:
clang-tools-extra/trunk/clang-tidy/misc/ArgumentCommentCheck.cpp
clang-tools-extra/trunk/clang-tidy/misc/ArgumentCommentCheck.h
clang-tools-extra/trunk/test/clang-tidy/misc-argument-comment.cpp
Modified: clang-tools-extra/trunk/clang-tidy/misc/ArgumentCommentCheck.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/misc/ArgumentCommentCheck.cpp?rev=294193&r1=294192&r2=294193&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/misc/ArgumentCommentCheck.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/misc/ArgumentCommentCheck.cpp Mon Feb 6 09:47:17 2017
@@ -12,6 +12,7 @@
#include "clang/ASTMatchers/ASTMatchFinder.h"
#include "clang/Lex/Lexer.h"
#include "clang/Lex/Token.h"
+#include "../utils/LexerUtils.h"
using namespace clang::ast_matchers;
@@ -86,8 +87,26 @@ getCommentsInRange(ASTContext *Ctx, Char
return Comments;
}
-bool ArgumentCommentCheck::isLikelyTypo(llvm::ArrayRef<ParmVarDecl *> Params,
- StringRef ArgName, unsigned ArgIndex) {
+static std::vector<std::pair<SourceLocation, StringRef>>
+getCommentsBeforeLoc(ASTContext *Ctx, SourceLocation Loc) {
+ std::vector<std::pair<SourceLocation, StringRef>> 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<ParmVarDecl *> Params,
+ StringRef ArgName, unsigned ArgIndex) {
std::string ArgNameLowerStr = ArgName.lower();
StringRef ArgNameLower = ArgNameLowerStr;
// The threshold is arbitrary.
@@ -128,72 +147,129 @@ static bool sameName(StringRef InComment
return InComment.compare_lower(InDecl) == 0;
}
+// 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 finds M by gmock_M.
+static const CXXMethodDecl *
+findMockedMethod(const CXXMethodDecl *ExpectMethod) {
+ if (!ExpectMethod->getNameInfo().getName().isIdentifier())
+ return nullptr;
+ StringRef Name = ExpectMethod->getName();
+ if (!Name.startswith("gmock_"))
+ return nullptr;
+ Name = Name.substr(strlen("gmock_"));
+
+ const DeclContext *Ctx = ExpectMethod->getDeclContext();
+ if (Ctx == nullptr || !Ctx->isRecord())
+ return nullptr;
+ for (const auto *D : Ctx->decls()) {
+ if (const auto *Method = dyn_cast<CXXMethodDecl>(D)) {
+ if (Method->getName() != Name)
+ continue;
+ // Sanity check the mocked method.
+ if (Method->getNextDeclInContext() == ExpectMethod &&
+ Method->getLocation().isMacroID() &&
+ Method->getNumParams() == ExpectMethod->getNumParams()) {
+ 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;
+}
+
void ArgumentCommentCheck::checkCallArgs(ASTContext *Ctx,
- const FunctionDecl *Callee,
+ const FunctionDecl *OriginalCallee,
SourceLocation ArgBeginLoc,
llvm::ArrayRef<const Expr *> Args) {
+ const FunctionDecl *Callee = resolveMocks(OriginalCallee);
+ if (!Callee)
+ return;
+
Callee = Callee->getFirstDecl();
- for (unsigned I = 0,
- E = std::min<unsigned>(Args.size(), Callee->getNumParams());
- I != E; ++I) {
+ unsigned NumArgs = std::min<unsigned>(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.
+ // 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 = CharSourceRange::getCharRange(
- I == 0 ? ArgBeginLoc : Args[I - 1]->getLocEnd(),
- Args[I]->getLocStart());
- BeforeArgument = Lexer::makeFileCharRange(
- BeforeArgument, Ctx->getSourceManager(), Ctx->getLangOpts());
+ CharSourceRange BeforeArgument =
+ makeFileCharRange(ArgBeginLoc, Args[I]->getLocStart());
+ ArgBeginLoc = Args[I]->getLocEnd();
+
+ 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(
+ Args[I]->getLocStart(), Args[NumArgs - 1]->getLocEnd());
+ Comments = getCommentsBeforeLoc(Ctx, ArgsRange.getBegin());
+ }
- for (auto Comment : getCommentsInRange(Ctx, BeforeArgument)) {
+ for (auto Comment : Comments) {
llvm::SmallVector<StringRef, 2> Matches;
- if (IdentRE.match(Comment.second, &Matches)) {
- 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)) {
- Diag << FixItHint::CreateReplacement(
- Comment.first,
- (Matches[1] + II->getName() + Matches[3]).str());
- }
+ 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;
+ }
+ diag(PVD->getLocation(), "%0 declared here", DiagnosticIDs::Note) << II;
+ if (OriginalCallee != Callee) {
+ diag(OriginalCallee->getLocation(),
+ "actual callee (%0) is declared here", DiagnosticIDs::Note)
+ << OriginalCallee;
}
}
}
}
}
-static const FunctionDecl *resolveMocks(const MatchFinder::MatchResult &Result,
- const FunctionDecl *Func) {
- if (auto *Method = dyn_cast<CXXMethodDecl>(Func)) {
- if (Method->getLocation().isMacroID() &&
- Lexer::getImmediateMacroName(Method->getLocation(),
- *Result.SourceManager,
- Result.Context->getLangOpts())
- .contains("MOCK_METHOD") &&
- Method->size_overridden_methods() != 0) {
- Func = *Method->begin_overridden_methods();
- }
- }
- return Func;
-}
-
void ArgumentCommentCheck::check(const MatchFinder::MatchResult &Result) {
const auto *E = Result.Nodes.getNodeAs<Expr>("expr");
if (const auto *Call = dyn_cast<CallExpr>(E)) {
@@ -201,12 +277,15 @@ void ArgumentCommentCheck::check(const M
if (!Callee)
return;
- Callee = resolveMocks(Result, Callee);
-
checkCallArgs(Result.Context, Callee, Call->getCallee()->getLocEnd(),
llvm::makeArrayRef(Call->getArgs(), Call->getNumArgs()));
} else {
const auto *Construct = cast<CXXConstructExpr>(E);
+ if (Construct->getNumArgs() == 1 &&
+ Construct->getArg(0)->getSourceRange() == Construct->getSourceRange()) {
+ // Ignore implicit construction.
+ return;
+ }
checkCallArgs(
Result.Context, Construct->getConstructor(),
Construct->getParenOrBraceRange().getBegin(),
Modified: clang-tools-extra/trunk/clang-tidy/misc/ArgumentCommentCheck.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/misc/ArgumentCommentCheck.h?rev=294193&r1=294192&r2=294193&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/misc/ArgumentCommentCheck.h (original)
+++ clang-tools-extra/trunk/clang-tidy/misc/ArgumentCommentCheck.h Mon Feb 6 09:47:17 2017
@@ -43,8 +43,6 @@ private:
const bool StrictMode;
llvm::Regex IdentRE;
- bool isLikelyTypo(llvm::ArrayRef<ParmVarDecl *> Params, StringRef ArgName,
- unsigned ArgIndex);
void checkCallArgs(ASTContext *Ctx, const FunctionDecl *Callee,
SourceLocation ArgBeginLoc,
llvm::ArrayRef<const Expr *> Args);
Added: clang-tools-extra/trunk/test/clang-tidy/misc-argument-comment-gmock.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/misc-argument-comment-gmock.cpp?rev=294193&view=auto
==============================================================================
--- clang-tools-extra/trunk/test/clang-tidy/misc-argument-comment-gmock.cpp (added)
+++ clang-tools-extra/trunk/test/clang-tidy/misc-argument-comment-gmock.cpp Mon Feb 6 09:47:17 2017
@@ -0,0 +1,101 @@
+// RUN: %check_clang_tidy %s misc-argument-comment %t
+
+namespace testing {
+namespace internal {
+
+template <typename F>
+struct Function;
+
+template <typename R>
+struct Function<R()> {
+ typedef R Result;
+};
+
+template <typename R, typename A1>
+struct Function<R(A1)>
+ : Function<R()> {
+ typedef A1 Argument1;
+};
+
+template <typename R, typename A1, typename A2>
+struct Function<R(A1, A2)>
+ : Function<R(A1)> {
+ typedef A2 Argument2;
+};
+
+} // namespace internal
+
+template <typename F>
+class MockSpec {
+ public:
+ void f();
+};
+
+template <typename T>
+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<GMOCK_ARG_(tn, N, __VA_ARGS__)>&
+#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() {
+ 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));
+}
Modified: clang-tools-extra/trunk/test/clang-tidy/misc-argument-comment.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/misc-argument-comment.cpp?rev=294193&r1=294192&r2=294193&view=diff
==============================================================================
--- clang-tools-extra/trunk/test/clang-tidy/misc-argument-comment.cpp (original)
+++ clang-tools-extra/trunk/test/clang-tidy/misc-argument-comment.cpp Mon Feb 6 09:47:17 2017
@@ -53,59 +53,3 @@ void f(bool _with_underscores_);
void ignores_underscores() {
f(/*With_Underscores=*/false);
}
-
-// gmock
-namespace testing {
-namespace internal {
-
-template <typename F>
-struct Function;
-
-template <typename R>
-struct Function<R()> {
- typedef R Result;
-};
-
-template <typename R, typename A1>
-struct Function<R(A1)>
- : Function<R()> {
- typedef A1 Argument1;
-};
-
-template <typename R, typename A1, typename A2>
-struct Function<R(A1, A2)>
- : Function<R(A1)> {
- typedef A2 Argument2;
-};
-} // namespace internal
-} // 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_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
-#define MOCK_METHOD2(m, ...) GMOCK_METHOD2_(, , , m, __VA_ARGS__)
-
-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);
-};
-class MockDerived : public Derived {
- public:
- MOCK_METHOD2(Method, void(int, int));
-};
-
-void test_gmock() {
- 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);
-}
More information about the cfe-commits
mailing list