[clang-tidy] check to find declarations declaring more than one name

[firolino]

Summary

This check can be used to find declarations, which declare more than one name.
It helps improving readability and prevents potential bugs caused by inattention
and C/C++ syntax specifics.

In addition, appropriate fix-it hints are provided and all user-intended
indentation will be preserved. For example:

{
  long ** lint1, lint2 = 0, * lint3, **linn;

  const int cx = 1, cy = 2;

  int const CS :: * pp = &CS::a, CS::* const qq = &CS::a;

  decltype(int()) declint1 = 5, declint2 = 3;
  
  typedef int ta, tb;
}

will be transformed to:

{
  long ** lint1;
  long lint2 = 0;
  long * lint3;
  long **linn;
  
  const int cx = 1;
  const int cy = 2;
  
  int const CS :: * pp = &CS::a;
  int const CS::* const qq = &CS::a;
  
  decltype(int()) declint1 = 5;
  decltype(int()) declint2 = 3;
  
  typedef int ta;
  typedef int tb;
}

Only declarations within a compound statement are matched. Meaning, global declarations
and function parameters are not matched. Moreover, it does not match on the following:

{
  for(int i = 0, j = 0;;);
}

This check will be used for:

• CppCoreGuideline ES.10
• DCL04

in the future and provide appropriate options for it.

[firolino]

I have tested it on the whole llvm/clang/extra source code without any problems.

[Eugene.Zelenko]

Please mention this check in docs/ReleaseNotes.rst (in alphabetical order).

This is duplicate of D25024. Will be goo idea to combine code in one. Please note that there are multiple coding guidelines which suggest this style.

[firolino]

I have agreed with Kirill to continue this. See http://lists.llvm.org/pipermail/cfe-dev/2016-October/051270.html
I would suggest to close D25024.

[JDevlieghere]

Some small stuff I noticed while reading through the code, I didn't check it in much detail though.

clang-tidy/readability/OneNamePerDeclarationCheck.cpp

44

Early exit would reduce the level of indentation of the whole function.

48

Same as previous comment.

56

I think this can be const as well.

117

const

160

const auto

165

const auto

246

const

250

const

[firolino]

Updated docs/ReleaseNotes.txt

[Eugene.Zelenko]

clang-tidy/readability/OneNamePerDeclarationCheck.cpp

48

Please run Clang-tidy readability-simplify-boolean-expr. Will be good idea to run other readability and modernize checks too.

[firolino]

Addressed comment from JDevlieghere

[firolino]

In D27621#618379, @JDevlieghere wrote:

Some small stuff I noticed while reading through the code, I didn't check it in much detail though.

Thanks, fixed it.

[firolino]

Applied readability-* checks.

[firolino]

@Eugene.Zelenko No modernize-* check was reported. I applied most of the readability-* checks. For example, I did not put braces around:

if (DeclStmt == nullptr)
    return;

if (DeclStmt->isSingleDecl() || DeclStmt->getLocStart().isMacroID())
    return;

Hope that is ok?

[firolino]

Added test case with struct keyword:

struct StructOne cs1, cs2( 42 );

[malcolm.parsons]

clang-tidy/readability/OneNamePerDeclarationCheck.cpp

49

Can you reject single decls in the matcher? e.g. unless(declCountIs(1)).

72

DeclStmt->getDeclGroup() is repeated several times.
Store it in a variable.

77

auto *

80

auto *

110

auto *

168

auto *

215

auto *

247

Should checks care about formatting, or leave to clang-format?

docs/clang-tidy/checks/readability-one-name-per-declaration.rst

47

typedef

[firolino]

Applied suggestions from malcolm.

[firolino]

typdef -> typedef

[firolino]

Little typo fix.

[firolino]

clang-tidy/readability/OneNamePerDeclarationCheck.cpp

247

At least, they should not change the formatting and try to preserve it. The user shouldn't be forced to run clang-format multiple times. Moreover, not everyone is using clang-format etc.

Having:

const int myvalue( 42 ), value ( 4 );

{
    int a, b, c;
}

and getting:

const int myvalue(42);
const int value(4);

{
    int a;
int b;
int c;
}

seems very unsatisfying to me. This would force a non-formatting-tool user to start using one.

[firolino]

A few namespace cleanups

• Removed explicit llvm::'s, since the used names are already introduced into the clang namespace by llvm.h

[malcolm.parsons]

The fixit construction looks overly complicated.
All you need to do is change a , to a ; and insert a copy of the type:

<< FixItHint::CreateReplacement(CommaRange, ";")
<< FixItHint::CreateInsertionFromRange(VarLocation, TypeRange)

and insert some whitespace where needed.

[firolino]

In D27621#619649, @malcolm.parsons wrote:

The fixit construction looks overly complicated.
All you need to do is change a , to a ; and insert a copy of the type:

<< FixItHint::CreateReplacement(CommaRange, ";")
<< FixItHint::CreateInsertionFromRange(VarLocation, TypeRange)

and insert some whitespace where needed.

I could do CommaRange = findToken(tok:comma) an do a in-place replacment and thus, get rid of the workaround part as well. But for the TypeRange, I didn't find a proper way to get only the type part. For example, getTypeSourceInfo()->getTypeLoc().getSourceRange() for

const int a(1), b(2);
int const c(3), d(4);

will be

const int
int

I am not able to get the qualifier as well. I even saw something in the source like "we do not provide SourceRange for Qualifiers...". I would like to get rid of getUserWrittenType, but didn't find a way. It gets also dirty for MemberFunctionPointers:

int S::*p = &S::a, S::* const q = &S::a;
const int S::*r = &S::b, S::*t;
int const CS :: * pp = &CS::a, CS::* const qq = &CS::a;

TypeRange will be

int S
int S
int const CS

[firolino]

In D27621#619649, @malcolm.parsons wrote:

The fixit construction looks overly complicated.

Yes, you were right. I have changed a couple of things. It looks better now. Still working on a better getUserWrittenType.

[arphaman]

clang-tidy/readability/OneNamePerDeclarationCheck.cpp

134

If you use ArrayRef in findLocationAfterToken then you won't need to create a std::vector here as you should be able to pass in a constant array {tok::semi, tok::comma}

clang-tidy/utils/LexerUtils.h

31

You can use ArrayRef<tok::TokenKind> here.

[firolino]

clang-tidy/readability/OneNamePerDeclarationCheck.cpp

134

This should even work with std::vector, since we have initializer_lists. I have created Token because of Do not use Braced Initializer Lists to Call a Constructor and readability reasons.

Anyway, this function isn't included in the newest version :)

[firolino]

Simplified fix-it hint generation.

[malcolm.parsons]

clang-tidy/readability/OneNamePerDeclarationCheck.cpp

87

A SourceLocation can be implicitly converted to a SourceRange.

88

Is an insert simpler?

FixItHint::CreateInsertion(CommaLocation.getLocWithOffset(1),
                           "\n" + CurrentIndent + UserWrittenType + " ");

[firolino]

clang-tidy/readability/OneNamePerDeclarationCheck.cpp

88

Won't work, if there are whitespaces between comma and variable name. I need to get those first and ltrim() them.

The insert would give for:

int a,b;

int aa, bb;

int xk = 1,
    yk = 2;

->

int a;
int b;

int aa;
int  bb;

int xk = 1;
int 
    yk = 2;

[firolino]

clang-tidy/readability/OneNamePerDeclarationCheck.cpp

87

Thanks. Just saw the constructor.

[firolino]

Small improvments:

• SourceRange -> SourceLocation (malcolms last comment)
• added a few consts

[rsmith]

Please add a test to make sure this does not fire on C++17 decomposition declarations:

void f() {
  struct X { int a, b, c; };
  auto [a, b, c] = X();
}

[malcolm.parsons]

clang-tidy/readability/OneNamePerDeclarationCheck.cpp

155

Can this be confused by comments that contain ::?

test/clang-tidy/readability-one-name-per-declaration-simple.cpp

18

You don't need to repeat the whole warning every time.

[firolino]

In D27621#620464, @rsmith wrote:

Please add a test to make sure this does not fire on C++17 decomposition declarations:

void f() {
  struct X { int a, b, c; };
  auto [a, b, c] = X();
}

Done. Thanks!

[firolino]

clang-tidy/readability/OneNamePerDeclarationCheck.cpp

155

Yes, it does on int /*next time I will use C::*/ S::*p = ...;
Working on fix/different getUserWrittenType.

[Eugene.Zelenko]

I think will be good idea to extend check for class members.

test/clang-tidy/readability-one-name-per-declaration-modern.cpp

14

Please use = default; Same below.

26

Please remove empty line.

35

Please remove empty line.

[firolino]

• small improvements
• re-added lost handling for:

bool defPre = false,
#ifdef IS_ENABLED
       defTest = false;
#else
       defTest = true;
#endif

• added support for new test cases:

int /* :: */ S::*pp2 = &S::a, var1 = 0;
void /*(*/ ( /*(*/ *f3)(int), (*g3)(int, float);
/* *& */ int /* *& */ ** /* *& */ pp,*xx;

[firolino]

Unfortunately, I am not able to make getUserWrittenType easier right now. TypeLoc does not give me the desired range. See also:

• http://lists.llvm.org/pipermail/cfe-dev/2013-March/028467.html
• http://lists.llvm.org/pipermail/cfe-dev/2010-April/008664.html
• https://reviews.llvm.org/D27621#619756

[firolino]

Added fix and test case for const int * const.

[firolino]

small improvement

[firolino]

little cleanup in getUserWrittenType

[firolino]

further cleanup in getUserWrittenType

[firolino]

Added support for new test cases:

int S::*mdpa1[2] = {&S::a, &S::a}, var1 = 0;
// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: declaration statement can be split
// 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]]:9: warning: declaration statement can be split
// 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]]:9: warning: declaration statement can be split
// 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]]:9: warning: declaration statement can be split
// 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]]:9: warning: declaration statement can be split
// CHECK-FIXES: {{^        }}void (S::**mdfpa3[2])() = {&mdfpa1[0], &mdfpa1[1]};
// CHECK-FIXES: {{^        }}void (S::*mdfpa4)() = &S::f;

[aaron.ballman]

clang-tidy/readability/OneNamePerDeclarationCheck.cpp

23

We may want to consider adding this to ASTMatchers.h at some point (can be done in a future patch).

26

This should accept by const std::string &.

33

is a object -> is an object

35

Why do we not want to match this?

45

No need to compare against nullptr explicitly (I think we have a clang-tidy check that will warn on this, in fact).

54

Please don't use auto as the type is not spelled out explicitly in the initialization. Same elsewhere, though it is fine with diag() and in for loop initializers.

62

This reads a bit awkwardly since it doesn't explain why the code is problematic. Perhaps: "multiple declarations should be split into individual declarations to improve readability" or something like that?

68

No need for the newline.

82

Missing a full stop at the end of the sentence. Also, no hyphen in preprocessor, and it should be "add an appropriate".

84

Will this (and the below \n) be converted into a CRLF automatically on platforms where that is the newline character sequence in the source?

88

This should probably use a Twine to avoid a bunch of copies.

102

You can drop the const from the dyn_cast, here and elsewhere.

108

Why references and not pointers? Perhaps this needs a comment.

132

You may want to include tests for pathological code, like:

int *(i), (*j), (((k)));

133

m should be capitalized, and you can elide the braces.

138

Why are you calling the Internal version of this function?

149

Capitalize "u".

161

Instead of adding yet another custom check for whether something is whitespace, can you use isWhitespace() from CharInfo.h, and special case \n? Also, why unsigned char instead of char?

201

Why the extra compound statement?

202

Should be I instead of i.

247

I think what @malcolm.parsons was getting at is that we have talked about automatically running clang-format over changes made from the FixIt engine instead of trying to add custom logic to all of the places we use FixIts.

clang-tidy/readability/ReadabilityTidyModule.cpp

64

Should this also be registered as an alias for cert-dcl04-c and CppCoreGuideline ES.10, or is there work left to be done for those checks?

clang-tidy/utils/LexerUtils.h

15

Please remove this include.

docs/ReleaseNotes.rst

142

that -> than

docs/clang-tidy/checks/readability-one-name-per-declaration.rst

7

Remove the comma. which -> that.

8

improving -> improve

test/clang-tidy/readability-one-name-per-declaration-complex.cpp

245

Can you add a test with:

template <typename A, typename B> // Should not be touched
void f(A, B);

[firolino]

Addressed suggestions from @aaron.ballman Thanks!

clang-tidy/readability/OneNamePerDeclarationCheck.cpp

23

Is there a // FIXME: Once I am in AstMatcher.h, remove me. OK?

26

Since I need a copy, it is more efficient to pass-by-value instead pass-by-reference + copy. So, I get a benefit for rvalues and move will be applied. Search for "want speed? pass by value!" articles.

35

If we decide, whether we transform

class A { 
} Object1, Object2;

to

class A { 
} Object1, 
Object2;

or

class A { 
} 
Object1, 
Object2;

I might consider adding support for it. Moreover, this kind of definition is usually seen globally and I don't know how to handle globals yet. See http://lists.llvm.org/pipermail/cfe-dev/2015-November/046262.html

54

Alright. What about:

const auto FirstVarIt = DeclStmt->getDeclGroup().begin();
const auto DL = SM.getDecomposedLoc(Loc);

62

Yes. Sounds better.

84

Actually, I was thinking about this but I don't have a Windows etc. with a running llvm environment. Is it OK, if I leave a FIXME for now?

102

Alright, but can you explain why?

108

// Typedefs on function pointers are covered into LValueReferenceType's ?

138

Because I need its QualType.

161

CharInfo, nice! unsigned char because there is no negative ASCII. Even isWhitespace has unsigned char.

clang-tidy/readability/ReadabilityTidyModule.cpp

64

I will include Options for these checks in the future, since they have special rules when to split a declaration. See https://reviews.llvm.org/D25024#inline-221483

[aaron.ballman]

clang-tidy/readability/OneNamePerDeclarationCheck.cpp

23

You could put in a FIXME, or just create the ASTMatchers patch and mark this patch as dependent on that one.

35

I think this should be handled. It can be handled in either of the forms you show, or by saying:

A Object1;
A Object2;

If all of these turn out to be a problem, we can still diagnose without providing a fixit.

As for globals in general, they can be handled in a separate patch once we figure out the declaration grouping.

54

Iterators are fine (because they usually involve a lot of template boilerplate for containers and their interfaces are regular and common enough), but the call to getDecomposedLoc() should not use auto -- the reader requires too much internal knowledge to understand what the return type is.

84

I think it's fine with or without the fixme (it's certainly not the first instance of \n in the compiler).

102

The const on the declaration is sufficient to ensure const correctness. It's morally equivalent to:

void f(int i) {
  const int ci = i; // No need to const_cast this either.
}

108

So should this be specific to function pointer types? If so, I don't see that predicated anywhere.

138

You should not call functions that have Internal in the name (generally speaking). Those are generally considered to be warts we want to remove once we get rid of the last external caller.

All of your uses of Type are as a clang::Type *, so are you sure you need the QualType at all? (Btw, you should consider a better name than Type.)

161

Hmm, I'm used to seeing characters using char and arithmetic types using unsigned char, but I suppose I've also seen UTF-8 code points represented as unsigned char, so good enough for me. :-)

[firolino]

clang-tidy/readability/OneNamePerDeclarationCheck.cpp

35

OK. I will try to split the object definition from the class definition, as you have suggested. Thus, I can kick out the tagDecl-matcher as well. If there is no easy way to do this, it will be reported anyway but without a fixit.

Note for me: Update documentation!

102

Ah, got it. Thought first, you meant to remove any const in that dyn_cast line.

138

operator-> is overloaded in QualType... I was so confused all the time. Changed completely to Type.

[firolino]

clang-tidy/readability/OneNamePerDeclarationCheck.cpp

35

What about

struct S {
} S1;

I would like to report this too, since two names are being declared here. S and S1. What do you think?

[firolino]

clang-tidy/readability/OneNamePerDeclarationCheck.cpp

35

struct {
} nn1, nn2;

Shall we ignore anonymous definitions?

[firolino]

clang-tidy/readability/OneNamePerDeclarationCheck.cpp

35

To be more precise: Warn and provide a fixit for struct S {} S1. Only warn for struct {} nn1, nn2.

[aaron.ballman]

clang-tidy/readability/OneNamePerDeclarationCheck.cpp

35

What about

struct S {
} S1;

I would like to report this too, since two names are being declared here. S and S1. What do you think?

I don't think that this should be diagnosed. For one, this is a relatively common pattern (arguably more common than struct S { } S1, S2;), but also, it names two very distinct entities (a type and a variable).

struct {
} nn1, nn2;

Shall we ignore anonymous definitions?

Yes, we basically have to.

[firolino]

clang-tidy/readability/OneNamePerDeclarationCheck.cpp

35

Transforming

typedef struct X { int t; } X, Y;

to

typedef struct X { int t; };
typedef X X;
typedef X Y;

will be valid, but looks odd.

[firolino]

• applied suggestions from Aaron
• added support and test cases for TagDecl e.g.

struct S {} S1, S2;

[aaron.ballman]

clang-tidy/readability/OneNamePerDeclarationCheck.cpp

35

I am on the fence about this transformation -- it does not declare new objects, but instead declares new types. A very common pattern in some libraries (such as the Win32 SDK) is to declare:

typedef struct Blah {

} Blah, *PBlah, **PPBlah;

Because of that pattern, diagnosing the above typedef is likely to be very chatty in some projects, and I don't know that the fixit provides much real benefit for types.

At least for the CERT DCL04-C version of this rule, that code should not be flagged. Do you think any of the variations of this check should flag it?

[firolino]

clang-tidy/readability/OneNamePerDeclarationCheck.cpp

35

Maybe we should reconsider where to split and where not. The original idea was to minimize confusion on code like

int* a,b;

whereas

struct S {} *s1, s2;

seems not so confusing as above. However,

struct S {}* s1, s2;

looks dangerous again.

Even following code looks unfamiliar to me.

typedef struct Blah {} Blah, *PBlah;

It really depends on the beholder. So, how about letting this check split everything and provide options for maximum flexibility? So specific rulesets or the user may configure it as they wish. We could add for example split-after-tag-def, split-tag-typedef in addition to cppcore and cert and use a default setting (split-after-tag-def=true, split-tag-typedef=false).

[aaron.ballman]

clang-tidy/readability/OneNamePerDeclarationCheck.cpp

35

I've typically found that users go with whatever the default options are, rather than specify a slew of options, and if the default options produce an onerous check, they simply disable the entire check. I'd rather not push the design off to the users unless there's really no better alternative.

I think that the only "dangerous" operations this sort of check is meant to catch is when the declaration group uses different specifiers for the declarator or when there's an initializer on one declarator but none of the rest. I think the other operations are more likely to be a consistency thing than a bug-prone thing. e.g.,

// Differing specifiers
int i, j, k; // Not at all likely to be mistaken code.
int i, *j, &k; // Not really likely to be mistaken code.
int *i, j; // More likely to be mistaken code.
int i, *j; // Not really likely to be mistaken code.
typedef int *one, two, three; // More likely to be mistaken code.
typedef int one, *two, **three; // Not really likely to be mistaken code.

// Initialization
int i = 10, j = 10; // Not at all likely to be mistaken code.
int i = 10, j; // Not really likely to be mistaken code.
int i, j = 10; // More likely to be mistaken code.

I think declarations that declare both types and variables should ignore the type name and focus only on the variable name following the same heuristics. e.g.,

struct S {} s; // Not at all likely to be mistaken code.
struct S {} s, *t; // Not really likely to be mistaken code.
struct S {} *s, t; // More likely to be mistaken code.

I could imagine an option like FlagPossiblyDangerous or something else; if the flag is set, the check diagnoses based on some heuristic of likely dangerous operations and if the flag is not set, the check flags all declarators in the same declaration.

What do you think?

[firolino]

clang-tidy/readability/OneNamePerDeclarationCheck.cpp

35

We are judging from our perspective what is "likely dangerous" and this might not agree with the users view. This is why I don't wanna force any limitations based on our decisions. This check might not be used only to reduce "readability error" but even for a simple style guide enforcement that wants one variable declaration per line.

I know that int i, j, k; is not bad at all or likely to be mistaken, but style guides from openssl or the linux kernel are enforcing one declaration per line.

I've typically found that users go with whatever the default options are

So, why not simple use options and set the default whatever we think is right. We are not loosing anything with this way, just getting more flexible. Anyone else can reset the options to their needs.

I would think of following options:
Style = Default, CppCore, Cert
SplitTagDef = True, False
SplitTagTypeDef = True, False

and with default:

Style = Default     # splits anything, regardless of tokens and initializers
SplitTagDef = True
SplitTagTypeDef = False

The check will be registered under cert with Style = Cert and CppCore for cppcore respectively.

And since it is topic related, there are still two things on my todo list:

• attributes
• members

struct alignas(8) S {} s1, s2;
alignas(8) char a, b;
[[maybe_unused]] bool x, y;

struct S {
  int a, b;
};

Should we include this as well, or leave it for future work?

[aaron.ballman]

clang-tidy/readability/OneNamePerDeclarationCheck.cpp

35

So, why not simple use options and set the default whatever we think is right. We are not loosing anything with this way, just getting more flexible. Anyone else can reset the options to their needs.

I think we are describing the same solution, but perhaps from different angles. I am proposing that we have this checker flag everything, but that we have a sensible set of default options that reduce the number of diagnostics issued for existing code bases. I fear that if the default behavior of this check is to point out *every* declaration group with more than one name being declared, the output will be way too chatty and so the entire check is disabled. I think that if we had a default behavior that only pointed out the suspicious declarations, that would reduce the "noise" by default. One way to test this would be to run the check over some large code bases of various style guides to see how many results there are. I would recommend running it over the LLVM and Clang code bases (which do not have a restriction on declarations) and OpenSSL (which does have this restriction) and report back the diagnostics you get from each (as well as totals). Feel free to pick other/additional code bases to test again.

Should we include this as well, or leave it for future work?

It's up to you, but I think it's reasonable to leave for future work. Attributes are likely to be somewhat harder than members in many regards due to where attributes can be written. I suspect that members can be handled like variable declarations in general.

[firolino]

clang-tidy/readability/OneNamePerDeclarationCheck.cpp

35

OK, I get what you mean. Btw, thanks for taking your time! This review gets longer and longer :)

[alexfh]

A few more comments.

clang-tidy/readability/OneNamePerDeclarationCheck.cpp

153

I wonder whether re-lexing the whole range and working on tokens instead would be a better and more robust approach? You wouldn't have to deal with whitespace and comments then. What do you think?

163

Why do we want long int and not long?

clang-tidy/readability/OneNamePerDeclarationCheck.h

25

nit: move the private section to the end.

clang-tidy/utils/LexerUtils.cpp

41–56

Is it significantly more efficient than

Token Tok;
do {
  Tok = getPreviousNonCommentToken(Context, Location);
} while (!Tok.isOneOf(tok::unknown, TokenToFind));
return Tok.getLocation();

?

If it's not, then this function might be not so much useful to be here. At least, its interface is rather specific to the use case you have. Why, for example, it returns a location instead of the token itself?

[firolino]

clang-tidy/readability/OneNamePerDeclarationCheck.cpp

153

Sounds interesting! I am gonna give this a try.

163

Sorry, misleading comment.

clang-tidy/readability/OneNamePerDeclarationCheck.h

25

LLVM Coding Standards is putting the private section at the beginning as well. Even in the code base, it seems to be the general rule. While you are reading code from top to bottom, you need to get the necessary context (variables and their types) first, before proceeding to the functional code. Otherwise, you would have to scroll all the time down to the private variables - when passing its usage in a function - to get its type. This would impair the reading fluency and thus the straight-forwarding understanding of the code.

clang-tidy/utils/LexerUtils.cpp

41–56

More efficient? In worst-case (token not found) yes, in my use-case probably not that significant. You are right, changing the interface to Token findTokenBackward(...) seems to be more generic than returning the location directly.

[firolino]

clang-tidy/readability/OneNamePerDeclarationCheck.h

25

Or is it, because there is just a function and not a private variable?

[firolino]

@aaron.ballman I am going to implement CppCore, Cert and Everything and test it on some projects and provide the results next week, to find out which one to set Default.

[firolino]

• nothing special, just went through some open comments and marked them as Done.

clang-tidy/readability/OneNamePerDeclarationCheck.cpp

247

Sounds good! Will clang-format be able to detect the code format in the future?

[aaron.ballman]

clang-tidy/readability/OneNamePerDeclarationCheck.cpp

37–38

Is there a case where this could happen? I would have imagined this as an assert(), if anything.

[alexfh]

clang-tidy/readability/OneNamePerDeclarationCheck.cpp

130

Should we suggest using x = ...; in C++11 code?

clang-tidy/readability/OneNamePerDeclarationCheck.h

25

I don't think there's a rule to this effect in LLVM Coding Standards or a widely accepted convention, it's rather my personal preference (maybe because it's consistent with the Google C++ Style Guide, https://google.github.io/styleguide/cppguide.html#Declaration_Order). Also, the argument you're using doesn't seem to apply here, since the private implementation details aren't used in the public part below it (because, there are no non-empty definitions in it).

[JonasToth]

This comment was removed by JonasToth.

[lebedev.ri]

@firolino do you plan on finishing this?

[firolino]

In D27621#1061562, @lebedev.ri wrote:

@firolino do you plan on finishing this?

Sometimes the universe is really funny! I have just re-started working on it today.

[JonasToth]

Hi @firolino,
I implemented a less general version of you check https://reviews.llvm.org/D51949 as I wanted to achieve a slightly different goal (and this revision seems to be abonded). My aim is to have general transformation capabilities to separate declarations (variables for now, as these refactorings are most important) that can isolate single variables, or split whole declarations.

I used parts of your test-suite as it is very complete. If you plan to continue working on this check, I'd propose to include it into the other (right now almost) finished version.

My Best, Jonas

[firolino]

In D27621#1248849, @JonasToth wrote:

Hi @firolino,
I implemented a less general version of you check https://reviews.llvm.org/D51949 as I wanted to achieve a slightly different goal (and this revision seems to be abonded). My aim is to have general transformation capabilities to separate declarations (variables for now, as these refactorings are most important) that can isolate single variables, or split whole declarations.

I used parts of your test-suite as it is very complete. If you plan to continue working on this check, I'd propose to include it into the other (right now almost) finished version.

My Best, Jonas

Thanks for the information. Looking forward to your final patch!

clang-tidy/readability/CMakeLists.txt

 set(LLVM_LINK_COMPONENTS support)
 
 add_clang_library(clangTidyReadabilityModule
   AvoidConstParamsInDecls.cpp
   BracesAroundStatementsCheck.cpp
   ContainerSizeEmptyCheck.cpp
   DeletedDefaultCheck.cpp
   ElseAfterReturnCheck.cpp
   FunctionSizeCheck.cpp
   IdentifierNamingCheck.cpp
   ImplicitBoolCastCheck.cpp
   InconsistentDeclarationParameterNameCheck.cpp
   MisplacedArrayIndexCheck.cpp
   NamedParameterCheck.cpp
   NamespaceCommentCheck.cpp
   NonConstParameterCheck.cpp
+  OneNamePerDeclarationCheck.cpp
   ReadabilityTidyModule.cpp
   RedundantControlFlowCheck.cpp
   RedundantDeclarationCheck.cpp
   RedundantMemberInitCheck.cpp
   RedundantStringCStrCheck.cpp
   RedundantSmartptrGetCheck.cpp
   RedundantStringInitCheck.cpp
   SimplifyBooleanExprCheck.cpp

clang-tidy/readability/OneNamePerDeclarationCheck.h

+//===--- OneNamePerDeclarationCheck.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_ONE_NAME_PER_DECLARATION_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_ONE_NAME_PER_DECLARATION_H
+
+#include "../ClangTidy.h"
+#include <string>
+
+namespace clang {
+namespace tidy {
+namespace readability {
+
+/// Checks for declarations, declaring more than one name.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/readability-one-name-per-declaration.html
+class OneNamePerDeclarationCheck : public ClangTidyCheck {
+private:
+  std::string getUserWrittenType(const DeclStmt *DeclStmt, SourceManager &SM);
+
+public:
+  OneNamePerDeclarationCheck(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_ONE_NAME_PER_DECLARATION_H

clang-tidy/readability/OneNamePerDeclarationCheck.cpp

+//===--- OneNamePerDeclarationCheck.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 "OneNamePerDeclarationCheck.h"
+#include "../utils/LexerUtils.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Lex/Lexer.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace readability {
+
+static std::string removeMultiLineComments(std::string Str);
+static std::string getCurrentLineIndent(SourceLocation Loc,
+                                        const SourceManager &SM);
+
+void OneNamePerDeclarationCheck::registerMatchers(MatchFinder *Finder) {
+  // Matches all non-single declaration within a compound statement {...}.
+  Finder->addMatcher(declStmt(hasParent(compoundStmt()),
+                              hasDescendant(namedDecl()),
+                              unless(declCountIs(1)))
+                         .bind("declstmt"),
+                     this);
+}
+
+void OneNamePerDeclarationCheck::check(const MatchFinder::MatchResult &Result) {
+  const auto *DeclStatement = Result.Nodes.getNodeAs<DeclStmt>("declstmt");
+  if (!DeclStatement)
+    return;
+
+  // Macros will be ignored
+  if (DeclStatement->getLocStart().isMacroID())
+    return;
+
+  bool IsTagDef = false;
+  const DeclGroupRef DeclGroup = DeclStatement->getDeclGroup();
+
+  if (const auto *TagDef = dyn_cast<TagDecl>(*DeclGroup.begin())) {
+    // Ignore:
+    //  - Anonymous struct, class, enum or union
+    //  - Single variable definitons (e.g. struct S {} a;)
+    //     - First element is the TagDecl and second the variable, thus 2 below
+    if (TagDef->getName().empty() || DeclGroup.getDeclGroup().size() == 2)
+      return;
+
+    IsTagDef = true;
+  }
+
+  SourceManager &SM = *Result.SourceManager;
+  const LangOptions &LangOpts = getLangOpts();
+
+  const SourceLocation DeclStmtStart = DeclStatement->getLocStart();
+  const std::string CurrentIndent = getCurrentLineIndent(DeclStmtStart, SM);
+  const std::string UserWrittenType = getUserWrittenType(DeclStatement, SM);
+
+  auto Diag =
+      diag(DeclStmtStart, "multiple declarations should be split into "
+                          "individual declarations to improve readability");
+
+  // We will iterate through the declaration group starting with the second
+  // declaration. Then, the previous comma will be searched and replaced by a
+  // ';' and the UserWrittenType inserted after it.
+  // For a statement that is a tag + var definition (e.g. struct S {} a,b;),
+  // we will start looking for a closing }-brace instead a comma.
+
+  auto SecondElement = DeclGroup.begin() + 1;
+
+  for (auto It = SecondElement; It != DeclGroup.end(); ++It) {
+    const auto NameLocation = dyn_cast<NamedDecl>(*It)->getLocation();
+    SourceLocation CommaLocation = utils::lexer::findTokenLocationBackward(
+        *Result.Context, NameLocation, tok::comma);
+
+    // If we are starting with a TagDecl, find the closing }-brace
+    // instead a comma
+    if (IsTagDef && (It == SecondElement))
+      CommaLocation = utils::lexer::findTokenLocationBackward(
+          *Result.Context, NameLocation, tok::r_brace);
+
+    if (CommaLocation.isValid()) {
+      const SourceRange AfterCommaToVarNameRange(
+          CommaLocation.getLocWithOffset(1), NameLocation);
+      std::string AnyTokenBetweenCommaAndVarName =
+          Lexer::getSourceText(
+              CharSourceRange::getTokenRange(AfterCommaToVarNameRange), SM,
+              LangOpts)
+              .ltrim(); // may be &, *, etc.
+
+      // Check for preprocessor directive and add appropriate newline.
+      if (AnyTokenBetweenCommaAndVarName.front() == '#')
+        AnyTokenBetweenCommaAndVarName.insert(0, "\n");
+
+      if (IsTagDef && (It == SecondElement))
+        Diag << FixItHint::CreateInsertion(
+            CommaLocation.getLocWithOffset(1),
+            ";"); // Insert ; after closing }-brace
+      else
+        Diag << FixItHint::CreateReplacement(CommaLocation, ";");
+
+      Diag << FixItHint::CreateReplacement(AfterCommaToVarNameRange,
+                                           (llvm::Twine("\n") + CurrentIndent +
+                                            UserWrittenType + " " +
+                                            AnyTokenBetweenCommaAndVarName)
+                                               .str());
+    }
+  }
+}
+
+std::string
+OneNamePerDeclarationCheck::getUserWrittenType(const DeclStmt *DeclStmt,
+                                               SourceManager &SM) {
+  const auto FirstVarIt = DeclStmt->getDeclGroup().begin();
+
+  if (const auto *TagDef = dyn_cast<TagDecl>(*FirstVarIt)) {
+    auto TypeString = TagDef->getNameAsString();
+
+    if (!getLangOpts().CPlusPlus)
+      TypeString = (TagDef->getKindName() + " " + TypeString)
+                       .str(); // Add struct, enum or union before the typename
+
+    if (isa<TypedefDecl>(*(FirstVarIt + 1)))
+      return "typedef " + TypeString;
+
+    return TypeString;
+  }
+
+  SourceLocation Location;
+  const Type *StmtType = nullptr;
+  if (const auto *FirstVar = dyn_cast<DeclaratorDecl>(*FirstVarIt)) {
+    Location = FirstVar->getLocation();
+    StmtType = FirstVar->getType().getTypePtr();
+  } else if (const auto *FirstVar = dyn_cast<TypedefDecl>(*FirstVarIt)) {
+    Location = FirstVar->getLocation();
+    StmtType = FirstVar->getTypeSourceInfo()->getType().getTypePtr();
+    // Typedefs on function pointers are covered into LValueReferenceType's
+    while (StmtType->isLValueReferenceType()) {
+      StmtType = StmtType->getPointeeType().getTypePtr();
+    }
+  } else {
+    llvm_unreachable(
+        "Declaration is neither a DeclaratorDecl nor a TypedefDecl");
+  }
+
+  const SourceRange FVLoc(DeclStmt->getLocStart(), Location);
+  std::string UserWrittenType =
+      Lexer::getSourceText(CharSourceRange::getCharRange(FVLoc), SM,
+                           getLangOpts())
+          .trim();
+
+  UserWrittenType = removeMultiLineComments(UserWrittenType);
+
+  // UserWrittenType might be and we want ->
+  // const int S::* -> const int
+  // const int *&   -> const int
+  // long **        -> long int
+  size_t Pos = std::string::npos;
+
+  if (StmtType->isPointerType() || StmtType->isArrayType() ||
+      StmtType->isReferenceType() || StmtType->isFunctionPointerType() ||
+      StmtType->isFunctionProtoType()) {
+    Pos = UserWrittenType.find_first_of("&*(");
+    if (Pos != std::string::npos) // Might be hidden behind typedef etc.
+      UserWrittenType.erase(Pos);
+
+    while (StmtType->isAnyPointerType() || StmtType->isArrayType())
+      StmtType = StmtType->getPointeeOrArrayElementType();
+  }
+
+  if (StmtType->isMemberFunctionPointerType() &&
+      (Pos = UserWrittenType.find("(")) && Pos != std::string::npos) {
+    UserWrittenType.erase(Pos);
+    return StringRef(UserWrittenType).trim();
+  }
+
+  if (StmtType->isMemberDataPointerType() &&
+      (Pos = UserWrittenType.rfind("::")) && Pos != std::string::npos) {
+    // User might have inserted additional whitespaces:
+    // int   S  ::
+    //      ^-  ^- needed positions
+    UserWrittenType.erase(Pos);
+    UserWrittenType = StringRef(UserWrittenType).trim();
+    Pos = UserWrittenType.rfind(" ");
+    UserWrittenType.erase(Pos);
+  }
+
+  return StringRef(UserWrittenType).trim();
+}
+
+static std::string removeMultiLineComments(std::string Str) {
+  size_t Pos1 = Str.find("/*");
+  while (Pos1 != std::string::npos) {
+    const size_t Pos2 = Str.find("*/", Pos1 + 1);
+    Str.erase(Pos1, Pos2 - Pos1 + 2);
+    Pos1 = Str.find("/*");
+  }
+
+  Str = StringRef(Str).trim();
+  return Str;
+}
+
+static std::string getCurrentLineIndent(SourceLocation Loc,
+                                        const SourceManager &SM) {
+  const std::pair<FileID, unsigned> V = SM.getDecomposedLoc(Loc);
+  const FileID FID = V.first;
+  const unsigned StartOffs = V.second;
+
+  const StringRef MB = SM.getBufferData(FID);
+
+  const unsigned LineNo = SM.getLineNumber(FID, StartOffs) - 1;
+  const SrcMgr::ContentCache *Content =
+      SM.getSLocEntry(FID).getFile().getContentCache();
+  const unsigned LineOffs = Content->SourceLineCache[LineNo];
+
+  // Find the whitespace at the start of the line.
+  StringRef IndentSpace;
+  size_t I = LineOffs;
+  while (isWhitespace(MB[I]) && MB[I] != '\n')
+    ++I;
+  IndentSpace = MB.substr(LineOffs, I - LineOffs);
+
+  return IndentSpace;
+}
+
+} // namespace readability
+} // namespace tidy
+} // namespace clang

clang-tidy/readability/ReadabilityTidyModule.cpp

 #include "ElseAfterReturnCheck.h"
 #include "FunctionSizeCheck.h"
 #include "IdentifierNamingCheck.h"
 #include "ImplicitBoolCastCheck.h"
 #include "InconsistentDeclarationParameterNameCheck.h"
 #include "MisplacedArrayIndexCheck.h"
 #include "NamedParameterCheck.h"
 #include "NonConstParameterCheck.h"
+#include "OneNamePerDeclarationCheck.h"
 #include "RedundantControlFlowCheck.h"
 #include "RedundantDeclarationCheck.h"
 #include "RedundantMemberInitCheck.h"
 #include "RedundantSmartptrGetCheck.h"
 #include "RedundantStringCStrCheck.h"
 #include "RedundantStringInitCheck.h"
 #include "SimplifyBooleanExprCheck.h"
 #include "StaticDefinitionInAnonymousNamespaceCheck.h"
void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {
     CheckFactories.registerCheck<IdentifierNamingCheck>(
         "readability-identifier-naming");
     CheckFactories.registerCheck<ImplicitBoolCastCheck>(
         "readability-implicit-bool-cast");
     CheckFactories.registerCheck<InconsistentDeclarationParameterNameCheck>(
         "readability-inconsistent-declaration-parameter-name");
     CheckFactories.registerCheck<MisplacedArrayIndexCheck>(
         "readability-misplaced-array-index");
+    CheckFactories.registerCheck<OneNamePerDeclarationCheck>(
+        "readability-one-name-per-declaration");
     CheckFactories.registerCheck<RedundantMemberInitCheck>(
         "readability-redundant-member-init");
     CheckFactories.registerCheck<StaticDefinitionInAnonymousNamespaceCheck>(
         "readability-static-definition-in-anonymous-namespace");
     CheckFactories.registerCheck<readability::NamedParameterCheck>(
         "readability-named-parameter");
     CheckFactories.registerCheck<NonConstParameterCheck>(
         "readability-non-const-parameter");

clang-tidy/utils/LexerUtils.h

 //===--- LexerUtils.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_UTILS_LEXER_UTILS_H
 #define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_UTILS_LEXER_UTILS_H
 
 #include "clang/AST/ASTContext.h"
 #include "clang/Lex/Lexer.h"
 
 namespace clang {
 namespace tidy {
 namespace utils {
 namespace lexer {
 
 /// Returns previous non-comment token skipping over any comment text or
 /// ``tok::unknown`` if not found.
 Token getPreviousNonCommentToken(const ASTContext &Context,
                                  SourceLocation Location);
 
+/// \brief This function searches backward from the given location until
+/// TokenToFind is found. If the tokens is not found, the returned source
+/// location will be invalid.
+SourceLocation findTokenLocationBackward(const ASTContext &Context,
+                                         SourceLocation Location,
+                                         tok::TokenKind TokenToFind);
+
 } // namespace lexer
 } // namespace utils
 } // namespace tidy
 } // namespace clang
 
 #endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_UTILS_LEXER_UTILS_H

clang-tidy/utils/LexerUtils.cpp

if (!Lexer::getRawToken(Location, Token, SourceManager,
         !Token.is(tok::comment)) {
       break;
     }
     Location = Location.getLocWithOffset(-1);
   }
   return Token;
 }
 
+SourceLocation findTokenLocationBackward(const ASTContext &Context,
+                                         SourceLocation Location,
+                                         tok::TokenKind TokenToFind) {
+  const auto &SM = Context.getSourceManager();
+  const auto &LO = Context.getLangOpts();
+  auto StartOfFile = SM.getLocForStartOfFile(SM.getFileID(Location));
+
+  Token CurrentToken;
+  CurrentToken.setKind(tok::unknown);
+
+  while (Location != StartOfFile) {
+    if (!Lexer::getRawToken(Location, CurrentToken, SM, LO) &&
+        CurrentToken.is(TokenToFind)) {
+      return Location;
+    }
+    Location = Location.getLocWithOffset(-1);
+  }
+
+  return SourceLocation();
+}
+
 } // namespace lexer
 } // namespace utils
 } // namespace tidy
 } // namespace clang

docs/ReleaseNotes.rst

- New `readability-misplaced-array-index
   Warns when there is array index before the [] instead of inside it.
 
 - New `readability-non-const-parameter
   <http://clang.llvm.org/extra/clang-tidy/checks/readability-non-const-parameter.html>`_ check
 
   Flags function parameters of a pointer type that could be changed to point to
   a constant type instead.
 
+- New `readability-one-name-per-declaration
+  <http://clang.llvm.org/extra/clang-tidy/checks/readability-one-name-per-declaration.html>`_ check
+
+  Finds declarations declaring more than one name.
+
 - New `readability-redundant-declaration
   <http://clang.llvm.org/extra/clang-tidy/checks/readability-redundant-declaration.html>`_ check
 
   Finds redundant variable and function declarations.
 
 - New `readability-redundant-member-init
   <http://clang.llvm.org/extra/clang-tidy/checks/readability-redundant-member-init.html>`_ check
 

docs/clang-tidy/checks/list.rst

.. toctree::
    readability-else-after-return
    readability-function-size
    readability-identifier-naming
    readability-implicit-bool-cast
    readability-inconsistent-declaration-parameter-name
    readability-misplaced-array-index
    readability-named-parameter
    readability-non-const-parameter
+   readability-one-name-per-declaration
    readability-redundant-control-flow
    readability-redundant-declaration
    readability-redundant-member-init
    readability-redundant-smartptr-get
    readability-redundant-string-cstr
    readability-redundant-string-init
    readability-simplify-boolean-expr
    readability-static-definition-in-anonymous-namespace
    readability-uniqueptr-delete-release

docs/clang-tidy/checks/readability-one-name-per-declaration.rst

+.. title:: clang-tidy - readability-one-name-per-declaration
+
+readability-one-name-per-declaration
+====================================
+
+This check can be used to find declarations that declare more than one name.
+It helps improve readability and prevents potential bugs caused by inattention
+and C/C++ syntax specifics.
+
+In addition, appropriate fix-it hints are provided and all user-intended 
+indentation will be preserved. For example:
+
+.. code-block:: c++
+
+  {
+    long ** lint1, lint2 = 0, * lint3, **linn;
+  
+    const int cx = 1, cy = 2;
+  
+    int const CS :: * pp = &CS::a, CS::* const qq = &CS::a;
+  
+    decltype(int()) declint1 = 5, declint2 = 3;
+    
+    typedef int ta, tb;
+  }
+
+will be transformed to:
+
+.. code-block:: c++
+
+  {
+    long ** lint1;
+    long lint2 = 0;
+    long * lint3;
+    long **linn;
+    
+    const int cx = 1;
+    const int cy = 2;
+    
+    int const CS :: * pp = &CS::a;
+    int const CS::* const qq = &CS::a;
+    
+    decltype(int()) declint1 = 5;
+    decltype(int()) declint2 = 3;
+    
+    typedef int ta;
+    typedef int tb;
+  }
+
+Only declarations within a compound statement are matched. Meaning, global declarations
+and function parameters are not matched. Moreover, it does not match on the following:
+
+.. code-block:: c++
+
+  {
+    for(int i = 0, j = 0;;);
+  }

test/clang-tidy/readability-one-name-per-declaration-complex.cpp

+// RUN: %check_clang_tidy %s readability-one-name-per-declaration %t -- -- \
+// RUN:    -std=c++11
+
+void dontTouchParameter(int param1, int param2)
+{}
+
+int returner(void) 
+{
+    int f0 = 0, f1 = 1;
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: multiple declarations should be split
+    // CHECK-FIXES: {{^    }}int f0 = 0;
+    // CHECK-FIXES: {{^    }}int f1 = 1;
+    
+    return 3;
+}
+
+struct StructOne 
+{
+    StructOne(){}
+    StructOne(int b){}
+    
+    int cantTouch1, cantTouch2;
+};
+
+using PointerType = int;
+
+struct TemT
+{
+    template<typename T>
+    T* getAs()
+    {
+        return nullptr;
+    }
+} TT1, TT2;
+
+void complex() 
+{
+    typedef int* IntPtr;
+    typedef int ArrayType[2];
+    typedef int FunType(void);
+    
+    IntPtr intptr1, intptr2 = nullptr, intptr3;
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: multiple declarations should be split
+    // CHECK-FIXES: {{^....}}IntPtr intptr1;
+    // CHECK-FIXES: {{^....}}IntPtr intptr2 = nullptr;
+    // CHECK-FIXES: {{^....}}IntPtr intptr3;
+    
+    ArrayType arraytype1, arraytype2 = {1}, arraytype3;
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: multiple declarations should be split
+    // CHECK-FIXES: {{^....}}ArrayType arraytype1;
+    // CHECK-FIXES: {{^....}}ArrayType arraytype2 = {1};
+    // CHECK-FIXES: {{^....}}ArrayType arraytype3;
+    
+    FunType funtype1, funtype2, functype3;
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: multiple declarations should be split
+    // CHECK-FIXES: {{^....}}FunType funtype1;
+    // CHECK-FIXES: {{^....}}FunType funtype2;
+    // CHECK-FIXES: {{^....}}FunType functype3;
+    
+    for(int index1 = 0, index2 = 0;;)
+    {
+        int localFor1 = 1, localFor2 = 2;
+        // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: multiple declarations should be split
+        // CHECK-FIXES: {{^        }}int localFor1 = 1;
+        // CHECK-FIXES: {{^        }}int localFor2 = 2;
+    }
+    
+    int v1, v2(3), v3, v4(4 ), v5{2}, v6 = {3};
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: multiple declarations should be split
+    // CHECK-FIXES: {{^    }}int v1;
+    // CHECK-FIXES: {{^    }}int v2(3);
+    // CHECK-FIXES: {{^    }}int v3;
+    // CHECK-FIXES: {{^    }}int v4(4 );
+    // CHECK-FIXES: {{^    }}int v5{2};
+    // CHECK-FIXES: {{^    }}int v6 = {3};
+    
+    StructOne s1, s2(23), s3, s4(3), *sptr = new StructOne(2);
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: multiple declarations should be split
+    // 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]]:5: warning: multiple declarations should be split
+    // 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]]:5: warning: multiple declarations should be split
+    // 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);
+            // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: multiple declarations should be split
+            // CHECK-FIXES: {{^            }}void f1(int);
+            // CHECK-FIXES: {{^            }}void g1(int, float);
+        }
+
+        {
+            void gg(int, float);
+            
+            void ( *f2)(int), (*g2)(int, float) = gg;
+            // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: multiple declarations should be split
+            // CHECK-FIXES: {{^            }}void ( *f2)(int);
+            // CHECK-FIXES: {{^            }}void (*g2)(int, float) = gg;
+            
+            void /*(*/ ( /*(*/ *f3)(int), (*g3)(int, float);
+            // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: multiple declarations should be split
+            // CHECK-FIXES: {{^            }}void /*(*/ ( /*(*/ *f3)(int);
+            // CHECK-FIXES: {{^            }}void (*g3)(int, float);
+            
+        }
+    
+    struct S { int a; const int b; void f() {}};
+    
+    int S::*p = &S::a, S::* const q = &S::a;
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: multiple declarations should be split
+    // 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]]:5: warning: multiple declarations should be split
+    // CHECK-FIXES: {{^    }}int /* :: */ S::*pp2 = &S::a;
+    // CHECK-FIXES: {{^    }}int var1 = 0;
+    
+    const int S::*r = &S::b, S::*t;
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: multiple declarations should be split
+    // 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]]:9: warning: multiple declarations should be split
+        // 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]]:9: warning: multiple declarations should be split
+        // 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]]:9: warning: multiple declarations should be split
+        // 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]]:9: warning: multiple declarations should be split
+        // 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]]:9: warning: multiple declarations should be split
+        // CHECK-FIXES: {{^        }}void (S::**mdfpa3[2])() = {&mdfpa1[0], &mdfpa1[1]};
+        // CHECK-FIXES: {{^        }}void (S::*mdfpa4)() = &S::f;
+    }
+    
+    typedef const int S::*MemPtr;
+    MemPtr aaa =  &S::a, bbb = &S::b;
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: multiple declarations should be split
+    // CHECK-FIXES: {{^    }}MemPtr aaa =  &S::a;
+    // CHECK-FIXES: {{^    }}MemPtr bbb = &S::b;
+    
+    class CS { public: int a; const int b; };
+    int const CS :: * pp = &CS::a, CS::* const qq = &CS::a;
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: multiple declarations should be split
+    // CHECK-FIXES: {{^    }}int const CS :: * pp = &CS::a;
+    // CHECK-FIXES: {{^    }}int const CS::* const qq = &CS::a;
+    
+    int intfunction = returner(), intarray[] =
+          {
+                  1,
+                  2,
+                  3,
+                  4
+          }, bb = 4;
+    // CHECK-MESSAGES: :[[@LINE-7]]:5: warning: multiple declarations should be split
+    // 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;
+    
+    TemT *T1 = &TT1, *T2 = &TT2;
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: multiple declarations should be split
+    // CHECK-FIXES: {{^    }}TemT *T1 = &TT1;
+    // CHECK-FIXES: {{^    }}TemT *T2 = &TT2;
+
+    const PointerType *PT1 = T1->getAs<PointerType>(),
+                      *PT2 = T2->getAs<PointerType>();
+    // CHECK-MESSAGES: :[[@LINE-2]]:5: warning: multiple declarations should be split
+    // CHECK-FIXES: {{^    }}const PointerType *PT1 = T1->getAs<PointerType>();
+    // CHECK-FIXES: {{^    }}const PointerType *PT2 = T2->getAs<PointerType>();
+    
+    bool defPre = false,
+#ifdef IS_ENABLED
+       defTest = false;
+#else
+       defTest = true;
+#endif
+    // CHECK-MESSAGES: :[[@LINE-6]]:5: warning: multiple declarations should be split
+    // CHECK-FIXES: {{^    }}bool defPre = false;
+    // CHECK-FIXES: {{^    }}bool 
+    // CHECK-FIXES: {{^#}}ifdef IS_ENABLED
+    // CHECK-FIXES: {{^       }}defTest = false;
+    // CHECK-FIXES: {{^#}}else
+    // CHECK-FIXES: {{^       }}defTest = true;
+    // CHECK-FIXES: {{^#}}endif
+    
+    const int *p1 = nullptr;
+    const int *p2 = nullptr;
+    
+    const int *&pref1 = p1, *&pref2 = p2;
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: multiple declarations should be split
+    // CHECK-FIXES: {{^    }}const int *&pref1 = p1;
+    // CHECK-FIXES: {{^    }}const int *&pref2 = p2;
+    
+    typedef int *tptr, tbt;
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: multiple declarations should be split
+    // CHECK-FIXES: {{^    }}typedef int *tptr;
+    // CHECK-FIXES: {{^    }}typedef int tbt;
+    
+    typedef int (&tfp)(int, long), tarr[10];
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: multiple declarations should be split
+    // CHECK-FIXES: {{^    }}typedef int (&tfp)(int, long);
+    // CHECK-FIXES: {{^    }}typedef int tarr[10];
+    
+    typedef int tarr2[10], tct;
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: multiple declarations should be split
+    // CHECK-FIXES: {{^    }}typedef int tarr2[10];
+    // CHECK-FIXES: {{^    }}typedef int tct;
+
+}
+
+template <typename A, typename B>
+void should_not_be_touched(A, B);
+

test/clang-tidy/readability-one-name-per-declaration-modern.cpp

+// RUN: %check_clang_tidy %s readability-one-name-per-declaration %t -- -- \
+// RUN:   -std=c++1y
+
+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 Types {    
+    typedef int MyType;    
+    int dontTouch1, dontTouch2;
+}
+
+void modern() 
+{
+    auto autoInt1 = 3, autoInt2 = 4;
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: multiple declarations should be split
+    // CHECK-FIXES: {{^....}}auto autoInt1 = 3;
+    // CHECK-FIXES: {{^....}}auto autoInt2 = 4;
+    
+    decltype(int()) declnottouch= 4;
+    
+    decltype(int()) declint1 = 5, declint2 = 3;
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: multiple declarations should be split    
+    // 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]]:5: warning: multiple declarations should be split
+    // 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]]:5: warning: multiple declarations should be split
+    // CHECK-FIXES: {{^....}}uType utype1;
+    // CHECK-FIXES: {{^....}}uType utype2;
+    
+    Types::MyType mytype1, mytype2, mytype3 = 3;
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: multiple declarations should be split
+    // 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]]:9: warning: multiple declarations should be split
+        // 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" };
+    }
+    
+    struct X { int a, b, c; };
+    auto [a, b, c] = X();
+}
+

test/clang-tidy/readability-one-name-per-declaration-simple.cpp

+// RUN: %check_clang_tidy %s readability-one-name-per-declaration %t
+
+int cantTouchA, cantTouchB;
+
+void simple() 
+{
+    int dontTouchC;
+    
+    long empty;
+    long long1 = 11, *long2 = &empty, * long3 = ∅
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: multiple declarations should be split
+    // CHECK-FIXES: {{^    }}long long1 = 11;
+    // CHECK-FIXES: {{^    }}long *long2 = ∅
+    // CHECK-FIXES: {{^    }}long * long3 = ∅
+    
+    long ** lint1, lint2 = 0, * lint3, **linn;
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: multiple declarations should be split
+    // CHECK-FIXES: {{^    }}long ** lint1;
+    // CHECK-FIXES: {{^    }}long lint2 = 0;
+    // CHECK-FIXES: {{^    }}long * lint3;
+    // CHECK-FIXES: {{^    }}long **linn;
+    
+      long int* lint4, *lint5,  lint6;
+      // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: multiple declarations should be split
+      // CHECK-FIXES: {{^     }}long int* lint4;
+      // CHECK-FIXES: {{^     }}long int *lint5;
+      // CHECK-FIXES: {{^     }}long int lint6;
+    
+    /* *& */ int /* *& */ ** /* *& */ pp,*xx;
+    // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: multiple declarations should be split
+    // CHECK-FIXES: {{^    }}/* *& */ int /* *& */ ** /* *& */ pp;
+    // CHECK-FIXES: {{^    }}int *xx;
+    
+    unsigned int uint1 = 0,uint2 = 44u, uint3, uint4=4;
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: multiple declarations should be split
+    // CHECK-FIXES: {{^    }}unsigned int uint1 = 0;
+    // CHECK-FIXES: {{^    }}unsigned int uint2 = 44u;
+    // CHECK-FIXES: {{^    }}unsigned int uint3;
+    // CHECK-FIXES: {{^    }}unsigned int uint4=4;
+    
+    const int * const cpc = &dontTouchC, simple = 0;
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: multiple declarations should be split
+    // CHECK-FIXES: {{^    }}const int * const cpc = &dontTouchC;
+    // CHECK-FIXES: {{^    }}const int simple = 0;
+    
+    double darray1[] = {}, darray2[] = {1,  2},dv1 = 3,dv2;
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: multiple declarations should be split
+    // CHECK-FIXES: {{^    }}double darray1[] = {};
+    // CHECK-FIXES: {{^    }}double darray2[] = {1, 2};
+    // CHECK-FIXES: {{^    }}double dv1 = 3;
+    // CHECK-FIXES: {{^    }}double dv2;
+    
+    int notransform[] =   {
+                              1,
+                              2
+                          };
+    
+    const int cx = 1, cy = 2;
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: multiple declarations should be split
+    // CHECK-FIXES: {{^    }}const int cx = 1;
+    // CHECK-FIXES: {{^    }}const int cy = 2;
+    
+    volatile int vx, vy;
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: multiple declarations should be split
+    // CHECK-FIXES: {{^    }}volatile int vx;
+    // CHECK-FIXES: {{^    }}volatile int vy;
+    
+    signed char sc1 = 'h', sc2;
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: multiple declarations should be split
+    // CHECK-FIXES: {{^    }}signed char sc1 = 'h';
+    // CHECK-FIXES: {{^    }}signed char sc2;
+    
+    long long ll1, ll2, ***ft;
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: multiple declarations should be split
+    // CHECK-FIXES: {{^    }}long long ll1;
+    // CHECK-FIXES: {{^    }}long long ll2;
+    // CHECK-FIXES: {{^    }}long long ***ft;
+    
+    const char *cstr1 = "str1", *cstr2="str2";
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: multiple declarations should be split
+    // CHECK-FIXES: {{^    }}const char *cstr1 = "str1";
+    // CHECK-FIXES: {{^    }}const char *cstr2="str2";
+    
+    const char *literal1 = "clang"    "test" \
+                           "one",
+               *literal2 = "empty", literal3[] = "three";
+    // CHECK-MESSAGES: :[[@LINE-3]]:5: warning: multiple declarations should be split
+    // CHECK-FIXES: {{^    }}const char *literal1 = "clang"   "test" \
+    // CHECK-FIXES: {{^                           }}"one";
+    // CHECK-FIXES: {{^    }}const char *literal2 = "empty";
+    // CHECK-FIXES: {{^    }}const char literal3[] = "three";
+    
+    int intarray[] =
+          {
+                1,
+                    2,
+                    3,
+                    4
+          }, bb = 5;
+    // CHECK-MESSAGES: :[[@LINE-7]]:5: warning: multiple declarations should be split
+    // CHECK-FIXES: {{^    }}int intarray[] =
+    // CHECK-FIXES: {{^    }}      {
+    // CHECK-FIXES: {{^    }}             1,
+    // CHECK-FIXES: {{^    }}                2,
+    // CHECK-FIXES: {{^    }}                3,
+    // CHECK-FIXES: {{^    }}                4
+    // CHECK-FIXES: {{^    }}      };
+    // CHECK-FIXES: {{^    }}int bb = 5;
+    
+    const int cint3 = 4, cintarray[] = { 1, 2, 3, 4 };
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: multiple declarations should be split
+    // CHECK-FIXES: {{^    }}const int cint3 = 4;
+    // CHECK-FIXES: {{^    }}const int cintarray[] = { 1, 2, 3, 4 };
+
+    union Ss{
+        int m1;
+        float m2;
+    } in, out;
+    // CHECK-MESSAGES: :[[@LINE-4]]:5: warning: multiple declarations should be split
+    // CHECK-FIXES: {{^    }}Ss in;
+    // CHECK-FIXES: {{^    }}Ss out;
+    
+    enum E {} E1, E2;
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: multiple declarations should be split
+    // CHECK-FIXES: {{^    }}E E1;
+    // CHECK-FIXES: {{^    }}E E2;
+
+    struct S {int t;} S1 = {1}, S2;
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: multiple declarations should be split
+    // CHECK-FIXES: {{^    }}S S1 = {1};
+    // CHECK-FIXES: {{^    }}S S2;
+
+    class C {}C1, C2;
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: multiple declarations should be split
+    // CHECK-FIXES: {{^    }}C C1;
+    // CHECK-FIXES: {{^    }}C C2; 
+
+    struct B {} ignoreMe1;
+    enum {} ignoreMe2, ignoreMe3;
+    struct {} ignoreMe4, ignoreMe5;
+
+    typedef struct X { int t; } X, Y;
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: multiple declarations should be split
+    // CHECK-FIXES: {{^    }}typedef X X;
+    // CHECK-FIXES: {{^    }}typedef X Y;
+
+    
+    int refme1;
+    int refme2 = 0;
+    const int &r1 = refme1, &r2 = refme2;
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: multiple declarations should be split
+    // CHECK-FIXES: {{^    }}const int &r1 = refme1;
+    // CHECK-FIXES: {{^    }}const int &r2 = refme2;
+    
+    typedef int ta, tb;
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: multiple declarations should be split
+    // CHECK-FIXES: {{^    }}typedef int ta;
+    // CHECK-FIXES: {{^    }}typedef int tb;
+    
+    typedef const int tca, tcb;
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: multiple declarations should be split
+    // CHECK-FIXES: {{^    }}typedef const int tca;
+    // CHECK-FIXES: {{^    }}typedef const int tcb;
+    
+    typedef int const tac, tbc;
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: multiple declarations should be split
+    // CHECK-FIXES: {{^    }}typedef int const tac;
+    // CHECK-FIXES: {{^    }}typedef int const tbc;
+    
+    int *(i), (*j), (((k)));
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: multiple declarations should be split
+    // CHECK-FIXES: {{^    }}int *(i);
+    // CHECK-FIXES: {{^    }}int (*j);
+    // CHECK-FIXES: {{^    }}int (((k)));
+}
+