readability-identifier-naming: fails to detect usage of member variable

[Quuxplusone]

Bugzilla Link	PR44447
Status	RESOLVED FIXED
Importance	P enhancement
Reported by	Cristian Morales Vega (christian.morales.vega@gmail.com)
Reported on	2020-01-02 13:45:52 -0800
Last modified on	2020-01-13 15:01:40 -0800
Version	unspecified
Hardware	PC Linux
CC	alexfh@google.com, djasper@google.com, klimek@google.com, N.James93@hotmail.co.uk
Fixed by commit(s)	rGfb79ef524171
Attachments
Blocks
Blocked by
See also

I have this class:

-------------------------
class la
{
  public:
    la(int number) : mNumber(number) {}

    int number() { return mNumber; }

  private:
    int mNumber;
};
-------------------------

Using 5c40544fa40bfb85ec888b6a03421b3905e4a4e7

clang-tidy --header-filter '.*' --checks=readability-identifier-naming -
config='{CheckOptions: [{key: readability-identifier-
naming.PrivateMemberSuffix, value: _}]}' --export-fixes=la.yaml la.cpp

generates this la.yaml file:

---
MainSourceFile:  'la.cpp'
Diagnostics:
  - DiagnosticName:  readability-identifier-naming
    DiagnosticMessage:
      Message:         'invalid case style for private member ''mNumber'''
      FilePath:        'la.cpp'
      FileOffset:      119
      Replacements:
        - FilePath:        'la.cpp'
          Offset:          42
          Length:          7
          ReplacementText: mNumber_
        - FilePath:        'la.cpp'
          Offset:          119
          Length:          7
          ReplacementText: mNumber_
...

So it correctly detects mNumber is used in the constructor, but fails to notice
its usage in the number() member function.

[Quuxplusone]

proposed fix is in the pipeline https://reviews.llvm.org/D72121

executing that yields the correct output
---
MainSourceFile:  'la.cpp'
Diagnostics:
  - DiagnosticName:  readability-identifier-naming
    DiagnosticMessage:
      Message:         'invalid case style for private member ''mNumber'''
      FilePath:        'la.cpp'
      FileOffset:      119
      Replacements:
        - FilePath:        'la.cpp'
          Offset:          42
          Length:          7
          ReplacementText: mNumber_
        - FilePath:        'la.cpp'
          Offset:          88
          Length:          7
          ReplacementText: mNumber_
        - FilePath:        'la.cpp'
          Offset:          119
          Length:          7
          ReplacementText: mNumber_
...

[Quuxplusone]

I would be happy to test it but not sure how to.
https://reviews.llvm.org/file/data/6psfhjp32p54ecwbm4c5/PHID-FILE-
xbs6tirf2nvlourrabc7/D72121.diff doesn't apply on top of
https://github.com/llvm-mirror/clang-tools-extra/commits/master.
How is this supposed to work?

[Quuxplusone]

I only created it a week ago from master branch(github repo), if there are conflicting changes I'll rebase and resubmit later on

[Quuxplusone]

Ahh I seen the issue, This patch was built on top of the official llvm github repo (https://github.com/llvm/llvm-project) the one you have from the mirror is rather out of date I'm afraid

[Quuxplusone]

Fixed in https://reviews.llvm.org/rGfb79ef524171c96a9f3df025ac7a8a3e00fdc0b4