Index: clang-tidy/readability/CMakeLists.txt =================================================================== --- clang-tidy/readability/CMakeLists.txt +++ clang-tidy/readability/CMakeLists.txt @@ -11,6 +11,7 @@ IdentifierNamingCheck.cpp ImplicitBoolConversionCheck.cpp InconsistentDeclarationParameterNameCheck.cpp + IsolateDeclarationCheck.cpp MagicNumbersCheck.cpp MisleadingIndentationCheck.cpp MisplacedArrayIndexCheck.cpp Index: clang-tidy/readability/IsolateDeclarationCheck.h =================================================================== --- /dev/null +++ clang-tidy/readability/IsolateDeclarationCheck.h @@ -0,0 +1,36 @@ +//===--- IsolateDeclarationCheck.h - clang-tidy -----------------*- C++ -*-===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// + +#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_ISOLATEDECLCHECK_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_ISOLATEDECLCHECK_H + +#include "../ClangTidy.h" + +namespace clang { +namespace tidy { +namespace readability { + +/// This check diagnoses all DeclStmt's declaring more than one variable and +/// tries to refactor the code to one statement per declaration. +/// +/// For the user-facing documentation see: +/// http://clang.llvm.org/extra/clang-tidy/checks/readability-isolate-declaration.html +class IsolateDeclarationCheck : public ClangTidyCheck { +public: + IsolateDeclarationCheck(StringRef Name, ClangTidyContext *Context) + : ClangTidyCheck(Name, Context) {} + void registerMatchers(ast_matchers::MatchFinder *Finder) override; + void check(const ast_matchers::MatchFinder::MatchResult &Result) override; +}; + +} // namespace readability +} // namespace tidy +} // namespace clang + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_ISOLATEDECLCHECK_H Index: clang-tidy/readability/IsolateDeclarationCheck.cpp =================================================================== --- /dev/null +++ clang-tidy/readability/IsolateDeclarationCheck.cpp @@ -0,0 +1,286 @@ +//===--- IsolateDeclarationCheck.cpp - clang-tidy -------------------------===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// + +#include "IsolateDeclarationCheck.h" +#include "../utils/LexerUtils.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" + +using namespace clang::ast_matchers; +using namespace clang::tidy::utils::lexer; + +namespace clang { +namespace tidy { +namespace readability { + +namespace { +AST_MATCHER(DeclStmt, isSingleDecl) { return Node.isSingleDecl(); } +AST_MATCHER(DeclStmt, onlyDeclaresVariables) { + return llvm::all_of(Node.decls(), [](Decl *D) { return isa(D); }); +} + +AST_MATCHER_P(IfStmt, hasInit, ast_matchers::internal::Matcher, + InnerMatcher) { + return InitStmt ? InnerMatcher.matches(*Node.getInit(), Finder, Builder) + : false; +} +} // namespace + +void IsolateDeclarationCheck::registerMatchers(MatchFinder *Finder) { + Finder->addMatcher( + declStmt(allOf(onlyDeclaresVariables(), unless(isSingleDecl()), + hasParent(compoundStmt()))) + .bind("decl_stmt"), + this); +} + +static SourceLocation findStartOfIndirection(SourceLocation Start, + int Indirections, + const SourceManager &SM, + const LangOptions &LangOpts) { + assert(Indirections >= 0 && "Indirections must be non-negative"); + if (Indirections == 0) + return Start; + + // Note that the post-fix decrement is necessary to perform the correct + // number of transformations. + while (Indirections-- != 0) { + Start = findPreviousAnyTokenKind(Start, SM, LangOpts, tok::star, tok::amp); + if (Start.isInvalid() || Start.isMacroID()) + return SourceLocation(); + } + return Start; +} + +static bool isMacroID(SourceRange R) { + return R.getBegin().isMacroID() || R.getEnd().isMacroID(); +} + +/// This function counts the number of written indirections for the given +/// Type \p T. It does \b NOT resolve typedefs as it's a helper for lexing +/// the source code. +/// \see declRanges +static int countIndirections(const Type *T, int Indirections = 0) { + if (isa(T) && T->isFunctionPointerType()) { + const auto *Pointee = + T->getPointeeType().getTypePtr()->castAs(); + return countIndirections( + Pointee->getReturnType().IgnoreParens().getTypePtr(), ++Indirections); + } + + // Note: Do not increment the 'Indirections' because it is not yet clear + // if there is an indirection added in the source code of the array + // declaration. + if (const auto *AT = dyn_cast(T)) + return countIndirections(AT->getElementType().IgnoreParens().getTypePtr(), + Indirections); + + if (isa(T) || isa(T)) + return countIndirections(T->getPointeeType().IgnoreParens().getTypePtr(), + ++Indirections); + + return Indirections; +} + +static bool typeIsMemberPointer(const Type *T) { + if (isa(T)) + return typeIsMemberPointer(T->getArrayElementTypeNoTypeQual()); + + if ((isa(T) || isa(T)) && + isa(T->getPointeeType())) + return typeIsMemberPointer(T->getPointeeType().getTypePtr()); + + return isa(T); +} + +/// This function tries to extract the SourceRanges that make up all +/// declarations in this \c DeclStmt. +/// +/// The resulting vector has the structure {UnderlyingType, Decl1, Decl2, ...}. +/// Each \c SourceRange is of the form [Begin, End). +/// If any of the create ranges is invalid or in a macro the result will be +/// \c None. +/// If the \c DeclStmt contains only one declaration, the result is \c None. +/// If the \c DeclStmt contains declarations other than \c VarDecl the result +/// is \c None. +/// +/// \code +/// int * ptr1 = nullptr, value = 42; +/// // [ ][ ] [ ] - The ranges here are inclusive +/// \endcode +/// \todo Generalize this function to take other declarations than \c VarDecl. +static Optional> +declRanges(const DeclStmt *DS, const SourceManager &SM, + const LangOptions &LangOpts) { + std::size_t DeclCount = std::distance(DS->decl_begin(), DS->decl_end()); + if (DeclCount < 2) + return None; + + if (!saveFromPreProcessor(DS->getSourceRange(), SM, LangOpts)) + return None; + + // The initial type of the declaration and each declaration has it's own + // slice. This is necessary, because pointers and references bind only + // to the local variable and not to all variables in the declaration. + // Example: 'int *pointer, value = 42;' + std::vector Slices; + Slices.reserve(DeclCount + 1); + + // Calculate the first slice, for now only variables are handled but in the + // future this should be relaxed and support various kinds of declarations. + const auto *FirstDecl = dyn_cast(*DS->decl_begin()); + + if (FirstDecl == nullptr) + return None; + + // FIXME: Member pointer are not transformed correctly right now, that's + // why they are treated as problematic here. + if (typeIsMemberPointer(FirstDecl->getType().IgnoreParens().getTypePtr())) + return None; + + // Consider the following case: 'int * pointer, value = 42;' + // Created slices (inclusive) [ ][ ] [ ] + // Because 'getBeginLoc' points to the start of the variable *name*, the + // location of the pointer must be determined separatly. + SourceLocation Start = findStartOfIndirection( + FirstDecl->getLocation(), + countIndirections(FirstDecl->getType().IgnoreParens().getTypePtr()), SM, + LangOpts); + + // Fix function-pointer declarations that have a '(' in front of the + // pointer. + // Example: 'void (*f2)(int), (*g2)(int, float) = gg;' + // Slices: [ ][ ] [ ] + if (FirstDecl->getType()->isFunctionPointerType()) + Start = findPreviousTokenKind(Start, SM, LangOpts, tok::l_paren); + + // It is popssible that a declarator is wrapped with parens. + // Example: 'float (((*f_ptr2)))[42], *f_ptr3, ((f_value2)) = 42.f;' + // The slice for the type-part must not contain these parens. Consequently + // 'Start' is moved to the most left paren if there are parens. + while (true) { + if (Start.isInvalid() || Start.isMacroID()) + break; + + Token T = getPreviousToken(Start, SM, LangOpts); + if (T.is(tok::l_paren)) { + Start = findPreviousTokenStart(Start, SM, LangOpts); + continue; + } + break; + } + + SourceRange DeclRange(DS->getBeginLoc(), Start); + if (DeclRange.isInvalid() || isMacroID(DeclRange)) + return None; + + // The first slice, that is prepended to every isolated declaration, is + // created. + Slices.emplace_back(DeclRange); + + // Create all following slices that each declare a variable. + SourceLocation DeclBegin = Start; + for (const auto &Decl : DS->decls()) { + const auto *CurrentDecl = cast(Decl); + + // FIXME: Member pointer are not transformed correctly right now, that's + // why they are treated as problematic here. + if (typeIsMemberPointer(CurrentDecl->getType().IgnoreParens().getTypePtr())) + return None; + + SourceLocation DeclEnd = + CurrentDecl->hasInit() + ? findNextTerminator(CurrentDecl->getInit()->getEndLoc(), SM, + LangOpts) + : findNextTerminator(CurrentDecl->getEndLoc(), SM, LangOpts); + + SourceRange VarNameRange(DeclBegin, DeclEnd); + if (VarNameRange.isInvalid() || isMacroID(VarNameRange)) + return None; + + Slices.emplace_back(VarNameRange); + DeclBegin = DeclEnd.getLocWithOffset(1); + } + return Slices; +} + +static Optional> +collectSourceRanges(llvm::ArrayRef Ranges, const SourceManager &SM, + const LangOptions &LangOpts) { + std::vector Snippets; + Snippets.reserve(Ranges.size()); + + for (const auto &Range : Ranges) { + CharSourceRange CharRange = Lexer::getAsCharRange( + CharSourceRange::getCharRange(Range.getBegin(), Range.getEnd()), SM, + LangOpts); + + if (CharRange.isInvalid()) + return None; + + bool InvalidText = false; + StringRef Snippet = + Lexer::getSourceText(CharRange, SM, LangOpts, &InvalidText); + + if (InvalidText) + return None; + + Snippets.emplace_back(Snippet); + } + + return Snippets; +} + +/// Expects a vector {TypeSnippet, Firstdecl, SecondDecl, ...}. +static std::vector +createIsolatedDecls(llvm::ArrayRef Snippets) { + // The first section is the type snippet, which does not make a decl itself. + assert(Snippets.size() > 2 && "Not enough snippets to create isolated decls"); + std::vector Decls(Snippets.size() - 1); + + for (std::size_t I = 1; I < Snippets.size(); ++I) + Decls[I - 1] = Twine(Snippets[0]) + .concat(Snippets[0].endswith(" ") ? "" : " ") + .concat(Snippets[I].ltrim()) + .concat(";") + .str(); + + return Decls; +} + +void IsolateDeclarationCheck::check(const MatchFinder::MatchResult &Result) { + const auto *WholeDecl = Result.Nodes.getNodeAs("decl_stmt"); + + auto Diag = + diag(WholeDecl->getBeginLoc(), + "multiple declarations in a single statement reduces readability"); + + Optional> PotentialRanges = + declRanges(WholeDecl, *Result.SourceManager, getLangOpts()); + if (!PotentialRanges) + return; + + Optional> PotentialSnippets = collectSourceRanges( + *PotentialRanges, *Result.SourceManager, getLangOpts()); + + if (!PotentialSnippets) + return; + + std::vector NewDecls = createIsolatedDecls(*PotentialSnippets); + std::string Replacement = llvm::join( + NewDecls, + (Twine("\n") + Lexer::getIndentationForLine(WholeDecl->getBeginLoc(), + *Result.SourceManager)) + .str()); + + Diag << FixItHint::CreateReplacement(WholeDecl->getSourceRange(), + Replacement); +} +} // namespace readability +} // namespace tidy +} // namespace clang Index: clang-tidy/readability/ReadabilityTidyModule.cpp =================================================================== --- clang-tidy/readability/ReadabilityTidyModule.cpp +++ clang-tidy/readability/ReadabilityTidyModule.cpp @@ -20,6 +20,7 @@ #include "IdentifierNamingCheck.h" #include "ImplicitBoolConversionCheck.h" #include "InconsistentDeclarationParameterNameCheck.h" +#include "IsolateDeclarationCheck.h" #include "MagicNumbersCheck.h" #include "MisleadingIndentationCheck.h" #include "MisplacedArrayIndexCheck.h" @@ -66,6 +67,8 @@ "readability-implicit-bool-conversion"); CheckFactories.registerCheck( "readability-inconsistent-declaration-parameter-name"); + CheckFactories.registerCheck( + "readability-isolate-declaration"); CheckFactories.registerCheck( "readability-magic-numbers"); CheckFactories.registerCheck( Index: clang-tidy/utils/LexerUtils.h =================================================================== --- clang-tidy/utils/LexerUtils.h +++ clang-tidy/utils/LexerUtils.h @@ -22,6 +22,65 @@ Token getPreviousToken(SourceLocation Location, const SourceManager &SM, const LangOptions &LangOpts, bool SkipComments = true); +SourceLocation findPreviousTokenStart(SourceLocation Start, + const SourceManager &SM, + const LangOptions &LangOpts); + +SourceLocation findPreviousTokenKind(SourceLocation Start, + const SourceManager &SM, + const LangOptions &LangOpts, + tok::TokenKind TK); + +SourceLocation findNextTerminator(SourceLocation Start, const SourceManager &SM, + const LangOptions &LangOpts); + +template +SourceLocation findPreviousAnyTokenKind(SourceLocation Start, + const SourceManager &SM, + const LangOptions &LangOpts, + TokenKind TK, TokenKinds... TKs) { + while (true) { + SourceLocation L = findPreviousTokenStart(Start, SM, LangOpts); + if (L.isInvalid() || L.isMacroID()) + return SourceLocation(); + + Token T; + // Returning 'true' is used to signal failure to retrieve the token. + if (Lexer::getRawToken(L, T, SM, LangOpts)) + return SourceLocation(); + + if (T.isOneOf(TK, TKs...)) + return T.getLocation(); + + Start = L; + } +} + +template +SourceLocation findNextAnyTokenKind(SourceLocation Start, + const SourceManager &SM, + const LangOptions &LangOpts, TokenKind TK, + TokenKinds... TKs) { + while (true) { + Optional CurrentToken = Lexer::findNextToken(Start, SM, LangOpts); + + if (!CurrentToken) + return SourceLocation(); + + Token PotentialMatch = *CurrentToken; + if (PotentialMatch.isOneOf(TK, TKs...)) + return PotentialMatch.getLocation(); + + Start = PotentialMatch.getLastLoc(); + } +} + +/// Relex the provide \p Range and return \c false if either a macro spanning +/// multiple tokens, a pre-processor directive or failure to retrieve the +/// next token is found, otherwise \c true. +bool saveFromPreProcessor(SourceRange Range, const SourceManager &SM, + const LangOptions &LangOpts); + } // namespace lexer } // namespace utils } // namespace tidy Index: clang-tidy/utils/LexerUtils.cpp =================================================================== --- clang-tidy/utils/LexerUtils.cpp +++ clang-tidy/utils/LexerUtils.cpp @@ -31,6 +31,66 @@ return Token; } +SourceLocation findPreviousTokenStart(SourceLocation Start, + const SourceManager &SM, + const LangOptions &LangOpts) { + if (Start.isInvalid() || Start.isMacroID()) + return SourceLocation(); + + SourceLocation BeforeStart = Start.getLocWithOffset(-1); + if (BeforeStart.isInvalid() || BeforeStart.isMacroID()) + return SourceLocation(); + + return Lexer::GetBeginningOfToken(BeforeStart, SM, LangOpts); +} + +SourceLocation findPreviousTokenKind(SourceLocation Start, + const SourceManager &SM, + const LangOptions &LangOpts, + tok::TokenKind TK) { + while (true) { + SourceLocation L = findPreviousTokenStart(Start, SM, LangOpts); + if (L.isInvalid() || L.isMacroID()) + return SourceLocation(); + + Token T; + if (Lexer::getRawToken(L, T, SM, LangOpts, /*IgnoreWhiteSpace=*/true)) + return SourceLocation(); + + if (T.is(TK)) + return T.getLocation(); + + Start = L; + } +} + +SourceLocation findNextTerminator(SourceLocation Start, const SourceManager &SM, + const LangOptions &LangOpts) { + return findNextAnyTokenKind(Start, SM, LangOpts, tok::comma, tok::semi); +} + +bool saveFromPreProcessor(SourceRange Range, const SourceManager &SM, + const LangOptions &LangOpts) { + assert(Range.isValid() && "Invalid Range for relexing provided"); + SourceLocation Loc = Range.getBegin(); + + while (Loc < Range.getEnd()) { + if (Loc.isMacroID()) + return false; + + llvm::Optional Tok = Lexer::findNextToken(Loc, SM, LangOpts); + + if (!Tok) + return false; + + if ((*Tok).is(tok::hash)) + return false; + + Loc = Lexer::getLocForEndOfToken(Loc, 0, SM, LangOpts).getLocWithOffset(1); + } + + return true; +} } // namespace lexer } // namespace utils } // namespace tidy Index: docs/ReleaseNotes.rst =================================================================== --- docs/ReleaseNotes.rst +++ docs/ReleaseNotes.rst @@ -106,6 +106,12 @@ This check warns the uses of the deprecated member types of ``std::ios_base`` and replaces those that have a non-deprecated equivalent. +- New :doc:`readability-isolate-decl + ` check. + + Diagnoses local variable declarations declaring more than one variable and + tries to refactor the code to one statement per declaration. + - New :doc:`readability-magic-numbers ` check. Index: docs/clang-tidy/checks/list.rst =================================================================== --- docs/clang-tidy/checks/list.rst +++ docs/clang-tidy/checks/list.rst @@ -227,6 +227,7 @@ readability-identifier-naming readability-implicit-bool-conversion readability-inconsistent-declaration-parameter-name + readability-isolate-declaration readability-magic-numbers readability-misleading-indentation readability-misplaced-array-index Index: docs/clang-tidy/checks/readability-isolate-declaration.rst =================================================================== --- /dev/null +++ docs/clang-tidy/checks/readability-isolate-declaration.rst @@ -0,0 +1,99 @@ +.. title:: clang-tidy - readability-isolate-declaration + +readability-isolate-declarationaration +====================================== + +Diagnoses local variable declarations declaring more than one variable and +tries to refactor the code to one statement per declaration. + +The automatic code-transformation will use the same indentation as the original +for every created statement and add a linebreak after each statement. + +.. code-block:: c++ + + void f() { + int * pointer = nullptr, value = 42, * const const_ptr = &value; + // This declaration will be diagnosed and transformed into: + // int * pointer = nullptr; + // int value = 42; + // int * const const_ptr = &value; + } + + +The check does exclude places where it is necessary or commong to declare +multiple variables in one statement and there is no other way supported in the +language. Please note that structured bindings are not considered. + +.. code-block:: c++ + + // It is not possible to transform this declaration and doing the declaration + // before the loop will increase the scope of the variable 'Begin' and 'End' + // which is undesired in it's own. + for (int Begin = 0, End = 100; Begin < End; ++Begin); + if (int Begin = 42, Result = some_function(Begin); Begin == Result); + + // It is not possible to transform this declaration because the result is + // not functionality preserving as 'j' and 'k' would not be part of the + // 'if' statement anymore. + if (SomeCondition()) + int i = 42, j = 43, k = function(i,j); + + +Limitations +----------- + +Global variables and member variables are excluded. + +The check currently does not support the automatic transformation of +member-pointer-types. + +.. code-block:: c++ + + struct S { + int a; + const int b; + void f() {} + }; + + void f() { + // Only a diagnostic message is emitted + int S::*p = &S::a, S::*const q = &S::a; + } + +Furthermore the transformation is very cautious when it detects various kinds +of macros or preprocessor directives in the range of the statement. In this +case the transformation will not happen to avoid unexpected side-effects due to +macros. + +.. code-block:: c++ + + #define NULL 0 + #define MY_NICE_TYPE int ** + #define VAR_NAME(name) name##__LINE__ + #define A_BUNCH_OF_VARIABLES int m1 = 42, m2 = 43, m3 = 44; + + void macros() { + int *p1 = NULL, *p2 = NULL; + // Will be transformed to + // int *p1 = NULL; + // int *p2 = NULL; + + MY_NICE_TYPE p3, v1, v2; + // Won't be transformed, but a diagnostic is emitted. + + int VAR_NAME(v3), + VAR_NAME(v4), + VAR_NAME(v5); + // Won't be transformed, but a diagnostic is emitted. + + A_BUNCH_OF_VARIABLES + // Won't be transformed, but a diagnostic is emitted. + + int Unconditional, + #if CONFIGURATION + IfConfigured = 42, + #else + IfConfigured = 0; + #endif + // Won't be transformed, but a diagnostic is emitted. + } Index: test/clang-tidy/readability-isolate-declaration-cxx17.cpp =================================================================== --- /dev/null +++ test/clang-tidy/readability-isolate-declaration-cxx17.cpp @@ -0,0 +1,103 @@ +// RUN: %check_clang_tidy %s readability-isolate-declaration %t -- -- -std=c++17 + +template +struct pair { + T1 first; + T2 second; + pair(T1 v1, T2 v2) : first(v1), second(v2) {} + + template + decltype(auto) get() const { + if constexpr (N == 0) + return first; + else if constexpr (N == 1) + return second; + } +}; + +void forbidden_transformations() { + if (int i = 42, j = i; i == j) + ; + switch (int i = 12, j = 14; i) + ; + + auto [i, j] = pair(42, 42); +} + +struct SomeClass { + SomeClass() = default; + SomeClass(int value); +}; + +namespace std { +template +class initializer_list {}; + +template +class vector { +public: + vector() = default; + vector(initializer_list init) {} +}; + +class string { +public: + string() = default; + string(const char *) {} +}; + +namespace string_literals { +string operator""s(const char *, decltype(sizeof(int))) { + return string(); +} +} // namespace string_literals +} // namespace std + +namespace Types { +typedef int MyType; +} // namespace Types + +int touch1, touch2; + +void modern() { + auto autoInt1 = 3, autoInt2 = 4; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: multiple declarations in a single statement reduces readability + // CHECK-FIXES: auto autoInt1 = 3; + // CHECK-FIXES: {{^ }}auto autoInt2 = 4; + + decltype(int()) declnottouch = 4; + decltype(int()) declint1 = 5, declint2 = 3; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: multiple declarations in a single statement reduces readability + // CHECK-FIXES: decltype(int()) declint1 = 5; + // CHECK-FIXES: {{^ }}decltype(int()) declint2 = 3; + + std::vector vectorA = {1, 2}, vectorB = {1, 2, 3}, vectorC({1, 1, 1}); + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: multiple declarations in a single statement reduces readability + // CHECK-FIXES: std::vector vectorA = {1, 2}; + // CHECK-FIXES: {{^ }}std::vector vectorB = {1, 2, 3}; + // CHECK-FIXES: {{^ }}std::vector vectorC({1, 1, 1}); + + using uType = int; + uType utype1, utype2; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: multiple declarations in a single statement reduces readability + // CHECK-FIXES: uType utype1; + // CHECK-FIXES: {{^ }}uType utype2; + + Types::MyType mytype1, mytype2, mytype3 = 3; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: multiple declarations in a single statement reduces readability + // CHECK-FIXES: Types::MyType mytype1; + // CHECK-FIXES: {{^ }}Types::MyType mytype2; + // CHECK-FIXES: {{^ }}Types::MyType mytype3 = 3; + + { + using namespace std::string_literals; + + std::vector s{"foo"s, "bar"s}, t{"foo"s}, u, a({"hey", "you"}), bb = {"h", "a"}; + // CHECK-MESSAGES: [[@LINE-1]]:5: warning: multiple declarations in a single statement reduces readability + // CHECK-FIXES: std::vector s{"foo"s, "bar"s}; + // CHECK-FIXES: {{^ }}std::vector t{"foo"s}; + // CHECK-FIXES: {{^ }}std::vector u; + // CHECK-FIXES: {{^ }}std::vector a({"hey", "you"}); + // CHECK-FIXES: {{^ }}std::vector bb = {"h", "a"}; + } +} Index: test/clang-tidy/readability-isolate-declaration-fixing.cpp =================================================================== --- /dev/null +++ test/clang-tidy/readability-isolate-declaration-fixing.cpp @@ -0,0 +1,64 @@ +// RUN: %check_clang_tidy %s readability-isolate-declaration %t +// XFAIL: * + +struct S { + int a; + const int b; + void f() {} +}; + +void member_pointers() { + // FIXME: Memberpointers are transformed incorrect. Emit only a warning + // for now. + int S::*p = &S::a, S::*const q = &S::a; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: multiple declarations in a single statement reduces readability + // CHECK-FIXES: int S::*p = &S::a; + // CHECK-FIXES: {{^ }}int S::*const q = &S::a; + + int /* :: */ S::*pp2 = &S::a, var1 = 0; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: multiple declarations in a single statement reduces readability + // CHECK-FIXES: int /* :: */ S::*pp2 = &S::a; + // CHECK-FIXES: {{^ }}int var1 = 0; + + const int S::*r = &S::b, S::*t; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: multiple declarations in a single statement reduces readability + // CHECK-FIXES: const int S::*r = &S::b; + // CHECK-FIXES: {{^ }}const int S::*t; + + { + int S::*mdpa1[2] = {&S::a, &S::a}, var1 = 0; + // CHECK-MESSAGES: [[@LINE-1]]:5: warning: multiple declarations in a single statement reduces readability + // CHECK-FIXES: int S::*mdpa1[2] = {&S::a, &S::a}; + // CHECK-FIXES: {{^ }}int var1 = 0; + + int S ::**mdpa2[2] = {&p, &pp2}, var2 = 0; + // CHECK-MESSAGES: [[@LINE-1]]:5: warning: multiple declarations in a single statement reduces readability + // CHECK-FIXES: int S ::**mdpa2[2] = {&p, &pp2}; + // CHECK-FIXES: {{^ }}int var2 = 0; + + void (S::*mdfp1)() = &S::f, (S::*mdfp2)() = &S::f; + // CHECK-MESSAGES: [[@LINE-1]]:5: warning: multiple declarations in a single statement reduces readability + // CHECK-FIXES: void (S::*mdfp1)() = &S::f; + // CHECK-FIXES: {{^ }}void (S::*mdfp2)() = &S::f; + + void (S::*mdfpa1[2])() = {&S::f, &S::f}, (S::*mdfpa2)() = &S::f; + // CHECK-MESSAGES: [[@LINE-1]]:5: warning: multiple declarations in a single statement reduces readability + // CHECK-FIXES: void (S::*mdfpa1[2])() = {&S::f, &S::f}; + // CHECK-FIXES: {{^ }}void (S::*mdfpa2)() = &S::f; + + void (S::* * mdfpa3[2])() = {&mdfpa1[0], &mdfpa1[1]}, (S::*mdfpa4)() = &S::f; + // CHECK-MESSAGES: [[@LINE-1]]:5: warning: multiple declarations in a single statement reduces readability + // CHECK-FIXES: void (S::* * mdfpa3[2])() = {&mdfpa1[0], &mdfpa1[1]}; + // CHECK-FIXES: {{^ }}void (S::*mdfpa4)() = &S::f; + } + + class CS { + public: + int a; + const int b; + }; + int const CS ::*pp = &CS::a, CS::*const qq = &CS::a; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: multiple declarations in a single statement reduces readability + // CHECK-FIXES: int const CS ::*pp = &CS::a; + // CHECK-FIXES: {{^ }}int const CS::*const qq = &CS::a; +} Index: test/clang-tidy/readability-isolate-declaration.c =================================================================== --- /dev/null +++ test/clang-tidy/readability-isolate-declaration.c @@ -0,0 +1,13 @@ +// RUN: %check_clang_tidy %s readability-isolate-declaration %t + +void c_specific() { + void (*signal(int sig, void (*func)(int)))(int); + int i = sizeof(struct S { int i; }); + int j = sizeof(struct T { int i; }), k; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: multiple declarations in a single statement reduces readability + // CHECK-FIXES: int j = sizeof(struct T { int i; }); + // CHECK-FIXES: {{^ }}int k; + + void g(struct U { int i; } s); // One decl + void h(struct V { int i; } s), m(int i, ...); // Two decls +} Index: test/clang-tidy/readability-isolate-declaration.cpp =================================================================== --- /dev/null +++ test/clang-tidy/readability-isolate-declaration.cpp @@ -0,0 +1,412 @@ +// RUN: %check_clang_tidy %s readability-isolate-declaration %t + +void f() { + int i; +} + +void f2() { + int i, j, *k, lala = 42; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: multiple declarations in a single statement reduces readability + // CHECK-FIXES: int i; + // CHECK-FIXES: {{^ }}int j; + // CHECK-FIXES: {{^ }}int *k; + // CHECK-FIXES: {{^ }}int lala = 42; + + int normal, weird = /* comment */ 42; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: multiple declarations in a single statement reduces readability + // CHECK-FIXES: int normal; + // CHECK-FIXES: {{^ }}int weird = /* comment */ 42; + + int /* here is a comment */ v1, + // another comment + v2 = 42 // Ok, more comments + ; + // CHECK-MESSAGES: [[@LINE-4]]:3: warning: multiple declarations in a single statement reduces readability + // CHECK-FIXES: int /* here is a comment */ v1; + // CHECK-FIXES: {{^ }}int /* here is a comment */ // another comment + // CHECK-FIXES: {{^ }}v2 = 42 // Ok, more comments + // CHECK-FIXES: {{^ }}; + + auto int1 = 42, int2 = 0, int3 = 43; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: multiple declarations in a single statement reduces readability + // CHECK-FIXES: auto int1 = 42; + // CHECK-FIXES: {{^ }}auto int2 = 0; + // CHECK-FIXES: {{^ }}auto int3 = 43; + + decltype(auto) ptr1 = &int1, ptr2 = &int1; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: multiple declarations in a single statement reduces readability + // CHECK-FIXES: decltype(auto) ptr1 = &int1; + // CHECK-FIXES: {{^ }}decltype(auto) ptr2 = &int1; + + decltype(k) ptr3 = &int1, ptr4 = &int1; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: multiple declarations in a single statement reduces readability + // CHECK-FIXES: decltype(k) ptr3 = &int1; + // CHECK-FIXES: {{^ }}decltype(k) ptr4 = &int1; +} + +void f3() { + int i, *pointer1; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: multiple declarations in a single statement reduces readability + // CHECK-FIXES: int i; + // CHECK-FIXES: {{^ }}int *pointer1; + // + int *pointer2 = nullptr, *pointer3 = &i; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: multiple declarations in a single statement reduces readability + // CHECK-FIXES: int *pointer2 = nullptr; + // CHECK-FIXES: {{^ }}int *pointer3 = &i; + + int *(i_ptr) = nullptr, *((i_ptr2)); + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: multiple declarations in a single statement reduces readability + // CHECK-FIXES: int *(i_ptr) = nullptr; + // CHECK-FIXES: {{^ }}int *((i_ptr2)); + + float(*f_ptr)[42], (((f_value))) = 42; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: multiple declarations in a single statement reduces readability + // CHECK-FIXES: float (*f_ptr)[42]; + // CHECK-FIXES: {{^ }}float (((f_value))) = 42; + + float(((*f_ptr2)))[42], ((*f_ptr3)), f_value2 = 42.f; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: multiple declarations in a single statement reduces readability + // CHECK-FIXES: float (((*f_ptr2)))[42]; + // CHECK-FIXES: {{^ }}float ((*f_ptr3)); + // CHECK-FIXES: {{^ }}float f_value2 = 42.f; + + float(((*f_ptr4)))[42], *f_ptr5, ((f_value3)); + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: multiple declarations in a single statement reduces readability + // CHECK-FIXES: float (((*f_ptr4)))[42]; + // CHECK-FIXES: {{^ }}float *f_ptr5; + // CHECK-FIXES: {{^ }}float ((f_value3)); + + void(((*f2))(int)), (*g2)(int, float); + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: multiple declarations in a single statement reduces readability + // CHECK-FIXES: void (((*f2))(int)); + // CHECK-FIXES: {{^ }}void (*g2)(int, float); + + float(*(*(*f_ptr6)))[42], (*f_ptr7); + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: multiple declarations in a single statement reduces readability + // CHECK-FIXES: float (*(*(*f_ptr6)))[42]; + // CHECK-FIXES: {{^ }}float (*f_ptr7); +} + +void f4() { + double d = 42. /* foo */, z = 43., /* hi */ y, c /* */ /* */, l = 2.; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: multiple declarations in a single statement reduces readability + // CHECK-FIXES: double d = 42. /* foo */; + // CHECK-FIXES: {{^ }}double z = 43.; + // CHECK-FIXES: {{^ }}double /* hi */ y; + // CHECK-FIXES: {{^ }}double c /* */ /* */; + // CHECK-FIXES: {{^ }}double l = 2.; +} + +struct SomeClass { + SomeClass() = default; + SomeClass(int value); +}; + +class Point { + double x; + double y; + +public: + Point(double x, double y) : x(x), y(y) {} +}; + +class Rectangle { + Point TopLeft; + Point BottomRight; + +public: + Rectangle(Point TopLeft, Point BottomRight) : TopLeft(TopLeft), BottomRight(BottomRight) {} +}; + +void f5() { + SomeClass v1, v2(42), v3{42}, v4(42.5); + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: multiple declarations in a single statement reduces readability + // CHECK-FIXES: SomeClass v1; + // CHECK-FIXES: {{^ }}SomeClass v2(42); + // CHECK-FIXES: {{^ }}SomeClass v3{42}; + // CHECK-FIXES: {{^ }}SomeClass v4(42.5); + + SomeClass v5 = 42, *p1 = nullptr; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: multiple declarations in a single statement reduces readability + // CHECK-FIXES: SomeClass v5 = 42; + // CHECK-FIXES: {{^ }}SomeClass *p1 = nullptr; + + Point P1(0., 2.), P2{2., 0.}; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: multiple declarations in a single statement reduces readability + // CHECK-FIXES: Point P1(0., 2.); + // CHECK-FIXES: {{^ }}Point P2{2., 0.}; + + Rectangle R1({0., 0.}, {1., -2.}), R2{{0., 1.}, {1., 0.}}, R3(P1, P2), R4{P1, P2}; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: multiple declarations in a single statement reduces readability + // CHECK-FIXES: Rectangle R1({0., 0.}, {1., -2.}); + // CHECK-FIXES: {{^ }}Rectangle R2{{[{][{]}}0., 1.}, {1., 0.{{[}][}]}}; + // CHECK-FIXES: {{^ }}Rectangle R3(P1, P2); + // CHECK-FIXES: {{^ }}Rectangle R4{P1, P2}; +} + +void f6() { + int array1[] = {1, 2, 3, 4}, array2[] = {1, 2, 3}, value1, value2 = 42; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: multiple declarations in a single statement reduces readability + // CHECK-FIXES: int array1[] = {1, 2, 3, 4}; + // CHECK-FIXES: {{^ }}int array2[] = {1, 2, 3}; + // CHECK-FIXES: {{^ }}int value1; + // CHECK-FIXES: {{^ }}int value2 = 42; +} + +template +struct TemplatedType { + TemplatedType() = default; + TemplatedType(T value); +}; + +void f7() { + TemplatedType TT1(42), TT2{42}, TT3; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: multiple declarations in a single statement reduces readability + // CHECK-FIXES: TemplatedType TT1(42); + // CHECK-FIXES: {{^ }}TemplatedType TT2{42}; + // CHECK-FIXES: {{^ }}TemplatedType TT3; + // + TemplatedType *TT4(nullptr), TT5, **TT6 = nullptr, *const *const TT7{nullptr}; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: multiple declarations in a single statement reduces readability + // CHECK-FIXES: TemplatedType *TT4(nullptr); + // CHECK-FIXES: {{^ }}TemplatedType TT5; + // CHECK-FIXES: {{^ }}TemplatedType **TT6 = nullptr; + // CHECK-FIXES: {{^ }}TemplatedType *const *const TT7{nullptr}; + + TemplatedType **TT8(nullptr), *TT9; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: multiple declarations in a single statement reduces readability + // CHECK-FIXES: TemplatedType **TT8(nullptr); + // CHECK-FIXES: {{^ }}TemplatedType *TT9; + + TemplatedType TT10{nullptr}, *TT11(nullptr); + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: multiple declarations in a single statement reduces readability + // CHECK-FIXES: TemplatedType TT10{nullptr}; + // CHECK-FIXES: {{^ }}TemplatedType *TT11(nullptr); +} + +void forbidden_transformations() { + for (int i = 0, j = 42; i < j; ++i) + ; +} + +#define NULL 0 +#define MY_NICE_TYPE int ** +#define VAR_NAME(name) name##__LINE__ +#define A_BUNCH_OF_VARIABLES int m1 = 42, m2 = 43, m3 = 44; + +void macros() { + int *p1 = NULL, *p2 = NULL; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: multiple declarations in a single statement reduces readability + // CHECK-FIXES: int *p1 = NULL; + // CHECK-FIXES: {{^ }}int *p2 = NULL; + + // Macros are involved, so there will be no transformation + MY_NICE_TYPE p3, v1, v2; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: multiple declarations in a single statement reduces readability + + int VAR_NAME(v3), + VAR_NAME(v4), + VAR_NAME(v5); + // CHECK-MESSAGES: [[@LINE-3]]:3: warning: multiple declarations in a single statement reduces readability + + A_BUNCH_OF_VARIABLES + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: multiple declarations in a single statement reduces readability + + int Unconditional, + // Explanatory comment. +#if CONFIGURATION + IfConfigured = 42, +#else + IfConfigured = 0; +#endif + // CHECK-MESSAGES: [[@LINE-7]]:3: warning: multiple declarations in a single statement reduces readability +} + +void dontTouchParameter(int param1, int param2) {} + +struct StructOne { + StructOne() {} + StructOne(int b) {} + + int member1, member2; + // TODO: Transform FieldDecl's as well +}; + +using PointerType = int; + +struct { + int i; +} AS1, AS2; +struct TemT { + template + T *getAs() { + return nullptr; + } +} TT1, TT2; + +void complex_typedefs() { + typedef int *IntPtr; + typedef int ArrayType[2]; + typedef int FunType(void); + + IntPtr intptr1, intptr2 = nullptr, intptr3; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: multiple declarations in a single statement reduces readability + // CHECK-FIXES: IntPtr intptr1; + // CHECK-FIXES: {{^ }}IntPtr intptr2 = nullptr; + // CHECK-FIXES: {{^ }}IntPtr intptr3; + + IntPtr *DoublePtr1 = nullptr, **TriplePtr, SinglePtr = nullptr; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: multiple declarations in a single statement reduces readability + // CHECK-FIXES: IntPtr *DoublePtr1 = nullptr; + // CHECK-FIXES: {{^ }}IntPtr **TriplePtr; + // CHECK-FIXES: {{^ }}IntPtr SinglePtr = nullptr; + + IntPtr intptr_array1[2], intptr_array2[4] = {nullptr, nullptr, nullptr, nullptr}; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: multiple declarations in a single statement reduces readability + // CHECK-FIXES: IntPtr intptr_array1[2]; + // CHECK-FIXES: {{^ }}IntPtr intptr_array2[4] = {nullptr, nullptr, nullptr, nullptr}; + + ArrayType arraytype1, arraytype2 = {1}, arraytype3; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: multiple declarations in a single statement reduces readability + // CHECK-FIXES: ArrayType arraytype1; + // CHECK-FIXES: {{^ }}ArrayType arraytype2 = {1}; + // CHECK-FIXES: {{^ }}ArrayType arraytype3; + + // Don't touch function declarations. + FunType funtype1, funtype2, functype3; + + for (int index1 = 0, index2 = 0;;) { + int localFor1 = 1, localFor2 = 2; + // CHECK-MESSAGES: [[@LINE-1]]:5: warning: multiple declarations in a single statement reduces readability + // CHECK-FIXES: int localFor1 = 1; + // CHECK-FIXES: {{^ }}int localFor2 = 2; + } + + StructOne s1, s2(23), s3, s4(3), *sptr = new StructOne(2); + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: multiple declarations in a single statement reduces readability + // CHECK-FIXES: StructOne s1; + // CHECK-FIXES: {{^ }}StructOne s2(23); + // CHECK-FIXES: {{^ }}StructOne s3; + // CHECK-FIXES: {{^ }}StructOne s4(3); + // CHECK-FIXES: {{^ }}StructOne *sptr = new StructOne(2); + + struct StructOne cs1, cs2(42); + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: multiple declarations in a single statement reduces readability + // CHECK-FIXES: struct StructOne cs1; + // CHECK-FIXES: {{^ }}struct StructOne cs2(42); + + int *ptrArray[3], dummy, **ptrArray2[5], twoDim[2][3], *twoDimPtr[2][3]; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: multiple declarations in a single statement reduces readability + // CHECK-FIXES: int *ptrArray[3]; + // CHECK-FIXES: {{^ }}int dummy; + // CHECK-FIXES: {{^ }}int **ptrArray2[5]; + // CHECK-FIXES: {{^ }}int twoDim[2][3]; + // CHECK-FIXES: {{^ }}int *twoDimPtr[2][3]; + + { + void f1(int), g1(int, float); + } + + { + void gg(int, float); + + void (*f2)(int), (*g2)(int, float) = gg; + // CHECK-MESSAGES: [[@LINE-1]]:5: warning: multiple declarations in a single statement reduces readability + // CHECK-FIXES: void (*f2)(int); + // CHECK-FIXES: {{^ }}void (*g2)(int, float) = gg; + + void /*(*/ (/*(*/ *f3)(int), (*g3)(int, float); + // CHECK-MESSAGES: [[@LINE-1]]:5: warning: multiple declarations in a single statement reduces readability + // CHECK-FIXES: void /*(*/ (/*(*/ *f3)(int); + // CHECK-FIXES: {{^ }}void /*(*/ (*g3)(int, float); + } + + // clang-format off + auto returner = []() { return int(32); }; + int intfunction = returner(), intarray[] = + { + 1, + 2, + 3, + 4 + }, bb = 4; + // CHECK-MESSAGES: [[@LINE-7]]:3: warning: multiple declarations in a single statement reduces readability + // CHECK-FIXES: int intfunction = returner(); + // CHECK-FIXES: {{^ }}int intarray[] = + // CHECK-FIXES: {{^ }}{ + // CHECK-FIXES: {{^ }}1, + // CHECK-FIXES: {{^ }}2, + // CHECK-FIXES: {{^ }}3, + // CHECK-FIXES: {{^ }}4 + // CHECK-FIXES: {{^ }}}; + // CHECK-FIXES: {{^ }}int bb = 4; + // clang-format on + + TemT *T1 = &TT1, *T2 = &TT2; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: multiple declarations in a single statement reduces readability + // CHECK-FIXES: TemT *T1 = &TT1; + // CHECK-FIXES: {{^ }}TemT *T2 = &TT2; + + const PointerType *PT1 = T1->getAs(), + *PT2 = T2->getAs(); + // CHECK-MESSAGES: [[@LINE-2]]:3: warning: multiple declarations in a single statement reduces readability + // CHECK-FIXES: const PointerType *PT1 = T1->getAs(); + // CHECK-FIXES: {{^ }}const PointerType *PT2 = T2->getAs(); + + const int *p1 = nullptr; + const int *p2 = nullptr; + + const int *&pref1 = p1, *&pref2 = p2; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: multiple declarations in a single statement reduces readability + // CHECK-FIXES: const int *&pref1 = p1; + // CHECK-FIXES: {{^ }}const int *&pref2 = p2; + + // clang-format off + const char *literal1 = "clang" "test"\ + "one", + *literal2 = "empty", literal3[] = "three"; + // CHECK-MESSAGES: [[@LINE-3]]:3: warning: multiple declarations in a single statement reduces readability + // CHECK-FIXES: const char *literal1 = "clang" "test"\ + // CHECK-FIXES: {{^ }}"one"; + // CHECK-FIXES: {{^ }}const char *literal2 = "empty"; + // CHECK-FIXES: {{^ }}const char literal3[] = "three"; + // clang-format on +} + +void g() try { + int i, j; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: multiple declarations in a single statement reduces readability + // CHECK-FIXES: int i; + // CHECK-FIXES: {{^ }}int j; +} catch (...) { +} + +struct S { + int a; + const int b; + void f() {} +}; + +void memberPointers() { + typedef const int S::*MemPtr; + MemPtr aaa = &S::a, bbb = &S::b; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: multiple declarations in a single statement reduces readability + // CHECK-FIXES: MemPtr aaa = &S::a; + // CHECK-FIXES: {{^ }}MemPtr bbb = &S::b; +} + +typedef int *tptr, tbt; +typedef int (&tfp)(int, long), tarr[10]; +typedef int tarr2[10], tct; + +template +void should_not_be_touched(A, B); + +int variable, function(void); + +int call_func_with_sideeffect(); +void bad_if_decl() { + if (true) + int i, j, k = call_func_with_sideeffect(); +}