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