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