Re: [clang-tools-extra] fb79ef5 - Fix readability-identifier-naming missing member variables

[Nico Weber via cfe-commits]

Great, thanks!

On Mon, Jan 13, 2020 at 4:04 PM Aaron Ballman 
wrote:

> On Mon, Jan 13, 2020 at 2:25 PM Nico Weber  wrote:
> >
> > This breaks tests on Windows: http://45.33.8.238/win/5636/step_8.txt
> >
> > Likely the usual "doesn't work with delayed template parsing" thing.
> >
> > Can you take a look, and if it takes a while to fix, revert while you
> investigate?
> >
> > (I wouldn't said this on the phab issue, but couldn't find one.)
>
> Thank you for pointing this out, it's fixed in
> c1b13a1b17719aebace1b3be7a6ac7f90b1901a6
>
> ~Aaron
>
> >
> > On Mon, Jan 13, 2020 at 1:29 PM Aaron Ballman via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
> >>
> >>
> >> Author: Nathan James
> >> Date: 2020-01-13T13:28:55-05:00
> >> New Revision: fb79ef524171c96a9f3df025ac7a8a3e00fdc0b4
> >>
> >> URL:
> https://github.com/llvm/llvm-project/commit/fb79ef524171c96a9f3df025ac7a8a3e00fdc0b4
> >> DIFF:
> https://github.com/llvm/llvm-project/commit/fb79ef524171c96a9f3df025ac7a8a3e00fdc0b4.diff
> >>
> >> LOG: Fix readability-identifier-naming missing member variables
> >>
> >> Fixes PR41122 (missing fixes for member variables in a destructor) and
> >> PR29005 (does not rename class members in all locations).
> >>
> >> Added:
> >>
>  
> clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-member-decl-usage.cpp
> >>
> >> Modified:
> >> clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
> >>
> >> Removed:
> >>
> >>
> >>
> >>
> 
> >> diff  --git
> a/clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
> b/clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
> >> index bd736743ae1c..8146cb36cf21 100644
> >> --- a/clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
> >> +++ b/clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
> >> @@ -237,10 +237,26 @@ void
> IdentifierNamingCheck::registerMatchers(MatchFinder *Finder) {
> >>Finder->addMatcher(namedDecl().bind("decl"), this);
> >>Finder->addMatcher(usingDecl().bind("using"), this);
> >>Finder->addMatcher(declRefExpr().bind("declRef"), this);
> >> -  Finder->addMatcher(cxxConstructorDecl().bind("classRef"), this);
> >> -  Finder->addMatcher(cxxDestructorDecl().bind("classRef"), this);
> >> +
> Finder->addMatcher(cxxConstructorDecl(unless(isImplicit())).bind("classRef"),
> >> + this);
> >> +
> Finder->addMatcher(cxxDestructorDecl(unless(isImplicit())).bind("classRef"),
> >> + this);
> >>Finder->addMatcher(typeLoc().bind("typeLoc"), this);
> >>Finder->addMatcher(nestedNameSpecifierLoc().bind("nestedNameLoc"),
> this);
> >> +  Finder->addMatcher(
> >> +  functionDecl(unless(cxxMethodDecl(isImplicit())),
> >> +
>  hasBody(forEachDescendant(memberExpr().bind("memberExpr",
> >> +  this);
> >> +  Finder->addMatcher(
> >> +  cxxConstructorDecl(
> >> +  unless(isImplicit()),
> >> +  forEachConstructorInitializer(
> >> +  allOf(isWritten(), withInitializer(forEachDescendant(
> >> +
>  memberExpr().bind("memberExpr")),
> >> +  this);
> >> +  Finder->addMatcher(fieldDecl(hasInClassInitializer(
> >> +
>  forEachDescendant(memberExpr().bind("memberExpr",
> >> + this);
> >>  }
> >>
> >>  void IdentifierNamingCheck::registerPPCallbacks(
> >> @@ -710,8 +726,6 @@ static void
> addUsage(IdentifierNamingCheck::NamingCheckFailureMap ,
> >>  void IdentifierNamingCheck::check(const MatchFinder::MatchResult
> ) {
> >>if (const auto *Decl =
> >>Result.Nodes.getNodeAs("classRef")) {
> >> -if (Decl->isImplicit())
> >> -  return;
> >>
> >>  addUsage(NamingCheckFailures, Decl->getParent(),
> >>   Decl->getNameInfo().getSourceRange());
> >> @@ -730,8 +744,6 @@ void IdentifierNamingCheck::check(const
> MatchFinder::MatchResult ) {
> >>
> >>if (const auto *Decl =
> >>Result.Nodes.getNodeAs("classRef")) {
> >> -if (Decl->isImplicit())
> >> -  return;
> >>
> >>  SourceRange Range = Decl->getNameInfo().getSourceRange();
> >>  if (Range.getBegin().isInvalid())
> >> @@ -806,6 +818,14 @@ void IdentifierNamingCheck::check(const
> MatchFinder::MatchResult ) {
> >>  return;
> >>}
> >>
> >> +  if (const auto *MemberRef =
> >> +  Result.Nodes.getNodeAs("memberExpr")) {
> >> +SourceRange Range =
> MemberRef->getMemberNameInfo().getSourceRange();
> >> +addUsage(NamingCheckFailures, MemberRef->getMemberDecl(), Range,
> >> + Result.SourceManager);
> >> +return;
> >> +  }
> >> +
> >>if (const auto *Decl = Result.Nodes.getNodeAs("decl")) {
> >>  if (!Decl->getIdentifier() || Decl->getName().empty() ||
> Decl->isImplicit())
> >>return;
> >>
> >> diff  --git
> a/clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-member-decl-usage.cpp
> 

Re: [clang-tools-extra] fb79ef5 - Fix readability-identifier-naming missing member variables

[Aaron Ballman via cfe-commits]

On Mon, Jan 13, 2020 at 2:25 PM Nico Weber  wrote:
>
> This breaks tests on Windows: http://45.33.8.238/win/5636/step_8.txt
>
> Likely the usual "doesn't work with delayed template parsing" thing.
>
> Can you take a look, and if it takes a while to fix, revert while you 
> investigate?
>
> (I wouldn't said this on the phab issue, but couldn't find one.)

Thank you for pointing this out, it's fixed in
c1b13a1b17719aebace1b3be7a6ac7f90b1901a6

~Aaron

>
> On Mon, Jan 13, 2020 at 1:29 PM Aaron Ballman via cfe-commits 
>  wrote:
>>
>>
>> Author: Nathan James
>> Date: 2020-01-13T13:28:55-05:00
>> New Revision: fb79ef524171c96a9f3df025ac7a8a3e00fdc0b4
>>
>> URL: 
>> https://github.com/llvm/llvm-project/commit/fb79ef524171c96a9f3df025ac7a8a3e00fdc0b4
>> DIFF: 
>> https://github.com/llvm/llvm-project/commit/fb79ef524171c96a9f3df025ac7a8a3e00fdc0b4.diff
>>
>> LOG: Fix readability-identifier-naming missing member variables
>>
>> Fixes PR41122 (missing fixes for member variables in a destructor) and
>> PR29005 (does not rename class members in all locations).
>>
>> Added:
>> 
>> clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-member-decl-usage.cpp
>>
>> Modified:
>> clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
>>
>> Removed:
>>
>>
>>
>> 
>> diff  --git 
>> a/clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp 
>> b/clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
>> index bd736743ae1c..8146cb36cf21 100644
>> --- a/clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
>> +++ b/clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
>> @@ -237,10 +237,26 @@ void 
>> IdentifierNamingCheck::registerMatchers(MatchFinder *Finder) {
>>Finder->addMatcher(namedDecl().bind("decl"), this);
>>Finder->addMatcher(usingDecl().bind("using"), this);
>>Finder->addMatcher(declRefExpr().bind("declRef"), this);
>> -  Finder->addMatcher(cxxConstructorDecl().bind("classRef"), this);
>> -  Finder->addMatcher(cxxDestructorDecl().bind("classRef"), this);
>> +  
>> Finder->addMatcher(cxxConstructorDecl(unless(isImplicit())).bind("classRef"),
>> + this);
>> +  
>> Finder->addMatcher(cxxDestructorDecl(unless(isImplicit())).bind("classRef"),
>> + this);
>>Finder->addMatcher(typeLoc().bind("typeLoc"), this);
>>Finder->addMatcher(nestedNameSpecifierLoc().bind("nestedNameLoc"), this);
>> +  Finder->addMatcher(
>> +  functionDecl(unless(cxxMethodDecl(isImplicit())),
>> +   
>> hasBody(forEachDescendant(memberExpr().bind("memberExpr",
>> +  this);
>> +  Finder->addMatcher(
>> +  cxxConstructorDecl(
>> +  unless(isImplicit()),
>> +  forEachConstructorInitializer(
>> +  allOf(isWritten(), withInitializer(forEachDescendant(
>> + memberExpr().bind("memberExpr")),
>> +  this);
>> +  Finder->addMatcher(fieldDecl(hasInClassInitializer(
>> + 
>> forEachDescendant(memberExpr().bind("memberExpr",
>> + this);
>>  }
>>
>>  void IdentifierNamingCheck::registerPPCallbacks(
>> @@ -710,8 +726,6 @@ static void 
>> addUsage(IdentifierNamingCheck::NamingCheckFailureMap ,
>>  void IdentifierNamingCheck::check(const MatchFinder::MatchResult ) {
>>if (const auto *Decl =
>>Result.Nodes.getNodeAs("classRef")) {
>> -if (Decl->isImplicit())
>> -  return;
>>
>>  addUsage(NamingCheckFailures, Decl->getParent(),
>>   Decl->getNameInfo().getSourceRange());
>> @@ -730,8 +744,6 @@ void IdentifierNamingCheck::check(const 
>> MatchFinder::MatchResult ) {
>>
>>if (const auto *Decl =
>>Result.Nodes.getNodeAs("classRef")) {
>> -if (Decl->isImplicit())
>> -  return;
>>
>>  SourceRange Range = Decl->getNameInfo().getSourceRange();
>>  if (Range.getBegin().isInvalid())
>> @@ -806,6 +818,14 @@ void IdentifierNamingCheck::check(const 
>> MatchFinder::MatchResult ) {
>>  return;
>>}
>>
>> +  if (const auto *MemberRef =
>> +  Result.Nodes.getNodeAs("memberExpr")) {
>> +SourceRange Range = MemberRef->getMemberNameInfo().getSourceRange();
>> +addUsage(NamingCheckFailures, MemberRef->getMemberDecl(), Range,
>> + Result.SourceManager);
>> +return;
>> +  }
>> +
>>if (const auto *Decl = Result.Nodes.getNodeAs("decl")) {
>>  if (!Decl->getIdentifier() || Decl->getName().empty() || 
>> Decl->isImplicit())
>>return;
>>
>> diff  --git 
>> a/clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-member-decl-usage.cpp
>>  
>> b/clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-member-decl-usage.cpp
>> new file mode 100644
>> index ..caffc3283ca2
>> --- /dev/null
>> +++ 
>> 

Re: [clang-tools-extra] fb79ef5 - Fix readability-identifier-naming missing member variables

[Nico Weber via cfe-commits]

This breaks tests on Windows: http://45.33.8.238/win/5636/step_8.txt

Likely the usual "doesn't work with delayed template parsing" thing.

Can you take a look, and if it takes a while to fix, revert while you
investigate?

(I wouldn't said this on the phab issue, but couldn't find one.)

On Mon, Jan 13, 2020 at 1:29 PM Aaron Ballman via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

>
> Author: Nathan James
> Date: 2020-01-13T13:28:55-05:00
> New Revision: fb79ef524171c96a9f3df025ac7a8a3e00fdc0b4
>
> URL:
> https://github.com/llvm/llvm-project/commit/fb79ef524171c96a9f3df025ac7a8a3e00fdc0b4
> DIFF:
> https://github.com/llvm/llvm-project/commit/fb79ef524171c96a9f3df025ac7a8a3e00fdc0b4.diff
>
> LOG: Fix readability-identifier-naming missing member variables
>
> Fixes PR41122 (missing fixes for member variables in a destructor) and
> PR29005 (does not rename class members in all locations).
>
> Added:
>
> clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-member-decl-usage.cpp
>
> Modified:
> clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
>
> Removed:
>
>
>
>
> 
> diff  --git
> a/clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
> b/clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
> index bd736743ae1c..8146cb36cf21 100644
> --- a/clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
> +++ b/clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
> @@ -237,10 +237,26 @@ void
> IdentifierNamingCheck::registerMatchers(MatchFinder *Finder) {
>Finder->addMatcher(namedDecl().bind("decl"), this);
>Finder->addMatcher(usingDecl().bind("using"), this);
>Finder->addMatcher(declRefExpr().bind("declRef"), this);
> -  Finder->addMatcher(cxxConstructorDecl().bind("classRef"), this);
> -  Finder->addMatcher(cxxDestructorDecl().bind("classRef"), this);
> +
> Finder->addMatcher(cxxConstructorDecl(unless(isImplicit())).bind("classRef"),
> + this);
> +
> Finder->addMatcher(cxxDestructorDecl(unless(isImplicit())).bind("classRef"),
> + this);
>Finder->addMatcher(typeLoc().bind("typeLoc"), this);
>Finder->addMatcher(nestedNameSpecifierLoc().bind("nestedNameLoc"),
> this);
> +  Finder->addMatcher(
> +  functionDecl(unless(cxxMethodDecl(isImplicit())),
> +
>  hasBody(forEachDescendant(memberExpr().bind("memberExpr",
> +  this);
> +  Finder->addMatcher(
> +  cxxConstructorDecl(
> +  unless(isImplicit()),
> +  forEachConstructorInitializer(
> +  allOf(isWritten(), withInitializer(forEachDescendant(
> + memberExpr().bind("memberExpr")),
> +  this);
> +  Finder->addMatcher(fieldDecl(hasInClassInitializer(
> +
>  forEachDescendant(memberExpr().bind("memberExpr",
> + this);
>  }
>
>  void IdentifierNamingCheck::registerPPCallbacks(
> @@ -710,8 +726,6 @@ static void
> addUsage(IdentifierNamingCheck::NamingCheckFailureMap ,
>  void IdentifierNamingCheck::check(const MatchFinder::MatchResult )
> {
>if (const auto *Decl =
>Result.Nodes.getNodeAs("classRef")) {
> -if (Decl->isImplicit())
> -  return;
>
>  addUsage(NamingCheckFailures, Decl->getParent(),
>   Decl->getNameInfo().getSourceRange());
> @@ -730,8 +744,6 @@ void IdentifierNamingCheck::check(const
> MatchFinder::MatchResult ) {
>
>if (const auto *Decl =
>Result.Nodes.getNodeAs("classRef")) {
> -if (Decl->isImplicit())
> -  return;
>
>  SourceRange Range = Decl->getNameInfo().getSourceRange();
>  if (Range.getBegin().isInvalid())
> @@ -806,6 +818,14 @@ void IdentifierNamingCheck::check(const
> MatchFinder::MatchResult ) {
>  return;
>}
>
> +  if (const auto *MemberRef =
> +  Result.Nodes.getNodeAs("memberExpr")) {
> +SourceRange Range = MemberRef->getMemberNameInfo().getSourceRange();
> +addUsage(NamingCheckFailures, MemberRef->getMemberDecl(), Range,
> + Result.SourceManager);
> +return;
> +  }
> +
>if (const auto *Decl = Result.Nodes.getNodeAs("decl")) {
>  if (!Decl->getIdentifier() || Decl->getName().empty() ||
> Decl->isImplicit())
>return;
>
> diff  --git
> a/clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-member-decl-usage.cpp
> b/clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-member-decl-usage.cpp
> new file mode 100644
> index ..caffc3283ca2
> --- /dev/null
> +++
> b/clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-member-decl-usage.cpp
> @@ -0,0 +1,137 @@
> +// RUN: %check_clang_tidy %s readability-identifier-naming %t -- \
> +// RUN:   -config='{CheckOptions: [ \
> +// RUN: {key: readability-identifier-naming.MemberCase, value:
> CamelCase}, \
> +// RUN: {key: 

[clang-tools-extra] fb79ef5 - Fix readability-identifier-naming missing member variables

[Aaron Ballman via cfe-commits]

Author: Nathan James
Date: 2020-01-13T13:28:55-05:00
New Revision: fb79ef524171c96a9f3df025ac7a8a3e00fdc0b4

URL: 
https://github.com/llvm/llvm-project/commit/fb79ef524171c96a9f3df025ac7a8a3e00fdc0b4
DIFF: 
https://github.com/llvm/llvm-project/commit/fb79ef524171c96a9f3df025ac7a8a3e00fdc0b4.diff

LOG: Fix readability-identifier-naming missing member variables

Fixes PR41122 (missing fixes for member variables in a destructor) and
PR29005 (does not rename class members in all locations).

Added: 

clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-member-decl-usage.cpp

Modified: 
clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp

Removed: 




diff  --git 
a/clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp 
b/clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
index bd736743ae1c..8146cb36cf21 100644
--- a/clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
+++ b/clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
@@ -237,10 +237,26 @@ void IdentifierNamingCheck::registerMatchers(MatchFinder 
*Finder) {
   Finder->addMatcher(namedDecl().bind("decl"), this);
   Finder->addMatcher(usingDecl().bind("using"), this);
   Finder->addMatcher(declRefExpr().bind("declRef"), this);
-  Finder->addMatcher(cxxConstructorDecl().bind("classRef"), this);
-  Finder->addMatcher(cxxDestructorDecl().bind("classRef"), this);
+  Finder->addMatcher(cxxConstructorDecl(unless(isImplicit())).bind("classRef"),
+ this);
+  Finder->addMatcher(cxxDestructorDecl(unless(isImplicit())).bind("classRef"),
+ this);
   Finder->addMatcher(typeLoc().bind("typeLoc"), this);
   Finder->addMatcher(nestedNameSpecifierLoc().bind("nestedNameLoc"), this);
+  Finder->addMatcher(
+  functionDecl(unless(cxxMethodDecl(isImplicit())),
+   
hasBody(forEachDescendant(memberExpr().bind("memberExpr",
+  this);
+  Finder->addMatcher(
+  cxxConstructorDecl(
+  unless(isImplicit()),
+  forEachConstructorInitializer(
+  allOf(isWritten(), withInitializer(forEachDescendant(
+ memberExpr().bind("memberExpr")),
+  this);
+  Finder->addMatcher(fieldDecl(hasInClassInitializer(
+ forEachDescendant(memberExpr().bind("memberExpr",
+ this);
 }
 
 void IdentifierNamingCheck::registerPPCallbacks(
@@ -710,8 +726,6 @@ static void 
addUsage(IdentifierNamingCheck::NamingCheckFailureMap ,
 void IdentifierNamingCheck::check(const MatchFinder::MatchResult ) {
   if (const auto *Decl =
   Result.Nodes.getNodeAs("classRef")) {
-if (Decl->isImplicit())
-  return;
 
 addUsage(NamingCheckFailures, Decl->getParent(),
  Decl->getNameInfo().getSourceRange());
@@ -730,8 +744,6 @@ void IdentifierNamingCheck::check(const 
MatchFinder::MatchResult ) {
 
   if (const auto *Decl =
   Result.Nodes.getNodeAs("classRef")) {
-if (Decl->isImplicit())
-  return;
 
 SourceRange Range = Decl->getNameInfo().getSourceRange();
 if (Range.getBegin().isInvalid())
@@ -806,6 +818,14 @@ void IdentifierNamingCheck::check(const 
MatchFinder::MatchResult ) {
 return;
   }
 
+  if (const auto *MemberRef =
+  Result.Nodes.getNodeAs("memberExpr")) {
+SourceRange Range = MemberRef->getMemberNameInfo().getSourceRange();
+addUsage(NamingCheckFailures, MemberRef->getMemberDecl(), Range,
+ Result.SourceManager);
+return;
+  }
+
   if (const auto *Decl = Result.Nodes.getNodeAs("decl")) {
 if (!Decl->getIdentifier() || Decl->getName().empty() || 
Decl->isImplicit())
   return;

diff  --git 
a/clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-member-decl-usage.cpp
 
b/clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-member-decl-usage.cpp
new file mode 100644
index ..caffc3283ca2
--- /dev/null
+++ 
b/clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-member-decl-usage.cpp
@@ -0,0 +1,137 @@
+// RUN: %check_clang_tidy %s readability-identifier-naming %t -- \
+// RUN:   -config='{CheckOptions: [ \
+// RUN: {key: readability-identifier-naming.MemberCase, value: CamelCase}, 
\
+// RUN: {key: readability-identifier-naming.ParameterCase, value: 
CamelCase} \
+// RUN:  ]}'
+
+int set_up(int);
+int clear(int);
+
+class Foo {
+public:
+  const int bar_baz; // comment-0
+  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: invalid case style for member 
'bar_baz'
+  // CHECK-FIXES: {{^}}  const int BarBaz; // comment-0
+
+  Foo(int Val) : bar_baz(Val) { // comment-1
+// CHECK-FIXES: {{^}}  Foo(int Val) : BarBaz(Val) { // comment-1
+set_up(bar_baz); // comment-2
+// CHECK-FIXES: {{^}}set_up(BarBaz); // comment-2
+  }
+
+  Foo() : Foo(0) {}
+
+  ~Foo() {
+