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