[PATCH] D86671: [clang-tidy] Add new case type to check variables with Hungarian notation

[Douglas Chen via Phabricator via cfe-commits]

dougpuob added a comment.

In D86671#2246570 <https://reviews.llvm.org/D86671#2246570>, @njames93 wrote:

> As Hungarian notation enforces prefixes as well as casing styles it would be wise to warn on cases where the style is Hungarian but a prefix has also been set.
>
> Another issue is this current set up will let you define any decls style as hungarian, this doesn't make sense for most decl types. 
> Potentially some validation should happen for that too,

Hi @njames93:
If decl types were not in the `HungarianNotationTable` table, current implementation will treat it as `CamelCase`(UpperCamel), because the prefix is empty.



================
Comment at: clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.h:127
   virtual llvm::Optional<FailureInfo>
-  GetDeclFailureInfo(const NamedDecl *Decl, const SourceManager &SM) const = 0;
+  GetDeclFailureInfo(const StringRef &Type, const NamedDecl *Decl,
+                     const SourceManager &SM) const = 0;
----------------
njames93 wrote:
> I'm not a fan of changing the base class for this support. The Type paramater can be inferred using the Decl. 
> You can `dyn_cast` the Decl to a `ValueDecl` and then get its type from that.
Agree with you. This change is possible to make it without changing the interface of function. Improve it in next patch. Thank you.


================
Comment at: clang-tools-extra/docs/ReleaseNotes.rst:73
 
+- Added: Support variables could be checked with Hungarian Notation which the
+  prefix encodes the actual data type of the variable.
----------------
Eugene.Zelenko wrote:
> dougpuob wrote:
> > Eugene.Zelenko wrote:
> > > See examples for entry format below.
> > Make it like the following ?
> > 
> > ```
> > Changes in existing checks
> > ^^^^^^^^^^^^^^^^^^^^^^^^^^
> > 
> > - Improved :doc:`readability-identifier-naming
> >   <clang-tidy/checks/readability-identifier-naming>` check.
> > 
> >   Added an option `GetConfigPerFile` to support including files which use
> >   different naming styles.
> > 
> > - Improved :doc:`readability-identifier-naming
> >   <clang-tidy/checkers/readability-identifier-naming-hungarian-notation.cpp>` check.  
> > 
> >   Support variables could be checked with Hungarian Notation which the prefix
> >   encodes the actual data type of the variable.
> > ```
> Yes, but link must be `clang-tidy/checks/readability-identifier-naming`, because it refer to documentation file, not regression test.
 OK~ Fix it in the next patch. Thank you.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D86671