This is an archive of the discontinued LLVM Phabricator instance.

[clang-tidy] Update IdentifierNamingCheck to remove extra leading/trailing underscores
ClosedPublic

Authored by jtbandes on Apr 20 2017, 7:31 PM.

Details

Summary

The goal of this change is to fix the following suboptimal replacements currently suggested by clang-tidy:

// with MemberPrefix == "_"
int __foo;  // accepted without complaint
// with MemberPrefix == "m_"
int _foo;
    ^~~~~~
    m__foo

I fixed this by

  • updating matchesStyle() to reject names which have a leading underscore after a prefix has already been stripped, or a trailing underscore if a suffix has already been stripped;
  • updating fixupWithStyle() to strip leading & trailing underscores before adding the user-defined prefix and suffix.

The replacements are now:

// MemberPrefix == "_"
int __foo;
    ^~~~~~
    _foo
// MemberPrefix == "m_"
int _foo;
    ^~~~~
    m_foo

Future improvements might elect to add .clang-tidy flags to improve what is being stripped. For instance, stripping m_ could allow m_foo to be automatically replaced with _foo.

Event Timeline

jtbandes created this revision.Apr 20 2017, 7:31 PM
jtbandes updated this revision to Diff 96180.Apr 21 2017, 10:44 AM
jtbandes edited the summary of this revision. (Show Details)

Remove unnecessary checks for empty prefix/suffix

alexfh accepted this revision.Apr 26 2017, 6:37 AM
LG with a nit.
clang-tidy/readability/IdentifierNamingCheck.cpp
378

Mid.empty()

This revision is now accepted and ready to land.Apr 26 2017, 6:37 AM
jtbandes updated this revision to Diff 96762.Apr 26 2017, 9:33 AM

Fixed nit

jtbandes marked an inline comment as done.Apr 26 2017, 9:36 AM

Done, thanks for the review!

What is the procedure for merging patches in? I'm sure I don't have permissions to do it myself.

Done, thanks for the review!

What is the procedure for merging patches in? I'm sure I don't have permissions to do it myself.

I'll commit the patch for you.

Thank you for working on this!

This revision was automatically updated to reflect the committed changes.