Peter Collingbourne | 35c3f61 | 2014-03-18 04:46:45 +0000 | [diff] [blame] | 1 | //===--- ArgumentCommentCheck.cpp - clang-tidy ----------------------------===// |
| 2 | // |
Chandler Carruth | 2946cd7 | 2019-01-19 08:50:56 +0000 | [diff] [blame] | 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 |
Peter Collingbourne | 35c3f61 | 2014-03-18 04:46:45 +0000 | [diff] [blame] | 6 | // |
| 7 | //===----------------------------------------------------------------------===// |
| 8 | |
| 9 | #include "ArgumentCommentCheck.h" |
Peter Collingbourne | 35c3f61 | 2014-03-18 04:46:45 +0000 | [diff] [blame] | 10 | #include "clang/AST/ASTContext.h" |
| 11 | #include "clang/ASTMatchers/ASTMatchFinder.h" |
| 12 | #include "clang/Lex/Lexer.h" |
| 13 | #include "clang/Lex/Token.h" |
Paul Hoad | 6bfd721 | 2019-02-08 17:00:01 +0000 | [diff] [blame] | 14 | |
Alexander Kornienko | d9fa4e9 | 2017-02-06 15:47:17 +0000 | [diff] [blame] | 15 | #include "../utils/LexerUtils.h" |
Peter Collingbourne | 35c3f61 | 2014-03-18 04:46:45 +0000 | [diff] [blame] | 16 | |
| 17 | using namespace clang::ast_matchers; |
| 18 | |
| 19 | namespace clang { |
| 20 | namespace tidy { |
Alexander Kornienko | 6f67bcb | 2017-11-23 17:02:48 +0000 | [diff] [blame] | 21 | namespace bugprone { |
Haojian Wu | 2a3498e | 2020-05-06 17:06:14 +0200 | [diff] [blame] | 22 | namespace { |
| 23 | AST_MATCHER(Decl, isFromStdNamespace) { |
| 24 | if (const auto *D = Node.getDeclContext()->getEnclosingNamespaceContext()) |
| 25 | return D->isStdNamespace(); |
| 26 | return false; |
| 27 | } |
| 28 | } // namespace |
Peter Collingbourne | 35c3f61 | 2014-03-18 04:46:45 +0000 | [diff] [blame] | 29 | |
Alexander Kornienko | 6e0cbc8 | 2014-09-12 08:53:36 +0000 | [diff] [blame] | 30 | ArgumentCommentCheck::ArgumentCommentCheck(StringRef Name, |
| 31 | ClangTidyContext *Context) |
| 32 | : ClangTidyCheck(Name, Context), |
Nathan James | 672207c | 2020-04-09 22:47:09 +0100 | [diff] [blame] | 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)), |
Alexander Kornienko | 6e0cbc8 | 2014-09-12 08:53:36 +0000 | [diff] [blame] | 43 | IdentRE("^(/\\* *)([_A-Za-z][_A-Za-z0-9]*)( *= *\\*/)$") {} |
NAKAMURA Takumi | afc4965 | 2014-03-18 07:22:43 +0000 | [diff] [blame] | 44 | |
Alexander Kornienko | 6b2a4d5 | 2016-08-04 14:54:54 +0000 | [diff] [blame] | 45 | void ArgumentCommentCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) { |
| 46 | Options.store(Opts, "StrictMode", StrictMode); |
Alexander Kornienko | 42443e5 | 2019-09-05 14:48:23 +0000 | [diff] [blame] | 47 | Options.store(Opts, "IgnoreSingleArgument", IgnoreSingleArgument); |
Paul Hoad | 6bfd721 | 2019-02-08 17:00:01 +0000 | [diff] [blame] | 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); |
Alexander Kornienko | 6b2a4d5 | 2016-08-04 14:54:54 +0000 | [diff] [blame] | 55 | } |
| 56 | |
Peter Collingbourne | 35c3f61 | 2014-03-18 04:46:45 +0000 | [diff] [blame] | 57 | void ArgumentCommentCheck::registerMatchers(MatchFinder *Finder) { |
Alexander Kornienko | 326e48c | 2014-07-30 14:31:36 +0000 | [diff] [blame] | 58 | Finder->addMatcher( |
Aaron Ballman | b9ea09c | 2015-09-17 13:31:25 +0000 | [diff] [blame] | 59 | callExpr(unless(cxxOperatorCallExpr()), |
Paul Hoad | 6bfd721 | 2019-02-08 17:00:01 +0000 | [diff] [blame] | 60 | // NewCallback's arguments relate to the pointed function, |
| 61 | // don't check them against NewCallback's parameter names. |
Alexander Kornienko | 326e48c | 2014-07-30 14:31:36 +0000 | [diff] [blame] | 62 | // FIXME: Make this configurable. |
Alexander Kornienko | 6b2a4d5 | 2016-08-04 14:54:54 +0000 | [diff] [blame] | 63 | unless(hasDeclaration(functionDecl( |
Haojian Wu | 2a3498e | 2020-05-06 17:06:14 +0200 | [diff] [blame] | 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()))) |
Alexander Kornienko | 326e48c | 2014-07-30 14:31:36 +0000 | [diff] [blame] | 70 | .bind("expr"), |
| 71 | this); |
Haojian Wu | 2a3498e | 2020-05-06 17:06:14 +0200 | [diff] [blame] | 72 | Finder->addMatcher( |
| 73 | cxxConstructExpr(unless(hasDeclaration(isFromStdNamespace()))) |
| 74 | .bind("expr"), |
| 75 | this); |
Peter Collingbourne | 35c3f61 | 2014-03-18 04:46:45 +0000 | [diff] [blame] | 76 | } |
| 77 | |
Alexander Kornienko | 3971dba | 2016-06-08 15:27:46 +0000 | [diff] [blame] | 78 | static std::vector<std::pair<SourceLocation, StringRef>> |
| 79 | getCommentsInRange(ASTContext *Ctx, CharSourceRange Range) { |
Peter Collingbourne | 35c3f61 | 2014-03-18 04:46:45 +0000 | [diff] [blame] | 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; |
Alexander Kornienko | 165fe63 | 2017-02-01 15:28:25 +0000 | [diff] [blame] | 103 | if (Tok.getLocation() == Range.getEnd() || Tok.is(tok::eof)) |
Peter Collingbourne | 35c3f61 | 2014-03-18 04:46:45 +0000 | [diff] [blame] | 104 | break; |
| 105 | |
Alexander Kornienko | 165fe63 | 2017-02-01 15:28:25 +0000 | [diff] [blame] | 106 | if (Tok.is(tok::comment)) { |
Peter Collingbourne | 35c3f61 | 2014-03-18 04:46:45 +0000 | [diff] [blame] | 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())); |
Alexander Kornienko | 165fe63 | 2017-02-01 15:28:25 +0000 | [diff] [blame] | 113 | } else { |
| 114 | // Clear comments found before the different token, e.g. comma. |
| 115 | Comments.clear(); |
Peter Collingbourne | 35c3f61 | 2014-03-18 04:46:45 +0000 | [diff] [blame] | 116 | } |
| 117 | } |
| 118 | |
| 119 | return Comments; |
| 120 | } |
| 121 | |
Alexander Kornienko | d9fa4e9 | 2017-02-06 15:47:17 +0000 | [diff] [blame] | 122 | static std::vector<std::pair<SourceLocation, StringRef>> |
| 123 | getCommentsBeforeLoc(ASTContext *Ctx, SourceLocation Loc) { |
| 124 | std::vector<std::pair<SourceLocation, StringRef>> Comments; |
| 125 | while (Loc.isValid()) { |
Jonas Toth | a78c249 | 2018-10-05 14:15:19 +0000 | [diff] [blame] | 126 | clang::Token Tok = utils::lexer::getPreviousToken( |
| 127 | Loc, Ctx->getSourceManager(), Ctx->getLangOpts(), |
| 128 | /*SkipComments=*/false); |
Alexander Kornienko | d9fa4e9 | 2017-02-06 15:47:17 +0000 | [diff] [blame] | 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 | |
| 141 | static bool isLikelyTypo(llvm::ArrayRef<ParmVarDecl *> Params, |
| 142 | StringRef ArgName, unsigned ArgIndex) { |
Alexander Kornienko | 6b2a4d5 | 2016-08-04 14:54:54 +0000 | [diff] [blame] | 143 | std::string ArgNameLowerStr = ArgName.lower(); |
| 144 | StringRef ArgNameLower = ArgNameLowerStr; |
| 145 | // The threshold is arbitrary. |
Peter Collingbourne | 35c3f61 | 2014-03-18 04:46:45 +0000 | [diff] [blame] | 146 | unsigned UpperBound = (ArgName.size() + 2) / 3 + 1; |
Alexander Kornienko | 6b2a4d5 | 2016-08-04 14:54:54 +0000 | [diff] [blame] | 147 | unsigned ThisED = ArgNameLower.edit_distance( |
Peter Collingbourne | 35c3f61 | 2014-03-18 04:46:45 +0000 | [diff] [blame] | 148 | Params[ArgIndex]->getIdentifier()->getName().lower(), |
| 149 | /*AllowReplacements=*/true, UpperBound); |
| 150 | if (ThisED >= UpperBound) |
| 151 | return false; |
| 152 | |
Benjamin Kramer | f5847cf | 2014-08-04 10:11:47 +0000 | [diff] [blame] | 153 | for (unsigned I = 0, E = Params.size(); I != E; ++I) { |
| 154 | if (I == ArgIndex) |
Peter Collingbourne | 35c3f61 | 2014-03-18 04:46:45 +0000 | [diff] [blame] | 155 | continue; |
Benjamin Kramer | f5847cf | 2014-08-04 10:11:47 +0000 | [diff] [blame] | 156 | IdentifierInfo *II = Params[I]->getIdentifier(); |
Peter Collingbourne | 35c3f61 | 2014-03-18 04:46:45 +0000 | [diff] [blame] | 157 | if (!II) |
| 158 | continue; |
| 159 | |
| 160 | const unsigned Threshold = 2; |
| 161 | // Other parameters must be an edit distance at least Threshold more away |
Paul Hoad | 6bfd721 | 2019-02-08 17:00:01 +0000 | [diff] [blame] | 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. |
Alexander Kornienko | 6b2a4d5 | 2016-08-04 14:54:54 +0000 | [diff] [blame] | 164 | unsigned OtherED = ArgNameLower.edit_distance(II->getName().lower(), |
| 165 | /*AllowReplacements=*/true, |
| 166 | ThisED + Threshold); |
Peter Collingbourne | 35c3f61 | 2014-03-18 04:46:45 +0000 | [diff] [blame] | 167 | if (OtherED < ThisED + Threshold) |
| 168 | return false; |
| 169 | } |
| 170 | |
| 171 | return true; |
| 172 | } |
| 173 | |
Alexander Kornienko | 6b2a4d5 | 2016-08-04 14:54:54 +0000 | [diff] [blame] | 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 | |
Alexander Kornienko | 258d9a5 | 2017-02-07 11:39:56 +0000 | [diff] [blame] | 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 | } |
| 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 | |
Alexander Kornienko | d9fa4e9 | 2017-02-06 15:47:17 +0000 | [diff] [blame] | 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 |
Alexander Kornienko | 258d9a5 | 2017-02-07 11:39:56 +0000 | [diff] [blame] | 201 | // corresponding matcher. This function returns M when given either M or |
| 202 | // gmock_M. |
| 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; |
Alexander Kornienko | d9fa4e9 | 2017-02-06 15:47:17 +0000 | [diff] [blame] | 212 | } |
| 213 | } |
Alexander Kornienko | 258d9a5 | 2017-02-07 11:39:56 +0000 | [diff] [blame] | 214 | return nullptr; |
| 215 | } |
Paul Hoad | 6bfd721 | 2019-02-08 17:00:01 +0000 | [diff] [blame] | 216 | if (const auto *Next = |
| 217 | dyn_cast_or_null<CXXMethodDecl>(Method->getNextDeclInContext())) { |
Alexander Kornienko | 258d9a5 | 2017-02-07 11:39:56 +0000 | [diff] [blame] | 218 | if (looksLikeExpectMethod(Next) && areMockAndExpectMethods(Method, Next)) |
| 219 | return Method; |
Alexander Kornienko | d9fa4e9 | 2017-02-06 15:47:17 +0000 | [diff] [blame] | 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. |
| 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 | |
Paul Hoad | 6bfd721 | 2019-02-08 17:00:01 +0000 | [diff] [blame] | 242 | // Given the argument type and the options determine if we should |
| 243 | // be adding an argument comment. |
| 244 | bool ArgumentCommentCheck::shouldAddComment(const Expr *Arg) const { |
Alexander Kornienko | b6d9703 | 2019-09-05 14:13:57 +0000 | [diff] [blame] | 245 | Arg = Arg->IgnoreImpCasts(); |
| 246 | if (isa<UnaryOperator>(Arg)) |
| 247 | Arg = cast<UnaryOperator>(Arg)->getSubExpr(); |
Paul Hoad | 6bfd721 | 2019-02-08 17:00:01 +0000 | [diff] [blame] | 248 | if (Arg->getExprLoc().isMacroID()) |
| 249 | return false; |
Paul Hoad | 6bfd721 | 2019-02-08 17:00:01 +0000 | [diff] [blame] | 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 | |
Peter Collingbourne | 35c3f61 | 2014-03-18 04:46:45 +0000 | [diff] [blame] | 259 | void ArgumentCommentCheck::checkCallArgs(ASTContext *Ctx, |
Alexander Kornienko | d9fa4e9 | 2017-02-06 15:47:17 +0000 | [diff] [blame] | 260 | const FunctionDecl *OriginalCallee, |
Peter Collingbourne | 35c3f61 | 2014-03-18 04:46:45 +0000 | [diff] [blame] | 261 | SourceLocation ArgBeginLoc, |
| 262 | llvm::ArrayRef<const Expr *> Args) { |
Alexander Kornienko | d9fa4e9 | 2017-02-06 15:47:17 +0000 | [diff] [blame] | 263 | const FunctionDecl *Callee = resolveMocks(OriginalCallee); |
| 264 | if (!Callee) |
| 265 | return; |
| 266 | |
Peter Collingbourne | 35c3f61 | 2014-03-18 04:46:45 +0000 | [diff] [blame] | 267 | Callee = Callee->getFirstDecl(); |
Alexander Kornienko | d9fa4e9 | 2017-02-06 15:47:17 +0000 | [diff] [blame] | 268 | unsigned NumArgs = std::min<unsigned>(Args.size(), Callee->getNumParams()); |
Alexander Kornienko | 42443e5 | 2019-09-05 14:48:23 +0000 | [diff] [blame] | 269 | if ((NumArgs == 0) || (IgnoreSingleArgument && NumArgs == 1)) |
Alexander Kornienko | d9fa4e9 | 2017-02-06 15:47:17 +0000 | [diff] [blame] | 270 | return; |
| 271 | |
Paul Hoad | 6bfd721 | 2019-02-08 17:00:01 +0000 | [diff] [blame] | 272 | auto MakeFileCharRange = [Ctx](SourceLocation Begin, SourceLocation End) { |
Alexander Kornienko | d9fa4e9 | 2017-02-06 15:47:17 +0000 | [diff] [blame] | 273 | return Lexer::makeFileCharRange(CharSourceRange::getCharRange(Begin, End), |
| 274 | Ctx->getSourceManager(), |
| 275 | Ctx->getLangOpts()); |
| 276 | }; |
| 277 | |
| 278 | for (unsigned I = 0; I < NumArgs; ++I) { |
Alexander Kornienko | 6b2a4d5 | 2016-08-04 14:54:54 +0000 | [diff] [blame] | 279 | const ParmVarDecl *PVD = Callee->getParamDecl(I); |
Peter Collingbourne | 35c3f61 | 2014-03-18 04:46:45 +0000 | [diff] [blame] | 280 | IdentifierInfo *II = PVD->getIdentifier(); |
| 281 | if (!II) |
| 282 | continue; |
Alexander Kornienko | 805b44d | 2014-09-03 14:56:30 +0000 | [diff] [blame] | 283 | if (auto Template = Callee->getTemplateInstantiationPattern()) { |
| 284 | // Don't warn on arguments for parameters instantiated from template |
Alexander Kornienko | d9fa4e9 | 2017-02-06 15:47:17 +0000 | [diff] [blame] | 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. |
Alexander Kornienko | 6b2a4d5 | 2016-08-04 14:54:54 +0000 | [diff] [blame] | 288 | if (Template->getNumParams() <= I || |
| 289 | Template->getParamDecl(I)->isParameterPack()) { |
Alexander Kornienko | 805b44d | 2014-09-03 14:56:30 +0000 | [diff] [blame] | 290 | continue; |
| 291 | } |
| 292 | } |
Peter Collingbourne | 35c3f61 | 2014-03-18 04:46:45 +0000 | [diff] [blame] | 293 | |
Alexander Kornienko | d9fa4e9 | 2017-02-06 15:47:17 +0000 | [diff] [blame] | 294 | CharSourceRange BeforeArgument = |
Paul Hoad | 6bfd721 | 2019-02-08 17:00:01 +0000 | [diff] [blame] | 295 | MakeFileCharRange(ArgBeginLoc, Args[I]->getBeginLoc()); |
Stephen Kelly | c09197e | 2018-08-09 22:43:02 +0000 | [diff] [blame] | 296 | ArgBeginLoc = Args[I]->getEndLoc(); |
Peter Collingbourne | 35c3f61 | 2014-03-18 04:46:45 +0000 | [diff] [blame] | 297 | |
Alexander Kornienko | d9fa4e9 | 2017-02-06 15:47:17 +0000 | [diff] [blame] | 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. |
Alexander Kornienko | 240a2e2 | 2019-09-04 16:19:32 +0000 | [diff] [blame] | 303 | CharSourceRange ArgsRange = |
| 304 | MakeFileCharRange(Args[I]->getBeginLoc(), Args[I]->getEndLoc()); |
Alexander Kornienko | d9fa4e9 | 2017-02-06 15:47:17 +0000 | [diff] [blame] | 305 | Comments = getCommentsBeforeLoc(Ctx, ArgsRange.getBegin()); |
| 306 | } |
| 307 | |
| 308 | for (auto Comment : Comments) { |
Peter Collingbourne | 35c3f61 | 2014-03-18 04:46:45 +0000 | [diff] [blame] | 309 | llvm::SmallVector<StringRef, 2> Matches; |
Alexander Kornienko | d9fa4e9 | 2017-02-06 15:47:17 +0000 | [diff] [blame] | 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()); |
Peter Collingbourne | 35c3f61 | 2014-03-18 04:46:45 +0000 | [diff] [blame] | 320 | } |
Alexander Kornienko | d9fa4e9 | 2017-02-06 15:47:17 +0000 | [diff] [blame] | 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; |
Peter Collingbourne | 35c3f61 | 2014-03-18 04:46:45 +0000 | [diff] [blame] | 327 | } |
| 328 | } |
| 329 | } |
Paul Hoad | 6bfd721 | 2019-02-08 17:00:01 +0000 | [diff] [blame] | 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 | } |
Peter Collingbourne | 35c3f61 | 2014-03-18 04:46:45 +0000 | [diff] [blame] | 341 | } |
Dmitri Gribenko | 04b34a2 | 2019-09-23 12:07:10 +0000 | [diff] [blame] | 342 | } |
Peter Collingbourne | 35c3f61 | 2014-03-18 04:46:45 +0000 | [diff] [blame] | 343 | |
| 344 | void ArgumentCommentCheck::check(const MatchFinder::MatchResult &Result) { |
Piotr Padlewski | 08124b1 | 2016-12-14 15:29:23 +0000 | [diff] [blame] | 345 | const auto *E = Result.Nodes.getNodeAs<Expr>("expr"); |
Alexander Kornienko | 6b2a4d5 | 2016-08-04 14:54:54 +0000 | [diff] [blame] | 346 | if (const auto *Call = dyn_cast<CallExpr>(E)) { |
Peter Collingbourne | 35c3f61 | 2014-03-18 04:46:45 +0000 | [diff] [blame] | 347 | const FunctionDecl *Callee = Call->getDirectCallee(); |
| 348 | if (!Callee) |
| 349 | return; |
| 350 | |
Stephen Kelly | c09197e | 2018-08-09 22:43:02 +0000 | [diff] [blame] | 351 | checkCallArgs(Result.Context, Callee, Call->getCallee()->getEndLoc(), |
Peter Collingbourne | 35c3f61 | 2014-03-18 04:46:45 +0000 | [diff] [blame] | 352 | llvm::makeArrayRef(Call->getArgs(), Call->getNumArgs())); |
| 353 | } else { |
Alexander Kornienko | 6b2a4d5 | 2016-08-04 14:54:54 +0000 | [diff] [blame] | 354 | const auto *Construct = cast<CXXConstructExpr>(E); |
Yitzhak Mandelbaum | 57990b4 | 2019-09-19 13:12:05 +0000 | [diff] [blame] | 355 | if (Construct->getNumArgs() > 0 && |
Alexander Kornienko | d9fa4e9 | 2017-02-06 15:47:17 +0000 | [diff] [blame] | 356 | Construct->getArg(0)->getSourceRange() == Construct->getSourceRange()) { |
| 357 | // Ignore implicit construction. |
| 358 | return; |
| 359 | } |
Peter Collingbourne | 35c3f61 | 2014-03-18 04:46:45 +0000 | [diff] [blame] | 360 | checkCallArgs( |
| 361 | Result.Context, Construct->getConstructor(), |
| 362 | Construct->getParenOrBraceRange().getBegin(), |
| 363 | llvm::makeArrayRef(Construct->getArgs(), Construct->getNumArgs())); |
| 364 | } |
| 365 | } |
| 366 | |
Alexander Kornienko | 6f67bcb | 2017-11-23 17:02:48 +0000 | [diff] [blame] | 367 | } // namespace bugprone |
Peter Collingbourne | 35c3f61 | 2014-03-18 04:46:45 +0000 | [diff] [blame] | 368 | } // namespace tidy |
| 369 | } // namespace clang |