[clang-tidy] new check 'readability-isolate-declaration'

Summary:
This patch introduces a new clang-tidy check that matches on all `declStmt` that declare more then one variable
and transform them into one statement per declaration if possible.

It currently only focusses on variable declarations but should be extended to cover more kinds of declarations in the future.
It is related to https://reviews.llvm.org/D27621 and does use it's extensive test-suite. Thank you to firolino for his work!

Reviewers: rsmith, aaron.ballman, alexfh, hokein, kbobyrev

Reviewed By: aaron.ballman

Subscribers: ZaMaZaN4iK, mgehre, nemanjai, kbarton, lebedev.ri, Eugene.Zelenko, mgorny, xazax.hun, cfe-commits

Tags: #clang-tools-extra

Differential Revision: https://reviews.llvm.org/D51949

llvm-svn: 345735
This commit is contained in:
Jonas Toth 2018-10-31 16:50:44 +00:00
parent 1c254c6716
commit 0ea5af7acb
13 changed files with 1139 additions and 0 deletions

View File

@ -11,6 +11,7 @@ add_clang_library(clangTidyReadabilityModule
IdentifierNamingCheck.cpp
ImplicitBoolConversionCheck.cpp
InconsistentDeclarationParameterNameCheck.cpp
IsolateDeclarationCheck.cpp
MagicNumbersCheck.cpp
MisleadingIndentationCheck.cpp
MisplacedArrayIndexCheck.cpp

View File

@ -0,0 +1,279 @@
//===--- 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<VarDecl>(D); });
}
} // 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 (T->isFunctionPointerType()) {
const auto *Pointee = T->getPointeeType()->castAs<FunctionType>();
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<ArrayType>(T))
return countIndirections(AT->getElementType().IgnoreParens().getTypePtr(),
Indirections);
if (isa<PointerType>(T) || isa<ReferenceType>(T))
return countIndirections(T->getPointeeType().IgnoreParens().getTypePtr(),
++Indirections);
return Indirections;
}
static bool typeIsMemberPointer(const Type *T) {
if (isa<ArrayType>(T))
return typeIsMemberPointer(T->getArrayElementTypeNoTypeQual());
if ((isa<PointerType>(T) || isa<ReferenceType>(T)) &&
isa<PointerType>(T->getPointeeType()))
return typeIsMemberPointer(T->getPointeeType().getTypePtr());
return isa<MemberPointerType>(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<std::vector<SourceRange>>
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 (rangeContainsExpansionsOrDirectives(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<SourceRange> 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<VarDecl>(*DS->decl_begin());
if (FirstDecl == nullptr)
return None;
// FIXME: Member pointers 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<VarDecl>(Decl);
// FIXME: Member pointers 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<std::vector<StringRef>>
collectSourceRanges(llvm::ArrayRef<SourceRange> Ranges, const SourceManager &SM,
const LangOptions &LangOpts) {
std::vector<StringRef> 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<std::string>
createIsolatedDecls(llvm::ArrayRef<StringRef> 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<std::string> 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<DeclStmt>("decl_stmt");
auto Diag =
diag(WholeDecl->getBeginLoc(),
"multiple declarations in a single statement reduces readability");
Optional<std::vector<SourceRange>> PotentialRanges =
declRanges(WholeDecl, *Result.SourceManager, getLangOpts());
if (!PotentialRanges)
return;
Optional<std::vector<StringRef>> PotentialSnippets = collectSourceRanges(
*PotentialRanges, *Result.SourceManager, getLangOpts());
if (!PotentialSnippets)
return;
std::vector<std::string> 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

View File

@ -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

View File

@ -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"
@ -67,6 +68,8 @@ public:
"readability-implicit-bool-conversion");
CheckFactories.registerCheck<InconsistentDeclarationParameterNameCheck>(
"readability-inconsistent-declaration-parameter-name");
CheckFactories.registerCheck<IsolateDeclarationCheck>(
"readability-isolate-declaration");
CheckFactories.registerCheck<MagicNumbersCheck>(
"readability-magic-numbers");
CheckFactories.registerCheck<MisleadingIndentationCheck>(

View File

@ -31,6 +31,67 @@ Token getPreviousToken(SourceLocation Location, const SourceManager &SM,
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 rangeContainsExpansionsOrDirectives(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 true;
llvm::Optional<Token> Tok = Lexer::findNextToken(Loc, SM, LangOpts);
if (!Tok)
return true;
if (Tok->is(tok::hash))
return true;
Loc = Lexer::getLocForEndOfToken(Loc, 0, SM, LangOpts).getLocWithOffset(1);
}
return false;
}
} // namespace lexer
} // namespace utils
} // namespace tidy

View File

@ -22,6 +22,66 @@ namespace lexer {
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 <typename TokenKind, typename... TokenKinds>
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 <typename TokenKind, typename... TokenKinds>
SourceLocation findNextAnyTokenKind(SourceLocation Start,
const SourceManager &SM,
const LangOptions &LangOpts, TokenKind TK,
TokenKinds... TKs) {
while (true) {
Optional<Token> CurrentToken = Lexer::findNextToken(Start, SM, LangOpts);
if (!CurrentToken)
return SourceLocation();
Token PotentialMatch = *CurrentToken;
if (PotentialMatch.isOneOf(TK, TKs...))
return PotentialMatch.getLocation();
Start = PotentialMatch.getLastLoc();
}
}
/// Re-lex the provide \p Range and return \c false if either a macro spans
/// multiple tokens, a pre-processor directive or failure to retrieve the
/// next token is found, otherwise \c true.
bool rangeContainsExpansionsOrDirectives(SourceRange Range,
const SourceManager &SM,
const LangOptions &LangOpts);
} // namespace lexer
} // namespace utils
} // namespace tidy

View File

@ -136,6 +136,12 @@ Improvements to clang-tidy
Detects usage of the deprecated member types of ``std::ios_base`` and replaces
those that have a non-deprecated equivalent.
- New :doc:`readability-isolate-decl
<clang-tidy/checks/readability-isolate-decl>` check.
Detects local variable declarations declaring more than one variable and
tries to refactor the code to one statement per declaration.
- New :doc:`readability-magic-numbers
<clang-tidy/checks/readability-magic-numbers>` check.

View File

@ -233,6 +233,7 @@ Clang-Tidy Checks
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

View File

@ -0,0 +1,100 @@
.. title:: clang-tidy - readability-isolate-declaration
readability-isolate-declaration
===============================
Detects 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 line break after each statement.
It keeps the order of the variable declarations consistent, too.
.. 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 excludes places where it is necessary or common 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 undesirable.
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.
}

View File

@ -0,0 +1,103 @@
// RUN: %check_clang_tidy %s readability-isolate-declaration %t -- -- -std=c++17
template <typename T1, typename T2>
struct pair {
T1 first;
T2 second;
pair(T1 v1, T2 v2) : first(v1), second(v2) {}
template <int N>
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<int, int>(42, 42);
}
struct SomeClass {
SomeClass() = default;
SomeClass(int value);
};
namespace std {
template <typename T>
class initializer_list {};
template <typename T>
class vector {
public:
vector() = default;
vector(initializer_list<T> 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<int> 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<int> vectorA = {1, 2};
// CHECK-FIXES: {{^ }}std::vector<int> vectorB = {1, 2, 3};
// CHECK-FIXES: {{^ }}std::vector<int> 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<std::string> 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<std::string> s{"foo"s, "bar"s};
// CHECK-FIXES: {{^ }}std::vector<std::string> t{"foo"s};
// CHECK-FIXES: {{^ }}std::vector<std::string> u;
// CHECK-FIXES: {{^ }}std::vector<std::string> a({"hey", "you"});
// CHECK-FIXES: {{^ }}std::vector<std::string> bb = {"h", "a"};
}
}

View File

@ -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;
}

View File

@ -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
}

View File

@ -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 <typename T>
struct TemplatedType {
TemplatedType() = default;
TemplatedType(T value);
};
void f7() {
TemplatedType<int> TT1(42), TT2{42}, TT3;
// CHECK-MESSAGES: [[@LINE-1]]:3: warning: multiple declarations in a single statement reduces readability
// CHECK-FIXES: TemplatedType<int> TT1(42);
// CHECK-FIXES: {{^ }}TemplatedType<int> TT2{42};
// CHECK-FIXES: {{^ }}TemplatedType<int> TT3;
//
TemplatedType<int *> *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<int *> *TT4(nullptr);
// CHECK-FIXES: {{^ }}TemplatedType<int *> TT5;
// CHECK-FIXES: {{^ }}TemplatedType<int *> **TT6 = nullptr;
// CHECK-FIXES: {{^ }}TemplatedType<int *> *const *const TT7{nullptr};
TemplatedType<int &> **TT8(nullptr), *TT9;
// CHECK-MESSAGES: [[@LINE-1]]:3: warning: multiple declarations in a single statement reduces readability
// CHECK-FIXES: TemplatedType<int &> **TT8(nullptr);
// CHECK-FIXES: {{^ }}TemplatedType<int &> *TT9;
TemplatedType<int *> TT10{nullptr}, *TT11(nullptr);
// CHECK-MESSAGES: [[@LINE-1]]:3: warning: multiple declarations in a single statement reduces readability
// CHECK-FIXES: TemplatedType<int *> TT10{nullptr};
// CHECK-FIXES: {{^ }}TemplatedType<int *> *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: Handle FieldDecl's as well
};
using PointerType = int;
struct {
int i;
} AS1, AS2;
struct TemT {
template <typename T>
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<PointerType>(),
*PT2 = T2->getAs<PointerType>();
// CHECK-MESSAGES: [[@LINE-2]]:3: warning: multiple declarations in a single statement reduces readability
// CHECK-FIXES: const PointerType *PT1 = T1->getAs<PointerType>();
// CHECK-FIXES: {{^ }}const PointerType *PT2 = T2->getAs<PointerType>();
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 <typename A, typename B>
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();
}