[clang-tidy] rule limitation: readability-identifier-naming doesn't allow distinguishing between class and struct members

[tothambrus11]

In our company, the style guide says class members should be postfixed by _, regardless whether they are public/private, but struct members shouldn't be postfixed. In the readability-identifier-naming rule, there are rules for Member* and a ClassMember*, but there are no rules for StructMember*.

I failed to understand from the documentation whether ClassMember* is supposed to match both structs and classes, or what the difference is between Member* and ClassMember*. Regardless, it would be nice to add a new matcher that unambiguously matches only struct members.

If somebody could guide me on the current semantics and where I should contribute, I would be happy to implement the fixes.

[llvmbot]

@llvm/issue-subscribers-clang-tidy

Author: Ambrus Tóth (tothambrus11)

In our company, the style guide says class members should be postfixed by `_`, regardless whether they are public/private, but struct members shouldn't be postfixed. In the `readability-identifier-naming` rule, there are rules for `Member*` and a `ClassMember*`, but there are no rules for `StructMember*`.

I failed to understand from the documentation whether ClassMember* is supposed to match both structs and classes, or what the difference is between Member* and ClassMember*. Regardless, it would be nice to add a new matcher that unambiguously matches only struct members.

If somebody could guide me where I should contribute, I would be happy to implement the fixes.

[vbvictor]

I can't answer directly to the question, but you can twist values of ClassMemberCase and MemberCase in
clang-tools-extra\test\clang-tidy\checkers\readability\identifier-naming.cpp to see what tests will fail.
To get started, read https://clang.llvm.org/extra/clang-tidy/Contributing.html#preparing-your-workspace

[jitre]

I’m facing the same issue.

[jitre]

Besides developing a custom checker, are there any other ideas?

[carlosgalvezp]

No, it would not make sense to create a custom checker. If this feature is truly wanted, the existing check should be extended.

IMHO, given that nobody has requested this feature for years until now, one might consider challenging your company guidelines. class and struct are the exact same thing, the only difference is the default visibility. What matters in terms of naming convention is the visibility of the member, not if it's inside a class or a struct.

When you are in this context:

void f(Foo foo)
{
  int x = foo.x;
}

You can't know whether Foo is a class or struct, and you shouldn't need to - that's an implementation detail. All that matters is the API of Foo. If you write foo.x_ instead of foo.x, it's clear you are accessing a private member, which you then can reason about whether it's the right thing to do or not in that context.

[tothambrus11]

Yes, I'm not particularly enthusiastic about those guidelines, it indeed feels inconsistent. The rules have been set 6 years ago in a department on the other side of the world, and since then nobody touched the rules (said to be impossible). Luckily, the teams in my department mostly don't enforce these...