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