[PATCH] D97753: [clang-tidy] Add a check for enforcing minimum length for variable names

Florin Iucha via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Mar 2 18:43:30 PST 2021


0x8000-0000 marked 10 inline comments as done.
0x8000-0000 added a comment.

@Eugene.Zelenko and @njames93

Thank you both for the kind and thoughtful feedback. I am incorporating all recommended changes.

First step are all the smaller scope comments, with regex support in a follow-up commit.

I will also generate the diffs in the future using the -U99999 option as suggested.



================
Comment at: clang-tools-extra/clang-tidy/readability/VariableLengthCheck.cpp:58-62
+  Finder->addMatcher(
+      varDecl(unless(anyOf(hasParent(declStmt(hasParent(forStmt()))),
+                           hasParent(cxxCatchStmt()))))
+          .bind("standaloneVar"),
+      this);
----------------
njames93 wrote:
> This will miss the match for `k` in the same example:
> ```lang=c
> for (int i = 4, j = 5;;)
>   int k = 5;
> ```
> Gotta scratch my head for this one though. Though like the case before its rather unlikely to show up in code so not the end of the world.
> 
> Also varDecl will match parameters, is this intended can you add tests and explain in documentation.
> If its unintended putting `unless(parmVarDecl())` in the matcher disable that.
Matching of function parameter names is intentional. I will make sure it is emphasized in the documentation.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97753



More information about the cfe-commits mailing list