• Home
  • Line#
  • Scopes#
  • Navigate#
  • Raw
  • Download
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