xref: /llvm-project/clang-tools-extra/clang-tidy/readability/IsolateDeclarationCheck.cpp (revision 976e0c07a04ff2e6df2a1b527cdcc95b9a961041)
10ea5af7aSJonas Toth //===--- IsolateDeclarationCheck.cpp - clang-tidy -------------------------===//
20ea5af7aSJonas Toth //
30ea5af7aSJonas Toth //                     The LLVM Compiler Infrastructure
40ea5af7aSJonas Toth //
50ea5af7aSJonas Toth // This file is distributed under the University of Illinois Open Source
60ea5af7aSJonas Toth // License. See LICENSE.TXT for details.
70ea5af7aSJonas Toth //
80ea5af7aSJonas Toth //===----------------------------------------------------------------------===//
90ea5af7aSJonas Toth 
100ea5af7aSJonas Toth #include "IsolateDeclarationCheck.h"
110ea5af7aSJonas Toth #include "../utils/LexerUtils.h"
120ea5af7aSJonas Toth #include "clang/ASTMatchers/ASTMatchFinder.h"
130ea5af7aSJonas Toth 
140ea5af7aSJonas Toth using namespace clang::ast_matchers;
150ea5af7aSJonas Toth using namespace clang::tidy::utils::lexer;
160ea5af7aSJonas Toth 
170ea5af7aSJonas Toth namespace clang {
180ea5af7aSJonas Toth namespace tidy {
190ea5af7aSJonas Toth namespace readability {
200ea5af7aSJonas Toth 
210ea5af7aSJonas Toth namespace {
220ea5af7aSJonas Toth AST_MATCHER(DeclStmt, isSingleDecl) { return Node.isSingleDecl(); }
230ea5af7aSJonas Toth AST_MATCHER(DeclStmt, onlyDeclaresVariables) {
240ea5af7aSJonas Toth   return llvm::all_of(Node.decls(), [](Decl *D) { return isa<VarDecl>(D); });
250ea5af7aSJonas Toth }
260ea5af7aSJonas Toth } // namespace
270ea5af7aSJonas Toth 
280ea5af7aSJonas Toth void IsolateDeclarationCheck::registerMatchers(MatchFinder *Finder) {
29*976e0c07SAlexander Kornienko   Finder->addMatcher(declStmt(onlyDeclaresVariables(), unless(isSingleDecl()),
30*976e0c07SAlexander Kornienko                               hasParent(compoundStmt()))
310ea5af7aSJonas Toth                          .bind("decl_stmt"),
320ea5af7aSJonas Toth                      this);
330ea5af7aSJonas Toth }
340ea5af7aSJonas Toth 
350ea5af7aSJonas Toth static SourceLocation findStartOfIndirection(SourceLocation Start,
360ea5af7aSJonas Toth                                              int Indirections,
370ea5af7aSJonas Toth                                              const SourceManager &SM,
380ea5af7aSJonas Toth                                              const LangOptions &LangOpts) {
390ea5af7aSJonas Toth   assert(Indirections >= 0 && "Indirections must be non-negative");
400ea5af7aSJonas Toth   if (Indirections == 0)
410ea5af7aSJonas Toth     return Start;
420ea5af7aSJonas Toth 
430ea5af7aSJonas Toth   // Note that the post-fix decrement is necessary to perform the correct
440ea5af7aSJonas Toth   // number of transformations.
450ea5af7aSJonas Toth   while (Indirections-- != 0) {
460ea5af7aSJonas Toth     Start = findPreviousAnyTokenKind(Start, SM, LangOpts, tok::star, tok::amp);
470ea5af7aSJonas Toth     if (Start.isInvalid() || Start.isMacroID())
480ea5af7aSJonas Toth       return SourceLocation();
490ea5af7aSJonas Toth   }
500ea5af7aSJonas Toth   return Start;
510ea5af7aSJonas Toth }
520ea5af7aSJonas Toth 
530ea5af7aSJonas Toth static bool isMacroID(SourceRange R) {
540ea5af7aSJonas Toth   return R.getBegin().isMacroID() || R.getEnd().isMacroID();
550ea5af7aSJonas Toth }
560ea5af7aSJonas Toth 
570ea5af7aSJonas Toth /// This function counts the number of written indirections for the given
580ea5af7aSJonas Toth /// Type \p T. It does \b NOT resolve typedefs as it's a helper for lexing
590ea5af7aSJonas Toth /// the source code.
600ea5af7aSJonas Toth /// \see declRanges
610ea5af7aSJonas Toth static int countIndirections(const Type *T, int Indirections = 0) {
620ea5af7aSJonas Toth   if (T->isFunctionPointerType()) {
630ea5af7aSJonas Toth     const auto *Pointee = T->getPointeeType()->castAs<FunctionType>();
640ea5af7aSJonas Toth     return countIndirections(
650ea5af7aSJonas Toth         Pointee->getReturnType().IgnoreParens().getTypePtr(), ++Indirections);
660ea5af7aSJonas Toth   }
670ea5af7aSJonas Toth 
680ea5af7aSJonas Toth   // Note: Do not increment the 'Indirections' because it is not yet clear
690ea5af7aSJonas Toth   // if there is an indirection added in the source code of the array
700ea5af7aSJonas Toth   // declaration.
710ea5af7aSJonas Toth   if (const auto *AT = dyn_cast<ArrayType>(T))
720ea5af7aSJonas Toth     return countIndirections(AT->getElementType().IgnoreParens().getTypePtr(),
730ea5af7aSJonas Toth                              Indirections);
740ea5af7aSJonas Toth 
750ea5af7aSJonas Toth   if (isa<PointerType>(T) || isa<ReferenceType>(T))
760ea5af7aSJonas Toth     return countIndirections(T->getPointeeType().IgnoreParens().getTypePtr(),
770ea5af7aSJonas Toth                              ++Indirections);
780ea5af7aSJonas Toth 
790ea5af7aSJonas Toth   return Indirections;
800ea5af7aSJonas Toth }
810ea5af7aSJonas Toth 
820ea5af7aSJonas Toth static bool typeIsMemberPointer(const Type *T) {
830ea5af7aSJonas Toth   if (isa<ArrayType>(T))
840ea5af7aSJonas Toth     return typeIsMemberPointer(T->getArrayElementTypeNoTypeQual());
850ea5af7aSJonas Toth 
860ea5af7aSJonas Toth   if ((isa<PointerType>(T) || isa<ReferenceType>(T)) &&
870ea5af7aSJonas Toth       isa<PointerType>(T->getPointeeType()))
880ea5af7aSJonas Toth     return typeIsMemberPointer(T->getPointeeType().getTypePtr());
890ea5af7aSJonas Toth 
900ea5af7aSJonas Toth   return isa<MemberPointerType>(T);
910ea5af7aSJonas Toth }
920ea5af7aSJonas Toth 
930ea5af7aSJonas Toth /// This function tries to extract the SourceRanges that make up all
940ea5af7aSJonas Toth /// declarations in this \c DeclStmt.
950ea5af7aSJonas Toth ///
960ea5af7aSJonas Toth /// The resulting vector has the structure {UnderlyingType, Decl1, Decl2, ...}.
970ea5af7aSJonas Toth /// Each \c SourceRange is of the form [Begin, End).
980ea5af7aSJonas Toth /// If any of the create ranges is invalid or in a macro the result will be
990ea5af7aSJonas Toth /// \c None.
1000ea5af7aSJonas Toth /// If the \c DeclStmt contains only one declaration, the result is \c None.
1010ea5af7aSJonas Toth /// If the \c DeclStmt contains declarations other than \c VarDecl the result
1020ea5af7aSJonas Toth /// is \c None.
1030ea5af7aSJonas Toth ///
1040ea5af7aSJonas Toth /// \code
1050ea5af7aSJonas Toth ///    int * ptr1 = nullptr, value = 42;
1060ea5af7aSJonas Toth /// // [  ][              ] [         ] - The ranges here are inclusive
1070ea5af7aSJonas Toth /// \endcode
1080ea5af7aSJonas Toth /// \todo Generalize this function to take other declarations than \c VarDecl.
1090ea5af7aSJonas Toth static Optional<std::vector<SourceRange>>
1100ea5af7aSJonas Toth declRanges(const DeclStmt *DS, const SourceManager &SM,
1110ea5af7aSJonas Toth            const LangOptions &LangOpts) {
1120ea5af7aSJonas Toth   std::size_t DeclCount = std::distance(DS->decl_begin(), DS->decl_end());
1130ea5af7aSJonas Toth   if (DeclCount < 2)
1140ea5af7aSJonas Toth     return None;
1150ea5af7aSJonas Toth 
1160ea5af7aSJonas Toth   if (rangeContainsExpansionsOrDirectives(DS->getSourceRange(), SM, LangOpts))
1170ea5af7aSJonas Toth     return None;
1180ea5af7aSJonas Toth 
1190ea5af7aSJonas Toth   // The initial type of the declaration and each declaration has it's own
1200ea5af7aSJonas Toth   // slice. This is necessary, because pointers and references bind only
1210ea5af7aSJonas Toth   // to the local variable and not to all variables in the declaration.
1220ea5af7aSJonas Toth   // Example: 'int *pointer, value = 42;'
1230ea5af7aSJonas Toth   std::vector<SourceRange> Slices;
1240ea5af7aSJonas Toth   Slices.reserve(DeclCount + 1);
1250ea5af7aSJonas Toth 
1260ea5af7aSJonas Toth   // Calculate the first slice, for now only variables are handled but in the
1270ea5af7aSJonas Toth   // future this should be relaxed and support various kinds of declarations.
1280ea5af7aSJonas Toth   const auto *FirstDecl = dyn_cast<VarDecl>(*DS->decl_begin());
1290ea5af7aSJonas Toth 
1300ea5af7aSJonas Toth   if (FirstDecl == nullptr)
1310ea5af7aSJonas Toth     return None;
1320ea5af7aSJonas Toth 
1330ea5af7aSJonas Toth   // FIXME: Member pointers are not transformed correctly right now, that's
1340ea5af7aSJonas Toth   // why they are treated as problematic here.
1350ea5af7aSJonas Toth   if (typeIsMemberPointer(FirstDecl->getType().IgnoreParens().getTypePtr()))
1360ea5af7aSJonas Toth     return None;
1370ea5af7aSJonas Toth 
1380ea5af7aSJonas Toth   // Consider the following case: 'int * pointer, value = 42;'
1390ea5af7aSJonas Toth   // Created slices (inclusive)    [  ][       ] [         ]
1400ea5af7aSJonas Toth   // Because 'getBeginLoc' points to the start of the variable *name*, the
1410ea5af7aSJonas Toth   // location of the pointer must be determined separatly.
1420ea5af7aSJonas Toth   SourceLocation Start = findStartOfIndirection(
1430ea5af7aSJonas Toth       FirstDecl->getLocation(),
1440ea5af7aSJonas Toth       countIndirections(FirstDecl->getType().IgnoreParens().getTypePtr()), SM,
1450ea5af7aSJonas Toth       LangOpts);
1460ea5af7aSJonas Toth 
1470ea5af7aSJonas Toth   // Fix function-pointer declarations that have a '(' in front of the
1480ea5af7aSJonas Toth   // pointer.
1490ea5af7aSJonas Toth   // Example: 'void (*f2)(int), (*g2)(int, float) = gg;'
1500ea5af7aSJonas Toth   // Slices:   [   ][        ] [                     ]
1510ea5af7aSJonas Toth   if (FirstDecl->getType()->isFunctionPointerType())
1520ea5af7aSJonas Toth     Start = findPreviousTokenKind(Start, SM, LangOpts, tok::l_paren);
1530ea5af7aSJonas Toth 
1540ea5af7aSJonas Toth   // It is popssible that a declarator is wrapped with parens.
1550ea5af7aSJonas Toth   // Example: 'float (((*f_ptr2)))[42], *f_ptr3, ((f_value2)) = 42.f;'
1560ea5af7aSJonas Toth   // The slice for the type-part must not contain these parens. Consequently
1570ea5af7aSJonas Toth   // 'Start' is moved to the most left paren if there are parens.
1580ea5af7aSJonas Toth   while (true) {
1590ea5af7aSJonas Toth     if (Start.isInvalid() || Start.isMacroID())
1600ea5af7aSJonas Toth       break;
1610ea5af7aSJonas Toth 
1620ea5af7aSJonas Toth     Token T = getPreviousToken(Start, SM, LangOpts);
1630ea5af7aSJonas Toth     if (T.is(tok::l_paren)) {
1640ea5af7aSJonas Toth       Start = findPreviousTokenStart(Start, SM, LangOpts);
1650ea5af7aSJonas Toth       continue;
1660ea5af7aSJonas Toth     }
1670ea5af7aSJonas Toth     break;
1680ea5af7aSJonas Toth   }
1690ea5af7aSJonas Toth 
1700ea5af7aSJonas Toth   SourceRange DeclRange(DS->getBeginLoc(), Start);
1710ea5af7aSJonas Toth   if (DeclRange.isInvalid() || isMacroID(DeclRange))
1720ea5af7aSJonas Toth     return None;
1730ea5af7aSJonas Toth 
1740ea5af7aSJonas Toth   // The first slice, that is prepended to every isolated declaration, is
1750ea5af7aSJonas Toth   // created.
1760ea5af7aSJonas Toth   Slices.emplace_back(DeclRange);
1770ea5af7aSJonas Toth 
1780ea5af7aSJonas Toth   // Create all following slices that each declare a variable.
1790ea5af7aSJonas Toth   SourceLocation DeclBegin = Start;
1800ea5af7aSJonas Toth   for (const auto &Decl : DS->decls()) {
1810ea5af7aSJonas Toth     const auto *CurrentDecl = cast<VarDecl>(Decl);
1820ea5af7aSJonas Toth 
1830ea5af7aSJonas Toth     // FIXME: Member pointers are not transformed correctly right now, that's
1840ea5af7aSJonas Toth     // why they are treated as problematic here.
1850ea5af7aSJonas Toth     if (typeIsMemberPointer(CurrentDecl->getType().IgnoreParens().getTypePtr()))
1860ea5af7aSJonas Toth       return None;
1870ea5af7aSJonas Toth 
1880ea5af7aSJonas Toth     SourceLocation DeclEnd =
1890ea5af7aSJonas Toth         CurrentDecl->hasInit()
1900ea5af7aSJonas Toth             ? findNextTerminator(CurrentDecl->getInit()->getEndLoc(), SM,
1910ea5af7aSJonas Toth                                  LangOpts)
1920ea5af7aSJonas Toth             : findNextTerminator(CurrentDecl->getEndLoc(), SM, LangOpts);
1930ea5af7aSJonas Toth 
1940ea5af7aSJonas Toth     SourceRange VarNameRange(DeclBegin, DeclEnd);
1950ea5af7aSJonas Toth     if (VarNameRange.isInvalid() || isMacroID(VarNameRange))
1960ea5af7aSJonas Toth       return None;
1970ea5af7aSJonas Toth 
1980ea5af7aSJonas Toth     Slices.emplace_back(VarNameRange);
1990ea5af7aSJonas Toth     DeclBegin = DeclEnd.getLocWithOffset(1);
2000ea5af7aSJonas Toth   }
2010ea5af7aSJonas Toth   return Slices;
2020ea5af7aSJonas Toth }
2030ea5af7aSJonas Toth 
2040ea5af7aSJonas Toth static Optional<std::vector<StringRef>>
2050ea5af7aSJonas Toth collectSourceRanges(llvm::ArrayRef<SourceRange> Ranges, const SourceManager &SM,
2060ea5af7aSJonas Toth                     const LangOptions &LangOpts) {
2070ea5af7aSJonas Toth   std::vector<StringRef> Snippets;
2080ea5af7aSJonas Toth   Snippets.reserve(Ranges.size());
2090ea5af7aSJonas Toth 
2100ea5af7aSJonas Toth   for (const auto &Range : Ranges) {
2110ea5af7aSJonas Toth     CharSourceRange CharRange = Lexer::getAsCharRange(
2120ea5af7aSJonas Toth         CharSourceRange::getCharRange(Range.getBegin(), Range.getEnd()), SM,
2130ea5af7aSJonas Toth         LangOpts);
2140ea5af7aSJonas Toth 
2150ea5af7aSJonas Toth     if (CharRange.isInvalid())
2160ea5af7aSJonas Toth       return None;
2170ea5af7aSJonas Toth 
2180ea5af7aSJonas Toth     bool InvalidText = false;
2190ea5af7aSJonas Toth     StringRef Snippet =
2200ea5af7aSJonas Toth         Lexer::getSourceText(CharRange, SM, LangOpts, &InvalidText);
2210ea5af7aSJonas Toth 
2220ea5af7aSJonas Toth     if (InvalidText)
2230ea5af7aSJonas Toth       return None;
2240ea5af7aSJonas Toth 
2250ea5af7aSJonas Toth     Snippets.emplace_back(Snippet);
2260ea5af7aSJonas Toth   }
2270ea5af7aSJonas Toth 
2280ea5af7aSJonas Toth   return Snippets;
2290ea5af7aSJonas Toth }
2300ea5af7aSJonas Toth 
2310ea5af7aSJonas Toth /// Expects a vector {TypeSnippet, Firstdecl, SecondDecl, ...}.
2320ea5af7aSJonas Toth static std::vector<std::string>
2330ea5af7aSJonas Toth createIsolatedDecls(llvm::ArrayRef<StringRef> Snippets) {
2340ea5af7aSJonas Toth   // The first section is the type snippet, which does not make a decl itself.
2350ea5af7aSJonas Toth   assert(Snippets.size() > 2 && "Not enough snippets to create isolated decls");
2360ea5af7aSJonas Toth   std::vector<std::string> Decls(Snippets.size() - 1);
2370ea5af7aSJonas Toth 
2380ea5af7aSJonas Toth   for (std::size_t I = 1; I < Snippets.size(); ++I)
2390ea5af7aSJonas Toth     Decls[I - 1] = Twine(Snippets[0])
2400ea5af7aSJonas Toth                        .concat(Snippets[0].endswith(" ") ? "" : " ")
2410ea5af7aSJonas Toth                        .concat(Snippets[I].ltrim())
2420ea5af7aSJonas Toth                        .concat(";")
2430ea5af7aSJonas Toth                        .str();
2440ea5af7aSJonas Toth 
2450ea5af7aSJonas Toth   return Decls;
2460ea5af7aSJonas Toth }
2470ea5af7aSJonas Toth 
2480ea5af7aSJonas Toth void IsolateDeclarationCheck::check(const MatchFinder::MatchResult &Result) {
2490ea5af7aSJonas Toth   const auto *WholeDecl = Result.Nodes.getNodeAs<DeclStmt>("decl_stmt");
2500ea5af7aSJonas Toth 
2510ea5af7aSJonas Toth   auto Diag =
2520ea5af7aSJonas Toth       diag(WholeDecl->getBeginLoc(),
2530ea5af7aSJonas Toth            "multiple declarations in a single statement reduces readability");
2540ea5af7aSJonas Toth 
2550ea5af7aSJonas Toth   Optional<std::vector<SourceRange>> PotentialRanges =
2560ea5af7aSJonas Toth       declRanges(WholeDecl, *Result.SourceManager, getLangOpts());
2570ea5af7aSJonas Toth   if (!PotentialRanges)
2580ea5af7aSJonas Toth     return;
2590ea5af7aSJonas Toth 
2600ea5af7aSJonas Toth   Optional<std::vector<StringRef>> PotentialSnippets = collectSourceRanges(
2610ea5af7aSJonas Toth       *PotentialRanges, *Result.SourceManager, getLangOpts());
2620ea5af7aSJonas Toth 
2630ea5af7aSJonas Toth   if (!PotentialSnippets)
2640ea5af7aSJonas Toth     return;
2650ea5af7aSJonas Toth 
2660ea5af7aSJonas Toth   std::vector<std::string> NewDecls = createIsolatedDecls(*PotentialSnippets);
2670ea5af7aSJonas Toth   std::string Replacement = llvm::join(
2680ea5af7aSJonas Toth       NewDecls,
2690ea5af7aSJonas Toth       (Twine("\n") + Lexer::getIndentationForLine(WholeDecl->getBeginLoc(),
2700ea5af7aSJonas Toth                                                   *Result.SourceManager))
2710ea5af7aSJonas Toth           .str());
2720ea5af7aSJonas Toth 
2730ea5af7aSJonas Toth   Diag << FixItHint::CreateReplacement(WholeDecl->getSourceRange(),
2740ea5af7aSJonas Toth                                        Replacement);
2750ea5af7aSJonas Toth }
2760ea5af7aSJonas Toth } // namespace readability
2770ea5af7aSJonas Toth } // namespace tidy
2780ea5af7aSJonas Toth } // namespace clang
279