[clang-tidy] Fix readability-identifer-naming Hungarian CString options

[amurzeau]

Summary

When reading readability-identifier-naming.HungarianNotation.CString
options, correctly use the type string stored in CStr.second instead of
the option name (CStr.first) as the HNOption.CString map key.

This will make CString options really working and properly parsed by the
checker.

[carlosgalvezp]

Please document fix in the Release Notes, as people might have adopted to the buggy behavior and should know they need to change to the proper one.

clang-tools-extra/test/clang-tidy/checkers/readability/Inputs/identifier-naming/hungarian-notation2/.clang-tidy

118–125 ↗

(On Diff #498954)

These changes generate a lot of review noise and don't seem to be related to the fix in the patch (which has to do with HungarianNotation.General. Please revert them and if wanted make another patch just for that.

[amurzeau]

Remove test changes, I will make a different patch for that.

[amurzeau]

Add a release note for this fix.

[carlosgalvezp]

clang-tools-extra/test/clang-tidy/checkers/readability/Inputs/identifier-naming/hungarian-notation2/.clang-tidy

115 ↗

(On Diff #498954)

Sorry, maybe I wasn't clear. Isn't this line meant to stay, since this is what the patch is fixing?

[amurzeau]

clang-tools-extra/test/clang-tidy/checkers/readability/Inputs/identifier-naming/hungarian-notation2/.clang-tidy

115 ↗

(On Diff #498954)

The patch only fixes reading readability-identifier-naming.HungarianNotation.CString.* options.

I've updated the test completely to make options parsing issues tested.
The test was not catching the issue with HungarianNotation.CString.* because all options were set to their default value in this .clang-tidy file, so the test couldn't really check that the option was really correctly read (and it turns out it wasn't for HungarianNotation.CString.*).

This one (readability-identifier-naming.HungarianNotation.General.TreatStructAsClass) is correctly read, but was just misnamed in this test file with Options instead of General.
I found that while updating this test.

Both the code and docs currently use readability-identifier-naming.HungarianNotation.General.TreatStructAsClass.

The minimal changes in this test that reproduce the exact issue that is fixed here are the changes on readability-identifier-naming.HungarianNotation.CString.* options.
But I found it weird to just test them without updating all other options as well, that's why I've updated all options instead.

[carlosgalvezp]

Alright, sounds good, thanks for the clarification! Then I misunderstood the change in the tests and indeed it belonged together in this patch, sorry for the confusion. I have approved both patches so we can land them now. Can you land the patch or would you like me to do it for you? If so please provide name and email (same as your Github account) for attribution.

[amurzeau]

In D144431#4146524, @carlosgalvezp wrote:

Alright, sounds good, thanks for the clarification! Then I misunderstood the change in the tests and indeed it belonged together in this patch, sorry for the confusion. I have approved both patches so we can land them now. Can you land the patch or would you like me to do it for you? If so please provide name and email (same as your Github account) for attribution.

No worries, yes please land the patches for me as Alexis Murzeau <[e-mail]> (phabricator mail and handle is the same as github).
Thanks!

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

void IdentifierNamingCheck::HungarianNotation::loadFileConfig(
 
   Buffer = {Section, "CString."};
   DefSize = Buffer.size();
   for (const auto &CStr : HNCStrings) {
     Buffer.truncate(DefSize);
     Buffer.append(CStr.first);
     StringRef Val = Options.get(Buffer, "");
     if (!Val.empty())
-      HNOption.CString[CStr.first] = Val.str();
+      HNOption.CString[CStr.second] = Val.str();
   }
 
   Buffer = {Section, "PrimitiveType."};
   DefSize = Buffer.size();
   for (const auto &PrimType : HungarainNotationPrimitiveTypes) {
     Buffer.truncate(DefSize);
     Buffer.append(PrimType);
     StringRef Val = Options.get(Buffer, "");

clang-tools-extra/docs/ReleaseNotes.rst

- Deprecated check-local options `HeaderFileExtensions`
   <clang-tidy/checks/misc/definitions-in-headers>` check.
   Global options of the same name should be used instead.
 
 - Deprecated check-local options `HeaderFileExtensions`
   in :doc:`misc-unused-using-decls
   <clang-tidy/checks/misc/unused-using-decls>` check.
   Global options of the same name should be used instead.
 
+- Fixed reading `HungarianNotation.CString.*` options in
+  :doc:`readability-identifier-naming
+  <clang-tidy/checks/readability/identifier-naming>` check.
+
 Removed checks
 ^^^^^^^^^^^^^^
 
 Improvements to include-fixer
 -----------------------------
 
 The improvements are...