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

Alexis Murzeau via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Feb 22 11:30:02 PST 2023


amurzeau marked an inline comment as done.
amurzeau added inline comments.


================
Comment at: clang-tools-extra/test/clang-tidy/checkers/readability/Inputs/identifier-naming/hungarian-notation2/.clang-tidy:115
     value:  On
-  - key:    readability-identifier-naming.HungarianNotation.Options.TreatStructAsClass
-    value:  false
+  - key:    readability-identifier-naming.HungarianNotation.General.TreatStructAsClass
+    value:  true
----------------
carlosgalvezp wrote:
> Sorry, maybe I wasn't clear. Isn't this line meant to stay, since this is what the patch is fixing?
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.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D144431/new/

https://reviews.llvm.org/D144431



More information about the cfe-commits mailing list