xref: /llvm-project/clang-tools-extra/clang-tidy/readability/IsolateDeclarationCheck.cpp (revision 0ea5af7acb33c8b455edea3a02ee2ffcbbcad3f8)
1*0ea5af7aSJonas Toth //===--- IsolateDeclarationCheck.cpp - clang-tidy -------------------------===//
2*0ea5af7aSJonas Toth //
3*0ea5af7aSJonas Toth //                     The LLVM Compiler Infrastructure
4*0ea5af7aSJonas Toth //
5*0ea5af7aSJonas Toth // This file is distributed under the University of Illinois Open Source
6*0ea5af7aSJonas Toth // License. See LICENSE.TXT for details.
7*0ea5af7aSJonas Toth //
8*0ea5af7aSJonas Toth //===----------------------------------------------------------------------===//
9*0ea5af7aSJonas Toth 
10*0ea5af7aSJonas Toth #include "IsolateDeclarationCheck.h"
11*0ea5af7aSJonas Toth #include "../utils/LexerUtils.h"
12*0ea5af7aSJonas Toth #include "clang/ASTMatchers/ASTMatchFinder.h"
13*0ea5af7aSJonas Toth 
14*0ea5af7aSJonas Toth using namespace clang::ast_matchers;
15*0ea5af7aSJonas Toth using namespace clang::tidy::utils::lexer;
16*0ea5af7aSJonas Toth 
17*0ea5af7aSJonas Toth namespace clang {
18*0ea5af7aSJonas Toth namespace tidy {
19*0ea5af7aSJonas Toth namespace readability {
20*0ea5af7aSJonas Toth 
21*0ea5af7aSJonas Toth namespace {
22*0ea5af7aSJonas Toth AST_MATCHER(DeclStmt, isSingleDecl) { return Node.isSingleDecl(); }
23*0ea5af7aSJonas Toth AST_MATCHER(DeclStmt, onlyDeclaresVariables) {
24*0ea5af7aSJonas Toth   return llvm::all_of(Node.decls(), [](Decl *D) { return isa<VarDecl>(D); });
25*0ea5af7aSJonas Toth }
26*0ea5af7aSJonas Toth } // namespace
27*0ea5af7aSJonas Toth 
28*0ea5af7aSJonas Toth void IsolateDeclarationCheck::registerMatchers(MatchFinder *Finder) {
29*0ea5af7aSJonas Toth   Finder->addMatcher(
30*0ea5af7aSJonas Toth       declStmt(allOf(onlyDeclaresVariables(), unless(isSingleDecl()),
31*0ea5af7aSJonas Toth                      hasParent(compoundStmt())))
32*0ea5af7aSJonas Toth           .bind("decl_stmt"),
33*0ea5af7aSJonas Toth       this);
34*0ea5af7aSJonas Toth }
35*0ea5af7aSJonas Toth 
36*0ea5af7aSJonas Toth static SourceLocation findStartOfIndirection(SourceLocation Start,
37*0ea5af7aSJonas Toth                                              int Indirections,
38*0ea5af7aSJonas Toth                                              const SourceManager &SM,
39*0ea5af7aSJonas Toth                                              const LangOptions &LangOpts) {
40*0ea5af7aSJonas Toth   assert(Indirections >= 0 && "Indirections must be non-negative");
41*0ea5af7aSJonas Toth   if (Indirections == 0)
42*0ea5af7aSJonas Toth     return Start;
43*0ea5af7aSJonas Toth 
44*0ea5af7aSJonas Toth   // Note that the post-fix decrement is necessary to perform the correct
45*0ea5af7aSJonas Toth   // number of transformations.
46*0ea5af7aSJonas Toth   while (Indirections-- != 0) {
47*0ea5af7aSJonas Toth     Start = findPreviousAnyTokenKind(Start, SM, LangOpts, tok::star, tok::amp);
48*0ea5af7aSJonas Toth     if (Start.isInvalid() || Start.isMacroID())
49*0ea5af7aSJonas Toth       return SourceLocation();
50*0ea5af7aSJonas Toth   }
51*0ea5af7aSJonas Toth   return Start;
52*0ea5af7aSJonas Toth }
53*0ea5af7aSJonas Toth 
54*0ea5af7aSJonas Toth static bool isMacroID(SourceRange R) {
55*0ea5af7aSJonas Toth   return R.getBegin().isMacroID() || R.getEnd().isMacroID();
56*0ea5af7aSJonas Toth }
57*0ea5af7aSJonas Toth 
58*0ea5af7aSJonas Toth /// This function counts the number of written indirections for the given
59*0ea5af7aSJonas Toth /// Type \p T. It does \b NOT resolve typedefs as it's a helper for lexing
60*0ea5af7aSJonas Toth /// the source code.
61*0ea5af7aSJonas Toth /// \see declRanges
62*0ea5af7aSJonas Toth static int countIndirections(const Type *T, int Indirections = 0) {
63*0ea5af7aSJonas Toth   if (T->isFunctionPointerType()) {
64*0ea5af7aSJonas Toth     const auto *Pointee = T->getPointeeType()->castAs<FunctionType>();
65*0ea5af7aSJonas Toth     return countIndirections(
66*0ea5af7aSJonas Toth         Pointee->getReturnType().IgnoreParens().getTypePtr(), ++Indirections);
67*0ea5af7aSJonas Toth   }
68*0ea5af7aSJonas Toth 
69*0ea5af7aSJonas Toth   // Note: Do not increment the 'Indirections' because it is not yet clear
70*0ea5af7aSJonas Toth   // if there is an indirection added in the source code of the array
71*0ea5af7aSJonas Toth   // declaration.
72*0ea5af7aSJonas Toth   if (const auto *AT = dyn_cast<ArrayType>(T))
73*0ea5af7aSJonas Toth     return countIndirections(AT->getElementType().IgnoreParens().getTypePtr(),
74*0ea5af7aSJonas Toth                              Indirections);
75*0ea5af7aSJonas Toth 
76*0ea5af7aSJonas Toth   if (isa<PointerType>(T) || isa<ReferenceType>(T))
77*0ea5af7aSJonas Toth     return countIndirections(T->getPointeeType().IgnoreParens().getTypePtr(),
78*0ea5af7aSJonas Toth                              ++Indirections);
79*0ea5af7aSJonas Toth 
80*0ea5af7aSJonas Toth   return Indirections;
81*0ea5af7aSJonas Toth }
82*0ea5af7aSJonas Toth 
83*0ea5af7aSJonas Toth static bool typeIsMemberPointer(const Type *T) {
84*0ea5af7aSJonas Toth   if (isa<ArrayType>(T))
85*0ea5af7aSJonas Toth     return typeIsMemberPointer(T->getArrayElementTypeNoTypeQual());
86*0ea5af7aSJonas Toth 
87*0ea5af7aSJonas Toth   if ((isa<PointerType>(T) || isa<ReferenceType>(T)) &&
88*0ea5af7aSJonas Toth       isa<PointerType>(T->getPointeeType()))
89*0ea5af7aSJonas Toth     return typeIsMemberPointer(T->getPointeeType().getTypePtr());
90*0ea5af7aSJonas Toth 
91*0ea5af7aSJonas Toth   return isa<MemberPointerType>(T);
92*0ea5af7aSJonas Toth }
93*0ea5af7aSJonas Toth 
94*0ea5af7aSJonas Toth /// This function tries to extract the SourceRanges that make up all
95*0ea5af7aSJonas Toth /// declarations in this \c DeclStmt.
96*0ea5af7aSJonas Toth ///
97*0ea5af7aSJonas Toth /// The resulting vector has the structure {UnderlyingType, Decl1, Decl2, ...}.
98*0ea5af7aSJonas Toth /// Each \c SourceRange is of the form [Begin, End).
99*0ea5af7aSJonas Toth /// If any of the create ranges is invalid or in a macro the result will be
100*0ea5af7aSJonas Toth /// \c None.
101*0ea5af7aSJonas Toth /// If the \c DeclStmt contains only one declaration, the result is \c None.
102*0ea5af7aSJonas Toth /// If the \c DeclStmt contains declarations other than \c VarDecl the result
103*0ea5af7aSJonas Toth /// is \c None.
104*0ea5af7aSJonas Toth ///
105*0ea5af7aSJonas Toth /// \code
106*0ea5af7aSJonas Toth ///    int * ptr1 = nullptr, value = 42;
107*0ea5af7aSJonas Toth /// // [  ][              ] [         ] - The ranges here are inclusive
108*0ea5af7aSJonas Toth /// \endcode
109*0ea5af7aSJonas Toth /// \todo Generalize this function to take other declarations than \c VarDecl.
110*0ea5af7aSJonas Toth static Optional<std::vector<SourceRange>>
111*0ea5af7aSJonas Toth declRanges(const DeclStmt *DS, const SourceManager &SM,
112*0ea5af7aSJonas Toth            const LangOptions &LangOpts) {
113*0ea5af7aSJonas Toth   std::size_t DeclCount = std::distance(DS->decl_begin(), DS->decl_end());
114*0ea5af7aSJonas Toth   if (DeclCount < 2)
115*0ea5af7aSJonas Toth     return None;
116*0ea5af7aSJonas Toth 
117*0ea5af7aSJonas Toth   if (rangeContainsExpansionsOrDirectives(DS->getSourceRange(), SM, LangOpts))
118*0ea5af7aSJonas Toth     return None;
119*0ea5af7aSJonas Toth 
120*0ea5af7aSJonas Toth   // The initial type of the declaration and each declaration has it's own
121*0ea5af7aSJonas Toth   // slice. This is necessary, because pointers and references bind only
122*0ea5af7aSJonas Toth   // to the local variable and not to all variables in the declaration.
123*0ea5af7aSJonas Toth   // Example: 'int *pointer, value = 42;'
124*0ea5af7aSJonas Toth   std::vector<SourceRange> Slices;
125*0ea5af7aSJonas Toth   Slices.reserve(DeclCount + 1);
126*0ea5af7aSJonas Toth 
127*0ea5af7aSJonas Toth   // Calculate the first slice, for now only variables are handled but in the
128*0ea5af7aSJonas Toth   // future this should be relaxed and support various kinds of declarations.
129*0ea5af7aSJonas Toth   const auto *FirstDecl = dyn_cast<VarDecl>(*DS->decl_begin());
130*0ea5af7aSJonas Toth 
131*0ea5af7aSJonas Toth   if (FirstDecl == nullptr)
132*0ea5af7aSJonas Toth     return None;
133*0ea5af7aSJonas Toth 
134*0ea5af7aSJonas Toth   // FIXME: Member pointers are not transformed correctly right now, that's
135*0ea5af7aSJonas Toth   // why they are treated as problematic here.
136*0ea5af7aSJonas Toth   if (typeIsMemberPointer(FirstDecl->getType().IgnoreParens().getTypePtr()))
137*0ea5af7aSJonas Toth     return None;
138*0ea5af7aSJonas Toth 
139*0ea5af7aSJonas Toth   // Consider the following case: 'int * pointer, value = 42;'
140*0ea5af7aSJonas Toth   // Created slices (inclusive)    [  ][       ] [         ]
141*0ea5af7aSJonas Toth   // Because 'getBeginLoc' points to the start of the variable *name*, the
142*0ea5af7aSJonas Toth   // location of the pointer must be determined separatly.
143*0ea5af7aSJonas Toth   SourceLocation Start = findStartOfIndirection(
144*0ea5af7aSJonas Toth       FirstDecl->getLocation(),
145*0ea5af7aSJonas Toth       countIndirections(FirstDecl->getType().IgnoreParens().getTypePtr()), SM,
146*0ea5af7aSJonas Toth       LangOpts);
147*0ea5af7aSJonas Toth 
148*0ea5af7aSJonas Toth   // Fix function-pointer declarations that have a '(' in front of the
149*0ea5af7aSJonas Toth   // pointer.
150*0ea5af7aSJonas Toth   // Example: 'void (*f2)(int), (*g2)(int, float) = gg;'
151*0ea5af7aSJonas Toth   // Slices:   [   ][        ] [                     ]
152*0ea5af7aSJonas Toth   if (FirstDecl->getType()->isFunctionPointerType())
153*0ea5af7aSJonas Toth     Start = findPreviousTokenKind(Start, SM, LangOpts, tok::l_paren);
154*0ea5af7aSJonas Toth 
155*0ea5af7aSJonas Toth   // It is popssible that a declarator is wrapped with parens.
156*0ea5af7aSJonas Toth   // Example: 'float (((*f_ptr2)))[42], *f_ptr3, ((f_value2)) = 42.f;'
157*0ea5af7aSJonas Toth   // The slice for the type-part must not contain these parens. Consequently
158*0ea5af7aSJonas Toth   // 'Start' is moved to the most left paren if there are parens.
159*0ea5af7aSJonas Toth   while (true) {
160*0ea5af7aSJonas Toth     if (Start.isInvalid() || Start.isMacroID())
161*0ea5af7aSJonas Toth       break;
162*0ea5af7aSJonas Toth 
163*0ea5af7aSJonas Toth     Token T = getPreviousToken(Start, SM, LangOpts);
164*0ea5af7aSJonas Toth     if (T.is(tok::l_paren)) {
165*0ea5af7aSJonas Toth       Start = findPreviousTokenStart(Start, SM, LangOpts);
166*0ea5af7aSJonas Toth       continue;
167*0ea5af7aSJonas Toth     }
168*0ea5af7aSJonas Toth     break;
169*0ea5af7aSJonas Toth   }
170*0ea5af7aSJonas Toth 
171*0ea5af7aSJonas Toth   SourceRange DeclRange(DS->getBeginLoc(), Start);
172*0ea5af7aSJonas Toth   if (DeclRange.isInvalid() || isMacroID(DeclRange))
173*0ea5af7aSJonas Toth     return None;
174*0ea5af7aSJonas Toth 
175*0ea5af7aSJonas Toth   // The first slice, that is prepended to every isolated declaration, is
176*0ea5af7aSJonas Toth   // created.
177*0ea5af7aSJonas Toth   Slices.emplace_back(DeclRange);
178*0ea5af7aSJonas Toth 
179*0ea5af7aSJonas Toth   // Create all following slices that each declare a variable.
180*0ea5af7aSJonas Toth   SourceLocation DeclBegin = Start;
181*0ea5af7aSJonas Toth   for (const auto &Decl : DS->decls()) {
182*0ea5af7aSJonas Toth     const auto *CurrentDecl = cast<VarDecl>(Decl);
183*0ea5af7aSJonas Toth 
184*0ea5af7aSJonas Toth     // FIXME: Member pointers are not transformed correctly right now, that's
185*0ea5af7aSJonas Toth     // why they are treated as problematic here.
186*0ea5af7aSJonas Toth     if (typeIsMemberPointer(CurrentDecl->getType().IgnoreParens().getTypePtr()))
187*0ea5af7aSJonas Toth       return None;
188*0ea5af7aSJonas Toth 
189*0ea5af7aSJonas Toth     SourceLocation DeclEnd =
190*0ea5af7aSJonas Toth         CurrentDecl->hasInit()
191*0ea5af7aSJonas Toth             ? findNextTerminator(CurrentDecl->getInit()->getEndLoc(), SM,
192*0ea5af7aSJonas Toth                                  LangOpts)
193*0ea5af7aSJonas Toth             : findNextTerminator(CurrentDecl->getEndLoc(), SM, LangOpts);
194*0ea5af7aSJonas Toth 
195*0ea5af7aSJonas Toth     SourceRange VarNameRange(DeclBegin, DeclEnd);
196*0ea5af7aSJonas Toth     if (VarNameRange.isInvalid() || isMacroID(VarNameRange))
197*0ea5af7aSJonas Toth       return None;
198*0ea5af7aSJonas Toth 
199*0ea5af7aSJonas Toth     Slices.emplace_back(VarNameRange);
200*0ea5af7aSJonas Toth     DeclBegin = DeclEnd.getLocWithOffset(1);
201*0ea5af7aSJonas Toth   }
202*0ea5af7aSJonas Toth   return Slices;
203*0ea5af7aSJonas Toth }
204*0ea5af7aSJonas Toth 
205*0ea5af7aSJonas Toth static Optional<std::vector<StringRef>>
206*0ea5af7aSJonas Toth collectSourceRanges(llvm::ArrayRef<SourceRange> Ranges, const SourceManager &SM,
207*0ea5af7aSJonas Toth                     const LangOptions &LangOpts) {
208*0ea5af7aSJonas Toth   std::vector<StringRef> Snippets;
209*0ea5af7aSJonas Toth   Snippets.reserve(Ranges.size());
210*0ea5af7aSJonas Toth 
211*0ea5af7aSJonas Toth   for (const auto &Range : Ranges) {
212*0ea5af7aSJonas Toth     CharSourceRange CharRange = Lexer::getAsCharRange(
213*0ea5af7aSJonas Toth         CharSourceRange::getCharRange(Range.getBegin(), Range.getEnd()), SM,
214*0ea5af7aSJonas Toth         LangOpts);
215*0ea5af7aSJonas Toth 
216*0ea5af7aSJonas Toth     if (CharRange.isInvalid())
217*0ea5af7aSJonas Toth       return None;
218*0ea5af7aSJonas Toth 
219*0ea5af7aSJonas Toth     bool InvalidText = false;
220*0ea5af7aSJonas Toth     StringRef Snippet =
221*0ea5af7aSJonas Toth         Lexer::getSourceText(CharRange, SM, LangOpts, &InvalidText);
222*0ea5af7aSJonas Toth 
223*0ea5af7aSJonas Toth     if (InvalidText)
224*0ea5af7aSJonas Toth       return None;
225*0ea5af7aSJonas Toth 
226*0ea5af7aSJonas Toth     Snippets.emplace_back(Snippet);
227*0ea5af7aSJonas Toth   }
228*0ea5af7aSJonas Toth 
229*0ea5af7aSJonas Toth   return Snippets;
230*0ea5af7aSJonas Toth }
231*0ea5af7aSJonas Toth 
232*0ea5af7aSJonas Toth /// Expects a vector {TypeSnippet, Firstdecl, SecondDecl, ...}.
233*0ea5af7aSJonas Toth static std::vector<std::string>
234*0ea5af7aSJonas Toth createIsolatedDecls(llvm::ArrayRef<StringRef> Snippets) {
235*0ea5af7aSJonas Toth   // The first section is the type snippet, which does not make a decl itself.
236*0ea5af7aSJonas Toth   assert(Snippets.size() > 2 && "Not enough snippets to create isolated decls");
237*0ea5af7aSJonas Toth   std::vector<std::string> Decls(Snippets.size() - 1);
238*0ea5af7aSJonas Toth 
239*0ea5af7aSJonas Toth   for (std::size_t I = 1; I < Snippets.size(); ++I)
240*0ea5af7aSJonas Toth     Decls[I - 1] = Twine(Snippets[0])
241*0ea5af7aSJonas Toth                        .concat(Snippets[0].endswith(" ") ? "" : " ")
242*0ea5af7aSJonas Toth                        .concat(Snippets[I].ltrim())
243*0ea5af7aSJonas Toth                        .concat(";")
244*0ea5af7aSJonas Toth                        .str();
245*0ea5af7aSJonas Toth 
246*0ea5af7aSJonas Toth   return Decls;
247*0ea5af7aSJonas Toth }
248*0ea5af7aSJonas Toth 
249*0ea5af7aSJonas Toth void IsolateDeclarationCheck::check(const MatchFinder::MatchResult &Result) {
250*0ea5af7aSJonas Toth   const auto *WholeDecl = Result.Nodes.getNodeAs<DeclStmt>("decl_stmt");
251*0ea5af7aSJonas Toth 
252*0ea5af7aSJonas Toth   auto Diag =
253*0ea5af7aSJonas Toth       diag(WholeDecl->getBeginLoc(),
254*0ea5af7aSJonas Toth            "multiple declarations in a single statement reduces readability");
255*0ea5af7aSJonas Toth 
256*0ea5af7aSJonas Toth   Optional<std::vector<SourceRange>> PotentialRanges =
257*0ea5af7aSJonas Toth       declRanges(WholeDecl, *Result.SourceManager, getLangOpts());
258*0ea5af7aSJonas Toth   if (!PotentialRanges)
259*0ea5af7aSJonas Toth     return;
260*0ea5af7aSJonas Toth 
261*0ea5af7aSJonas Toth   Optional<std::vector<StringRef>> PotentialSnippets = collectSourceRanges(
262*0ea5af7aSJonas Toth       *PotentialRanges, *Result.SourceManager, getLangOpts());
263*0ea5af7aSJonas Toth 
264*0ea5af7aSJonas Toth   if (!PotentialSnippets)
265*0ea5af7aSJonas Toth     return;
266*0ea5af7aSJonas Toth 
267*0ea5af7aSJonas Toth   std::vector<std::string> NewDecls = createIsolatedDecls(*PotentialSnippets);
268*0ea5af7aSJonas Toth   std::string Replacement = llvm::join(
269*0ea5af7aSJonas Toth       NewDecls,
270*0ea5af7aSJonas Toth       (Twine("\n") + Lexer::getIndentationForLine(WholeDecl->getBeginLoc(),
271*0ea5af7aSJonas Toth                                                   *Result.SourceManager))
272*0ea5af7aSJonas Toth           .str());
273*0ea5af7aSJonas Toth 
274*0ea5af7aSJonas Toth   Diag << FixItHint::CreateReplacement(WholeDecl->getSourceRange(),
275*0ea5af7aSJonas Toth                                        Replacement);
276*0ea5af7aSJonas Toth }
277*0ea5af7aSJonas Toth } // namespace readability
278*0ea5af7aSJonas Toth } // namespace tidy
279*0ea5af7aSJonas Toth } // namespace clang
280