xref: /llvm-project/clang-tools-extra/clang-tidy/readability/IsolateDeclarationCheck.cpp (revision 76bbbcb41bcf4a1d7a26bb11b78cf97b60ea7d4b)
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