1 //===--- ArgumentCommentCheck.cpp - clang-tidy ----------------------------===//
2 //
3 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
4 // See https://llvm.org/LICENSE.txt for license information.
5 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
6 //
7 //===----------------------------------------------------------------------===//
8
9 #include "ArgumentCommentCheck.h"
10 #include "clang/AST/ASTContext.h"
11 #include "clang/ASTMatchers/ASTMatchFinder.h"
12 #include "clang/Lex/Lexer.h"
13 #include "clang/Lex/Token.h"
14
15 #include "../utils/LexerUtils.h"
16
17 using namespace clang::ast_matchers;
18
19 namespace clang {
20 namespace tidy {
21 namespace bugprone {
22 namespace {
AST_MATCHER(Decl,isFromStdNamespace)23 AST_MATCHER(Decl, isFromStdNamespace) {
24 if (const auto *D = Node.getDeclContext()->getEnclosingNamespaceContext())
25 return D->isStdNamespace();
26 return false;
27 }
28 } // namespace
29
ArgumentCommentCheck(StringRef Name,ClangTidyContext * Context)30 ArgumentCommentCheck::ArgumentCommentCheck(StringRef Name,
31 ClangTidyContext *Context)
32 : ClangTidyCheck(Name, Context),
33 StrictMode(Options.getLocalOrGlobal("StrictMode", false)),
34 IgnoreSingleArgument(Options.get("IgnoreSingleArgument", false)),
35 CommentBoolLiterals(Options.get("CommentBoolLiterals", false)),
36 CommentIntegerLiterals(Options.get("CommentIntegerLiterals", false)),
37 CommentFloatLiterals(Options.get("CommentFloatLiterals", false)),
38 CommentStringLiterals(Options.get("CommentStringLiterals", false)),
39 CommentUserDefinedLiterals(
40 Options.get("CommentUserDefinedLiterals", false)),
41 CommentCharacterLiterals(Options.get("CommentCharacterLiterals", false)),
42 CommentNullPtrs(Options.get("CommentNullPtrs", false)),
43 IdentRE("^(/\\* *)([_A-Za-z][_A-Za-z0-9]*)( *= *\\*/)$") {}
44
storeOptions(ClangTidyOptions::OptionMap & Opts)45 void ArgumentCommentCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
46 Options.store(Opts, "StrictMode", StrictMode);
47 Options.store(Opts, "IgnoreSingleArgument", IgnoreSingleArgument);
48 Options.store(Opts, "CommentBoolLiterals", CommentBoolLiterals);
49 Options.store(Opts, "CommentIntegerLiterals", CommentIntegerLiterals);
50 Options.store(Opts, "CommentFloatLiterals", CommentFloatLiterals);
51 Options.store(Opts, "CommentStringLiterals", CommentStringLiterals);
52 Options.store(Opts, "CommentUserDefinedLiterals", CommentUserDefinedLiterals);
53 Options.store(Opts, "CommentCharacterLiterals", CommentCharacterLiterals);
54 Options.store(Opts, "CommentNullPtrs", CommentNullPtrs);
55 }
56
registerMatchers(MatchFinder * Finder)57 void ArgumentCommentCheck::registerMatchers(MatchFinder *Finder) {
58 Finder->addMatcher(
59 callExpr(unless(cxxOperatorCallExpr()),
60 // NewCallback's arguments relate to the pointed function,
61 // don't check them against NewCallback's parameter names.
62 // FIXME: Make this configurable.
63 unless(hasDeclaration(functionDecl(
64 hasAnyName("NewCallback", "NewPermanentCallback")))),
65 // Ignore APIs from the standard library, since their names are
66 // not specified by the standard, and standard library
67 // implementations in practice have to use reserved names to
68 // avoid conflicts with same-named macros.
69 unless(hasDeclaration(isFromStdNamespace())))
70 .bind("expr"),
71 this);
72 Finder->addMatcher(
73 cxxConstructExpr(unless(hasDeclaration(isFromStdNamespace())))
74 .bind("expr"),
75 this);
76 }
77
78 static std::vector<std::pair<SourceLocation, StringRef>>
getCommentsInRange(ASTContext * Ctx,CharSourceRange Range)79 getCommentsInRange(ASTContext *Ctx, CharSourceRange Range) {
80 std::vector<std::pair<SourceLocation, StringRef>> Comments;
81 auto &SM = Ctx->getSourceManager();
82 std::pair<FileID, unsigned> BeginLoc = SM.getDecomposedLoc(Range.getBegin()),
83 EndLoc = SM.getDecomposedLoc(Range.getEnd());
84
85 if (BeginLoc.first != EndLoc.first)
86 return Comments;
87
88 bool Invalid = false;
89 StringRef Buffer = SM.getBufferData(BeginLoc.first, &Invalid);
90 if (Invalid)
91 return Comments;
92
93 const char *StrData = Buffer.data() + BeginLoc.second;
94
95 Lexer TheLexer(SM.getLocForStartOfFile(BeginLoc.first), Ctx->getLangOpts(),
96 Buffer.begin(), StrData, Buffer.end());
97 TheLexer.SetCommentRetentionState(true);
98
99 while (true) {
100 Token Tok;
101 if (TheLexer.LexFromRawLexer(Tok))
102 break;
103 if (Tok.getLocation() == Range.getEnd() || Tok.is(tok::eof))
104 break;
105
106 if (Tok.is(tok::comment)) {
107 std::pair<FileID, unsigned> CommentLoc =
108 SM.getDecomposedLoc(Tok.getLocation());
109 assert(CommentLoc.first == BeginLoc.first);
110 Comments.emplace_back(
111 Tok.getLocation(),
112 StringRef(Buffer.begin() + CommentLoc.second, Tok.getLength()));
113 } else {
114 // Clear comments found before the different token, e.g. comma.
115 Comments.clear();
116 }
117 }
118
119 return Comments;
120 }
121
122 static std::vector<std::pair<SourceLocation, StringRef>>
getCommentsBeforeLoc(ASTContext * Ctx,SourceLocation Loc)123 getCommentsBeforeLoc(ASTContext *Ctx, SourceLocation Loc) {
124 std::vector<std::pair<SourceLocation, StringRef>> Comments;
125 while (Loc.isValid()) {
126 clang::Token Tok = utils::lexer::getPreviousToken(
127 Loc, Ctx->getSourceManager(), Ctx->getLangOpts(),
128 /*SkipComments=*/false);
129 if (Tok.isNot(tok::comment))
130 break;
131 Loc = Tok.getLocation();
132 Comments.emplace_back(
133 Loc,
134 Lexer::getSourceText(CharSourceRange::getCharRange(
135 Loc, Loc.getLocWithOffset(Tok.getLength())),
136 Ctx->getSourceManager(), Ctx->getLangOpts()));
137 }
138 return Comments;
139 }
140
isLikelyTypo(llvm::ArrayRef<ParmVarDecl * > Params,StringRef ArgName,unsigned ArgIndex)141 static bool isLikelyTypo(llvm::ArrayRef<ParmVarDecl *> Params,
142 StringRef ArgName, unsigned ArgIndex) {
143 std::string ArgNameLowerStr = ArgName.lower();
144 StringRef ArgNameLower = ArgNameLowerStr;
145 // The threshold is arbitrary.
146 unsigned UpperBound = (ArgName.size() + 2) / 3 + 1;
147 unsigned ThisED = ArgNameLower.edit_distance(
148 Params[ArgIndex]->getIdentifier()->getName().lower(),
149 /*AllowReplacements=*/true, UpperBound);
150 if (ThisED >= UpperBound)
151 return false;
152
153 for (unsigned I = 0, E = Params.size(); I != E; ++I) {
154 if (I == ArgIndex)
155 continue;
156 IdentifierInfo *II = Params[I]->getIdentifier();
157 if (!II)
158 continue;
159
160 const unsigned Threshold = 2;
161 // Other parameters must be an edit distance at least Threshold more away
162 // from this parameter. This gives us greater confidence that this is a
163 // typo of this parameter and not one with a similar name.
164 unsigned OtherED = ArgNameLower.edit_distance(II->getName().lower(),
165 /*AllowReplacements=*/true,
166 ThisED + Threshold);
167 if (OtherED < ThisED + Threshold)
168 return false;
169 }
170
171 return true;
172 }
173
sameName(StringRef InComment,StringRef InDecl,bool StrictMode)174 static bool sameName(StringRef InComment, StringRef InDecl, bool StrictMode) {
175 if (StrictMode)
176 return InComment == InDecl;
177 InComment = InComment.trim('_');
178 InDecl = InDecl.trim('_');
179 // FIXME: compare_lower only works for ASCII.
180 return InComment.compare_lower(InDecl) == 0;
181 }
182
looksLikeExpectMethod(const CXXMethodDecl * Expect)183 static bool looksLikeExpectMethod(const CXXMethodDecl *Expect) {
184 return Expect != nullptr && Expect->getLocation().isMacroID() &&
185 Expect->getNameInfo().getName().isIdentifier() &&
186 Expect->getName().startswith("gmock_");
187 }
areMockAndExpectMethods(const CXXMethodDecl * Mock,const CXXMethodDecl * Expect)188 static bool areMockAndExpectMethods(const CXXMethodDecl *Mock,
189 const CXXMethodDecl *Expect) {
190 assert(looksLikeExpectMethod(Expect));
191 return Mock != nullptr && Mock->getNextDeclInContext() == Expect &&
192 Mock->getNumParams() == Expect->getNumParams() &&
193 Mock->getLocation().isMacroID() &&
194 Mock->getNameInfo().getName().isIdentifier() &&
195 Mock->getName() == Expect->getName().substr(strlen("gmock_"));
196 }
197
198 // This uses implementation details of MOCK_METHODx_ macros: for each mocked
199 // method M it defines M() with appropriate signature and a method used to set
200 // up expectations - gmock_M() - with each argument's type changed the
201 // corresponding matcher. This function returns M when given either M or
202 // gmock_M.
findMockedMethod(const CXXMethodDecl * Method)203 static const CXXMethodDecl *findMockedMethod(const CXXMethodDecl *Method) {
204 if (looksLikeExpectMethod(Method)) {
205 const DeclContext *Ctx = Method->getDeclContext();
206 if (Ctx == nullptr || !Ctx->isRecord())
207 return nullptr;
208 for (const auto *D : Ctx->decls()) {
209 if (D->getNextDeclInContext() == Method) {
210 const auto *Previous = dyn_cast<CXXMethodDecl>(D);
211 return areMockAndExpectMethods(Previous, Method) ? Previous : nullptr;
212 }
213 }
214 return nullptr;
215 }
216 if (const auto *Next =
217 dyn_cast_or_null<CXXMethodDecl>(Method->getNextDeclInContext())) {
218 if (looksLikeExpectMethod(Next) && areMockAndExpectMethods(Method, Next))
219 return Method;
220 }
221 return nullptr;
222 }
223
224 // For gmock expectation builder method (the target of the call generated by
225 // `EXPECT_CALL(obj, Method(...))`) tries to find the real method being mocked
226 // (returns nullptr, if the mock method doesn't override anything). For other
227 // functions returns the function itself.
resolveMocks(const FunctionDecl * Func)228 static const FunctionDecl *resolveMocks(const FunctionDecl *Func) {
229 if (const auto *Method = dyn_cast<CXXMethodDecl>(Func)) {
230 if (const auto *MockedMethod = findMockedMethod(Method)) {
231 // If mocked method overrides the real one, we can use its parameter
232 // names, otherwise we're out of luck.
233 if (MockedMethod->size_overridden_methods() > 0) {
234 return *MockedMethod->begin_overridden_methods();
235 }
236 return nullptr;
237 }
238 }
239 return Func;
240 }
241
242 // Given the argument type and the options determine if we should
243 // be adding an argument comment.
shouldAddComment(const Expr * Arg) const244 bool ArgumentCommentCheck::shouldAddComment(const Expr *Arg) const {
245 Arg = Arg->IgnoreImpCasts();
246 if (isa<UnaryOperator>(Arg))
247 Arg = cast<UnaryOperator>(Arg)->getSubExpr();
248 if (Arg->getExprLoc().isMacroID())
249 return false;
250 return (CommentBoolLiterals && isa<CXXBoolLiteralExpr>(Arg)) ||
251 (CommentIntegerLiterals && isa<IntegerLiteral>(Arg)) ||
252 (CommentFloatLiterals && isa<FloatingLiteral>(Arg)) ||
253 (CommentUserDefinedLiterals && isa<UserDefinedLiteral>(Arg)) ||
254 (CommentCharacterLiterals && isa<CharacterLiteral>(Arg)) ||
255 (CommentStringLiterals && isa<StringLiteral>(Arg)) ||
256 (CommentNullPtrs && isa<CXXNullPtrLiteralExpr>(Arg));
257 }
258
checkCallArgs(ASTContext * Ctx,const FunctionDecl * OriginalCallee,SourceLocation ArgBeginLoc,llvm::ArrayRef<const Expr * > Args)259 void ArgumentCommentCheck::checkCallArgs(ASTContext *Ctx,
260 const FunctionDecl *OriginalCallee,
261 SourceLocation ArgBeginLoc,
262 llvm::ArrayRef<const Expr *> Args) {
263 const FunctionDecl *Callee = resolveMocks(OriginalCallee);
264 if (!Callee)
265 return;
266
267 Callee = Callee->getFirstDecl();
268 unsigned NumArgs = std::min<unsigned>(Args.size(), Callee->getNumParams());
269 if ((NumArgs == 0) || (IgnoreSingleArgument && NumArgs == 1))
270 return;
271
272 auto MakeFileCharRange = [Ctx](SourceLocation Begin, SourceLocation End) {
273 return Lexer::makeFileCharRange(CharSourceRange::getCharRange(Begin, End),
274 Ctx->getSourceManager(),
275 Ctx->getLangOpts());
276 };
277
278 for (unsigned I = 0; I < NumArgs; ++I) {
279 const ParmVarDecl *PVD = Callee->getParamDecl(I);
280 IdentifierInfo *II = PVD->getIdentifier();
281 if (!II)
282 continue;
283 if (auto Template = Callee->getTemplateInstantiationPattern()) {
284 // Don't warn on arguments for parameters instantiated from template
285 // parameter packs. If we find more arguments than the template
286 // definition has, it also means that they correspond to a parameter
287 // pack.
288 if (Template->getNumParams() <= I ||
289 Template->getParamDecl(I)->isParameterPack()) {
290 continue;
291 }
292 }
293
294 CharSourceRange BeforeArgument =
295 MakeFileCharRange(ArgBeginLoc, Args[I]->getBeginLoc());
296 ArgBeginLoc = Args[I]->getEndLoc();
297
298 std::vector<std::pair<SourceLocation, StringRef>> Comments;
299 if (BeforeArgument.isValid()) {
300 Comments = getCommentsInRange(Ctx, BeforeArgument);
301 } else {
302 // Fall back to parsing back from the start of the argument.
303 CharSourceRange ArgsRange =
304 MakeFileCharRange(Args[I]->getBeginLoc(), Args[I]->getEndLoc());
305 Comments = getCommentsBeforeLoc(Ctx, ArgsRange.getBegin());
306 }
307
308 for (auto Comment : Comments) {
309 llvm::SmallVector<StringRef, 2> Matches;
310 if (IdentRE.match(Comment.second, &Matches) &&
311 !sameName(Matches[2], II->getName(), StrictMode)) {
312 {
313 DiagnosticBuilder Diag =
314 diag(Comment.first, "argument name '%0' in comment does not "
315 "match parameter name %1")
316 << Matches[2] << II;
317 if (isLikelyTypo(Callee->parameters(), Matches[2], I)) {
318 Diag << FixItHint::CreateReplacement(
319 Comment.first, (Matches[1] + II->getName() + Matches[3]).str());
320 }
321 }
322 diag(PVD->getLocation(), "%0 declared here", DiagnosticIDs::Note) << II;
323 if (OriginalCallee != Callee) {
324 diag(OriginalCallee->getLocation(),
325 "actual callee (%0) is declared here", DiagnosticIDs::Note)
326 << OriginalCallee;
327 }
328 }
329 }
330
331 // If the argument comments are missing for literals add them.
332 if (Comments.empty() && shouldAddComment(Args[I])) {
333 std::string ArgComment =
334 (llvm::Twine("/*") + II->getName() + "=*/").str();
335 DiagnosticBuilder Diag =
336 diag(Args[I]->getBeginLoc(),
337 "argument comment missing for literal argument %0")
338 << II
339 << FixItHint::CreateInsertion(Args[I]->getBeginLoc(), ArgComment);
340 }
341 }
342 }
343
check(const MatchFinder::MatchResult & Result)344 void ArgumentCommentCheck::check(const MatchFinder::MatchResult &Result) {
345 const auto *E = Result.Nodes.getNodeAs<Expr>("expr");
346 if (const auto *Call = dyn_cast<CallExpr>(E)) {
347 const FunctionDecl *Callee = Call->getDirectCallee();
348 if (!Callee)
349 return;
350
351 checkCallArgs(Result.Context, Callee, Call->getCallee()->getEndLoc(),
352 llvm::makeArrayRef(Call->getArgs(), Call->getNumArgs()));
353 } else {
354 const auto *Construct = cast<CXXConstructExpr>(E);
355 if (Construct->getNumArgs() > 0 &&
356 Construct->getArg(0)->getSourceRange() == Construct->getSourceRange()) {
357 // Ignore implicit construction.
358 return;
359 }
360 checkCallArgs(
361 Result.Context, Construct->getConstructor(),
362 Construct->getParenOrBraceRange().getBegin(),
363 llvm::makeArrayRef(Construct->getArgs(), Construct->getNumArgs()));
364 }
365 }
366
367 } // namespace bugprone
368 } // namespace tidy
369 } // namespace clang
370