1 | //===--- InconsistentDeclarationParameterNameCheck.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 "InconsistentDeclarationParameterNameCheck.h" |
10 | #include "clang/AST/ASTContext.h" |
11 | #include "clang/ASTMatchers/ASTMatchFinder.h" |
12 | #include "llvm/ADT/STLExtras.h" |
13 | |
14 | #include <functional> |
15 | |
16 | using namespace clang::ast_matchers; |
17 | |
18 | namespace clang::tidy::readability { |
19 | |
20 | namespace { |
21 | |
22 | AST_MATCHER(FunctionDecl, hasOtherDeclarations) { |
23 | auto It = Node.redecls_begin(); |
24 | auto EndIt = Node.redecls_end(); |
25 | |
26 | if (It == EndIt) |
27 | return false; |
28 | |
29 | ++It; |
30 | return It != EndIt; |
31 | } |
32 | |
33 | struct DifferingParamInfo { |
34 | DifferingParamInfo(StringRef SourceName, StringRef OtherName, |
35 | SourceRange OtherNameRange, bool GenerateFixItHint) |
36 | : SourceName(SourceName), OtherName(OtherName), |
37 | OtherNameRange(OtherNameRange), GenerateFixItHint(GenerateFixItHint) {} |
38 | |
39 | StringRef SourceName; |
40 | StringRef OtherName; |
41 | SourceRange OtherNameRange; |
42 | bool GenerateFixItHint; |
43 | }; |
44 | |
45 | using DifferingParamsContainer = llvm::SmallVector<DifferingParamInfo, 10>; |
46 | |
47 | struct InconsistentDeclarationInfo { |
48 | InconsistentDeclarationInfo(SourceLocation DeclarationLocation, |
49 | DifferingParamsContainer &&DifferingParams) |
50 | : DeclarationLocation(DeclarationLocation), |
51 | DifferingParams(std::move(DifferingParams)) {} |
52 | |
53 | SourceLocation DeclarationLocation; |
54 | DifferingParamsContainer DifferingParams; |
55 | }; |
56 | |
57 | using InconsistentDeclarationsContainer = |
58 | llvm::SmallVector<InconsistentDeclarationInfo, 2>; |
59 | |
60 | bool checkIfFixItHintIsApplicable( |
61 | const FunctionDecl *ParameterSourceDeclaration, |
62 | const ParmVarDecl *SourceParam, const FunctionDecl *OriginalDeclaration) { |
63 | // Assumptions with regard to function declarations/definition: |
64 | // * If both function declaration and definition are seen, assume that |
65 | // definition is most up-to-date, and use it to generate replacements. |
66 | // * If only function declarations are seen, there is no easy way to tell |
67 | // which is up-to-date and which is not, so don't do anything. |
68 | // TODO: This may be changed later, but for now it seems the reasonable |
69 | // solution. |
70 | if (!ParameterSourceDeclaration->isThisDeclarationADefinition()) |
71 | return false; |
72 | |
73 | // Assumption: if parameter is not referenced in function definition body, it |
74 | // may indicate that it's outdated, so don't touch it. |
75 | if (!SourceParam->isReferenced()) |
76 | return false; |
77 | |
78 | // In case there is the primary template definition and (possibly several) |
79 | // template specializations (and each with possibly several redeclarations), |
80 | // it is not at all clear what to change. |
81 | if (OriginalDeclaration->getTemplatedKind() == |
82 | FunctionDecl::TK_FunctionTemplateSpecialization) |
83 | return false; |
84 | |
85 | // Other cases seem OK to allow replacements. |
86 | return true; |
87 | } |
88 | |
89 | bool nameMatch(StringRef L, StringRef R, bool Strict) { |
90 | if (Strict) |
91 | return L.empty() || R.empty() || L == R; |
92 | // We allow two names if one is a prefix/suffix of the other, ignoring case. |
93 | // Important special case: this is true if either parameter has no name! |
94 | return L.starts_with_insensitive(Prefix: R) || R.starts_with_insensitive(Prefix: L) || |
95 | L.ends_with_insensitive(Suffix: R) || R.ends_with_insensitive(Suffix: L); |
96 | } |
97 | |
98 | DifferingParamsContainer |
99 | findDifferingParamsInDeclaration(const FunctionDecl *ParameterSourceDeclaration, |
100 | const FunctionDecl *OtherDeclaration, |
101 | const FunctionDecl *OriginalDeclaration, |
102 | bool Strict) { |
103 | DifferingParamsContainer DifferingParams; |
104 | |
105 | const auto *SourceParamIt = ParameterSourceDeclaration->param_begin(); |
106 | const auto *OtherParamIt = OtherDeclaration->param_begin(); |
107 | |
108 | while (SourceParamIt != ParameterSourceDeclaration->param_end() && |
109 | OtherParamIt != OtherDeclaration->param_end()) { |
110 | auto SourceParamName = (*SourceParamIt)->getName(); |
111 | auto OtherParamName = (*OtherParamIt)->getName(); |
112 | |
113 | // FIXME: Provide a way to extract commented out parameter name from comment |
114 | // next to it. |
115 | if (!nameMatch(SourceParamName, OtherParamName, Strict)) { |
116 | SourceRange OtherParamNameRange = |
117 | DeclarationNameInfo((*OtherParamIt)->getDeclName(), |
118 | (*OtherParamIt)->getLocation()) |
119 | .getSourceRange(); |
120 | |
121 | bool GenerateFixItHint = checkIfFixItHintIsApplicable( |
122 | ParameterSourceDeclaration, SourceParam: *SourceParamIt, OriginalDeclaration); |
123 | |
124 | DifferingParams.emplace_back(SourceParamName, OtherParamName, |
125 | OtherParamNameRange, GenerateFixItHint); |
126 | } |
127 | |
128 | ++SourceParamIt; |
129 | ++OtherParamIt; |
130 | } |
131 | |
132 | return DifferingParams; |
133 | } |
134 | |
135 | InconsistentDeclarationsContainer |
136 | findInconsistentDeclarations(const FunctionDecl *OriginalDeclaration, |
137 | const FunctionDecl *ParameterSourceDeclaration, |
138 | SourceManager &SM, bool Strict) { |
139 | InconsistentDeclarationsContainer InconsistentDeclarations; |
140 | SourceLocation ParameterSourceLocation = |
141 | ParameterSourceDeclaration->getLocation(); |
142 | |
143 | for (const FunctionDecl *OtherDeclaration : OriginalDeclaration->redecls()) { |
144 | SourceLocation OtherLocation = OtherDeclaration->getLocation(); |
145 | if (OtherLocation != ParameterSourceLocation) { // Skip self. |
146 | DifferingParamsContainer DifferingParams = |
147 | findDifferingParamsInDeclaration(ParameterSourceDeclaration, |
148 | OtherDeclaration, |
149 | OriginalDeclaration, Strict); |
150 | if (!DifferingParams.empty()) { |
151 | InconsistentDeclarations.emplace_back(OtherDeclaration->getLocation(), |
152 | std::move(DifferingParams)); |
153 | } |
154 | } |
155 | } |
156 | |
157 | // Sort in order of appearance in translation unit to generate clear |
158 | // diagnostics. |
159 | llvm::sort(C&: InconsistentDeclarations, |
160 | Comp: [&SM](const InconsistentDeclarationInfo &Info1, |
161 | const InconsistentDeclarationInfo &Info2) { |
162 | return SM.isBeforeInTranslationUnit(LHS: Info1.DeclarationLocation, |
163 | RHS: Info2.DeclarationLocation); |
164 | }); |
165 | return InconsistentDeclarations; |
166 | } |
167 | |
168 | const FunctionDecl * |
169 | getParameterSourceDeclaration(const FunctionDecl *OriginalDeclaration) { |
170 | const FunctionTemplateDecl *PrimaryTemplate = |
171 | OriginalDeclaration->getPrimaryTemplate(); |
172 | if (PrimaryTemplate != nullptr) { |
173 | // In case of template specializations, use primary template declaration as |
174 | // the source of parameter names. |
175 | return PrimaryTemplate->getTemplatedDecl(); |
176 | } |
177 | |
178 | // In other cases, try to change to function definition, if available. |
179 | |
180 | if (OriginalDeclaration->isThisDeclarationADefinition()) |
181 | return OriginalDeclaration; |
182 | |
183 | for (const FunctionDecl *OtherDeclaration : OriginalDeclaration->redecls()) { |
184 | if (OtherDeclaration->isThisDeclarationADefinition()) { |
185 | return OtherDeclaration; |
186 | } |
187 | } |
188 | |
189 | // No definition found, so return original declaration. |
190 | return OriginalDeclaration; |
191 | } |
192 | |
193 | std::string joinParameterNames( |
194 | const DifferingParamsContainer &DifferingParams, |
195 | llvm::function_ref<StringRef(const DifferingParamInfo &)> ChooseParamName) { |
196 | llvm::SmallString<40> Str; |
197 | bool First = true; |
198 | for (const DifferingParamInfo &ParamInfo : DifferingParams) { |
199 | if (First) |
200 | First = false; |
201 | else |
202 | Str += ", " ; |
203 | Str.append(Refs: {"'" , ChooseParamName(ParamInfo), "'" }); |
204 | } |
205 | return std::string(Str); |
206 | } |
207 | |
208 | void formatDifferingParamsDiagnostic( |
209 | InconsistentDeclarationParameterNameCheck *Check, SourceLocation Location, |
210 | StringRef OtherDeclarationDescription, |
211 | const DifferingParamsContainer &DifferingParams) { |
212 | auto ChooseOtherName = [](const DifferingParamInfo &ParamInfo) { |
213 | return ParamInfo.OtherName; |
214 | }; |
215 | auto ChooseSourceName = [](const DifferingParamInfo &ParamInfo) { |
216 | return ParamInfo.SourceName; |
217 | }; |
218 | |
219 | auto ParamDiag = |
220 | Check->diag(Loc: Location, |
221 | Description: "differing parameters are named here: (%0), in %1: (%2)" , |
222 | Level: DiagnosticIDs::Level::Note) |
223 | << joinParameterNames(DifferingParams, ChooseParamName: ChooseOtherName) |
224 | << OtherDeclarationDescription |
225 | << joinParameterNames(DifferingParams, ChooseParamName: ChooseSourceName); |
226 | |
227 | for (const DifferingParamInfo &ParamInfo : DifferingParams) { |
228 | if (ParamInfo.GenerateFixItHint) { |
229 | ParamDiag << FixItHint::CreateReplacement( |
230 | RemoveRange: CharSourceRange::getTokenRange(R: ParamInfo.OtherNameRange), |
231 | Code: ParamInfo.SourceName); |
232 | } |
233 | } |
234 | } |
235 | |
236 | void formatDiagnosticsForDeclarations( |
237 | InconsistentDeclarationParameterNameCheck *Check, |
238 | const FunctionDecl *ParameterSourceDeclaration, |
239 | const FunctionDecl *OriginalDeclaration, |
240 | const InconsistentDeclarationsContainer &InconsistentDeclarations) { |
241 | Check->diag( |
242 | OriginalDeclaration->getLocation(), |
243 | "function %q0 has %1 other declaration%s1 with different parameter names" ) |
244 | << OriginalDeclaration |
245 | << static_cast<int>(InconsistentDeclarations.size()); |
246 | int Count = 1; |
247 | for (const InconsistentDeclarationInfo &InconsistentDeclaration : |
248 | InconsistentDeclarations) { |
249 | Check->diag(Loc: InconsistentDeclaration.DeclarationLocation, |
250 | Description: "the %ordinal0 inconsistent declaration seen here" , |
251 | Level: DiagnosticIDs::Level::Note) |
252 | << Count; |
253 | |
254 | formatDifferingParamsDiagnostic( |
255 | Check, Location: InconsistentDeclaration.DeclarationLocation, |
256 | OtherDeclarationDescription: "the other declaration" , DifferingParams: InconsistentDeclaration.DifferingParams); |
257 | |
258 | ++Count; |
259 | } |
260 | } |
261 | |
262 | void formatDiagnostics( |
263 | InconsistentDeclarationParameterNameCheck *Check, |
264 | const FunctionDecl *ParameterSourceDeclaration, |
265 | const FunctionDecl *OriginalDeclaration, |
266 | const InconsistentDeclarationsContainer &InconsistentDeclarations, |
267 | StringRef FunctionDescription, StringRef ParameterSourceDescription) { |
268 | for (const InconsistentDeclarationInfo &InconsistentDeclaration : |
269 | InconsistentDeclarations) { |
270 | Check->diag(Loc: InconsistentDeclaration.DeclarationLocation, |
271 | Description: "%0 %q1 has a %2 with different parameter names" ) |
272 | << FunctionDescription << OriginalDeclaration |
273 | << ParameterSourceDescription; |
274 | |
275 | Check->diag(ParameterSourceDeclaration->getLocation(), "the %0 seen here" , |
276 | DiagnosticIDs::Level::Note) |
277 | << ParameterSourceDescription; |
278 | |
279 | formatDifferingParamsDiagnostic( |
280 | Check, Location: InconsistentDeclaration.DeclarationLocation, |
281 | OtherDeclarationDescription: ParameterSourceDescription, DifferingParams: InconsistentDeclaration.DifferingParams); |
282 | } |
283 | } |
284 | |
285 | } // anonymous namespace |
286 | |
287 | void InconsistentDeclarationParameterNameCheck::storeOptions( |
288 | ClangTidyOptions::OptionMap &Opts) { |
289 | Options.store(Options&: Opts, LocalName: "IgnoreMacros" , Value: IgnoreMacros); |
290 | Options.store(Options&: Opts, LocalName: "Strict" , Value: Strict); |
291 | } |
292 | |
293 | void InconsistentDeclarationParameterNameCheck::registerMatchers( |
294 | MatchFinder *Finder) { |
295 | Finder->addMatcher(NodeMatch: functionDecl(hasOtherDeclarations()).bind(ID: "functionDecl" ), |
296 | Action: this); |
297 | } |
298 | |
299 | void InconsistentDeclarationParameterNameCheck::check( |
300 | const MatchFinder::MatchResult &Result) { |
301 | const auto *OriginalDeclaration = |
302 | Result.Nodes.getNodeAs<FunctionDecl>(ID: "functionDecl" ); |
303 | |
304 | if (VisitedDeclarations.contains(V: OriginalDeclaration)) |
305 | return; // Avoid multiple warnings. |
306 | |
307 | const FunctionDecl *ParameterSourceDeclaration = |
308 | getParameterSourceDeclaration(OriginalDeclaration); |
309 | |
310 | InconsistentDeclarationsContainer InconsistentDeclarations = |
311 | findInconsistentDeclarations(OriginalDeclaration, |
312 | ParameterSourceDeclaration, |
313 | SM&: *Result.SourceManager, Strict); |
314 | if (InconsistentDeclarations.empty()) { |
315 | // Avoid unnecessary further visits. |
316 | markRedeclarationsAsVisited(OriginalDeclaration); |
317 | return; |
318 | } |
319 | |
320 | SourceLocation StartLoc = OriginalDeclaration->getBeginLoc(); |
321 | if (StartLoc.isMacroID() && IgnoreMacros) { |
322 | markRedeclarationsAsVisited(OriginalDeclaration); |
323 | return; |
324 | } |
325 | |
326 | if (OriginalDeclaration->getTemplatedKind() == |
327 | FunctionDecl::TK_FunctionTemplateSpecialization) { |
328 | formatDiagnostics(Check: this, ParameterSourceDeclaration, OriginalDeclaration, |
329 | InconsistentDeclarations, |
330 | FunctionDescription: "function template specialization" , |
331 | ParameterSourceDescription: "primary template declaration" ); |
332 | } else if (ParameterSourceDeclaration->isThisDeclarationADefinition()) { |
333 | formatDiagnostics(Check: this, ParameterSourceDeclaration, OriginalDeclaration, |
334 | InconsistentDeclarations, FunctionDescription: "function" , ParameterSourceDescription: "definition" ); |
335 | } else { |
336 | formatDiagnosticsForDeclarations(Check: this, ParameterSourceDeclaration, |
337 | OriginalDeclaration, |
338 | InconsistentDeclarations); |
339 | } |
340 | |
341 | markRedeclarationsAsVisited(OriginalDeclaration); |
342 | } |
343 | |
344 | void InconsistentDeclarationParameterNameCheck::markRedeclarationsAsVisited( |
345 | const FunctionDecl *OriginalDeclaration) { |
346 | for (const FunctionDecl *Redecl : OriginalDeclaration->redecls()) { |
347 | VisitedDeclarations.insert(Redecl); |
348 | } |
349 | } |
350 | |
351 | } // namespace clang::tidy::readability |
352 | |