llvm / llvm-project

The LLVM Project is a collection of modular and reusable compiler and toolchain technologies.
http://llvm.org
Other
29.15k stars 12.03k forks source link

[clang-tidy] Disable `readability-identifier-naming.HungarianNotation.PrimitiveType` #58937

Open GTruf opened 2 years ago

GTruf commented 2 years ago

I want to leave only readability-identifier-naming.HungarianNotation.DerivedType.Pointer, how can I disable Hungarian notation for primitive types or at least explicitly give them all an empty prefix? In my opinion, it's very oddly done that if I just want to add a prefix p to all pointers (whether it's a class field or any other pointer), then I'll also have to face those prefixes for all the primitive types I don't need. I use clang-tidy 15.0.0.

For example, I just want int* m_somePointerField to be renamed m_pSomePointerField, but not m_piSomePointerField.

- { key: readability-identifier-naming.HungarianNotation.PrimitiveType.int, value: "" } doesn't work.

I know that this problem is sort of solved here #56358 (https://reviews.llvm.org/D129070), but your repository does not contain these changes.

llvmbot commented 2 years ago

@llvm/issue-subscribers-clang-tidy

GTruf commented 2 years ago

I was able to change this behavior by carefully studying the code (I rebuilt and checked, everything worked).

But before you look at my changes and remarks, I would strongly recommend that you simply separate class fields into "class field and class pointer field" like you do with other variables, e.g. GlobalPointerPrefix and GlobalVariablePrefix. Similarly, it could be like this (using private variables as an example) : PrivateMemberPrefix and PrivateMemberPointerPrefix.

Therefore, to be able to set empty prefixes, you need to remove the empty value check for line 483 in the clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp file:

// ...
for (const auto &PrimType : HungarainNotationPrimitiveTypes) {
    Buffer.truncate(DefSize);
    Buffer.append(PrimType);
    StringRef Val = Options.get(Buffer, "");
    //if (!Val.empty()) {   <--- remove this check
      std::string Type = PrimType.str();
      std::replace(Type.begin(), Type.end(), '-', ' ');
      HNOption.PrimitiveType[Type] = Val.str();
    //} <---
  }
// ...

In that case it won't take the default prefixes from the table, but it will be possible not to specify them and have an empty prefix if you don't need it (I haven't looked that deeply into your code, I think you can fix it all).

There are also two other problems, one of which is a bug (first): 1) I found out by logging parameter Case (method IdentifierNamingCheck::fixupWithCase) that in this case somePointerVar is equated to a public member (in this case there will be a compilation error due to assigning 7 to the pointer, but still):

class SomeClass
{
private:
    int16_t someVar = 4;
    int8_t* somePointerVar = 7;
    std::string someStringVar;
};

2) If I fundamentally want to have class fields in camelBack prefixed with m_, I write the following settings:

- { key: readability-identifier-naming.PrivateMemberCase, value: camelBack }
- { key: readability-identifier-naming.PrivateMemberPrefix, value: m_ }
- { key: readability-identifier-naming.PrivateMemberHungarianPrefix, value: On }

Then by the following example:

class SomeClass
{
private:
    int16_t someVar = 4;
    int8_t* somePointerVar = nullptr;
    std::string someStringVar;
};

Hence, someVar it will change to m_someVar, but somePointerVar it will change to m_psomePointerVar instead of making m_pSomePointerVar, so it should allow this setting.

/home/g.trufanov/repositories/SomeClass.hpp:9:13: warning: invalid case style for private member 'someVar' [readability-identifier-naming]
    int16_t someVar = 4;
            ^~~~~~~
            m_someVar
/home/g.trufanov/repositories/SomeClass.hpp:10:13: warning: invalid case style for private member 'somePointerVar' [readability-identifier-naming]
    int8_t* somePointerVar = nullptr;
            ^~~~~~~~~~~~~~
            m_psomePointerVar
/home/g.trufanov/repositories/SomeClass.hpp:11:17: warning: invalid case style for private member 'someStringVar' [readability-identifier-naming]
    std::string someStringVar;
                ^~~~~~~~~~~~~
                m_someStringVar