[clang-tools-extra] [clang-tidy][readability-identifier-length] refactoring and cleanup (PR #194610)
via cfe-commits
cfe-commits at lists.llvm.org
Tue Apr 28 06:00:24 PDT 2026
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang-tools-extra
Author: Alex Dutka (dutkalex)
<details>
<summary>Changes</summary>
This PR implements the refactorings discussed with @<!-- -->localspook in #<!-- -->193838
---
Full diff: https://github.com/llvm/llvm-project/pull/194610.diff
3 Files Affected:
- (modified) clang-tools-extra/clang-tidy/readability/IdentifierLengthCheck.cpp (+43-81)
- (modified) clang-tools-extra/clang-tidy/readability/IdentifierLengthCheck.h (+5-5)
- (modified) clang-tools-extra/docs/clang-tidy/checks/readability/identifier-length.rst (+3-17)
``````````diff
diff --git a/clang-tools-extra/clang-tidy/readability/IdentifierLengthCheck.cpp b/clang-tools-extra/clang-tidy/readability/IdentifierLengthCheck.cpp
index 8dd238ac8e595..8031cbeb00a51 100644
--- a/clang-tools-extra/clang-tidy/readability/IdentifierLengthCheck.cpp
+++ b/clang-tools-extra/clang-tidy/readability/IdentifierLengthCheck.cpp
@@ -138,99 +138,61 @@ static bool isShortLived(const ValueDecl *Var, const SourceManager *SrcMgr,
}
void IdentifierLengthCheck::check(const MatchFinder::MatchResult &Result) {
- const auto *StandaloneVar =
- Result.Nodes.getNodeAs<ValueDecl>("standaloneVar");
- if (StandaloneVar) {
- if (!StandaloneVar->getIdentifier())
- return;
+ auto ShouldWarn = [&](const ValueDecl *Var, unsigned MinNameLength,
+ llvm::Regex const &IgnoredNames) -> bool {
+ if (!Var->getIdentifier())
+ return false;
- const StringRef VarName = StandaloneVar->getName();
+ const StringRef VarName = Var->getName();
+ if (VarName.size() >= MinNameLength || IgnoredNames.match(VarName))
+ return false;
- if (VarName.size() >= MinimumVariableNameLength ||
- IgnoredVariableNames.match(VarName))
- return;
-
- if (isShortLived(StandaloneVar, Result.SourceManager, Result.Context,
+ if (isShortLived(Var, Result.SourceManager, Result.Context,
LineCountThreshold))
- return;
+ return false;
- diag(StandaloneVar->getLocation(), ErrorMessage)
- << 0 << StandaloneVar << MinimumVariableNameLength;
+ return true;
+ };
+
+ if (const auto *StandaloneVar =
+ Result.Nodes.getNodeAs<ValueDecl>("standaloneVar")) {
+ if (ShouldWarn(StandaloneVar, MinimumVariableNameLength,
+ IgnoredVariableNames))
+ diag(StandaloneVar->getLocation(), ErrorMessage)
+ << 0 << StandaloneVar << MinimumVariableNameLength;
+ return;
}
- const auto *BindingVar = Result.Nodes.getNodeAs<ValueDecl>("bindingVar");
- if (BindingVar) {
- if (!BindingVar->getIdentifier())
- return;
-
- const StringRef VarName = BindingVar->getName();
-
- if (VarName.size() >= MinimumBindingNameLength ||
- IgnoredBindingNames.match(VarName))
- return;
-
- if (isShortLived(BindingVar, Result.SourceManager, Result.Context,
- LineCountThreshold))
- return;
-
- diag(BindingVar->getLocation(), ErrorMessage)
- << 1 << BindingVar << MinimumBindingNameLength;
+ if (const auto *BindingVar =
+ Result.Nodes.getNodeAs<ValueDecl>("bindingVar")) {
+ if (ShouldWarn(BindingVar, MinimumBindingNameLength, IgnoredBindingNames))
+ diag(BindingVar->getLocation(), ErrorMessage)
+ << 1 << BindingVar << MinimumBindingNameLength;
+ return;
}
- auto *ExceptionVarName = Result.Nodes.getNodeAs<ValueDecl>("exceptionVar");
- if (ExceptionVarName) {
- if (!ExceptionVarName->getIdentifier())
- return;
-
- const StringRef VarName = ExceptionVarName->getName();
- if (VarName.size() >= MinimumExceptionNameLength ||
- IgnoredExceptionVariableNames.match(VarName))
- return;
-
- if (isShortLived(ExceptionVarName, Result.SourceManager, Result.Context,
- LineCountThreshold))
- return;
-
- diag(ExceptionVarName->getLocation(), ErrorMessage)
- << 2 << ExceptionVarName << MinimumExceptionNameLength;
+ if (const auto *ExceptionVar =
+ Result.Nodes.getNodeAs<ValueDecl>("exceptionVar")) {
+ if (ShouldWarn(ExceptionVar, MinimumExceptionNameLength,
+ IgnoredExceptionVariableNames))
+ diag(ExceptionVar->getLocation(), ErrorMessage)
+ << 2 << ExceptionVar << MinimumExceptionNameLength;
+ return;
}
- const auto *LoopVar = Result.Nodes.getNodeAs<ValueDecl>("loopVar");
- if (LoopVar) {
- if (!LoopVar->getIdentifier())
- return;
-
- const StringRef VarName = LoopVar->getName();
-
- if (VarName.size() >= MinimumLoopCounterNameLength ||
- IgnoredLoopCounterNames.match(VarName))
- return;
-
- if (isShortLived(LoopVar, Result.SourceManager, Result.Context,
- LineCountThreshold))
- return;
-
- diag(LoopVar->getLocation(), ErrorMessage)
- << 3 << LoopVar << MinimumLoopCounterNameLength;
+ if (const auto *LoopVar = Result.Nodes.getNodeAs<ValueDecl>("loopVar")) {
+ if (ShouldWarn(LoopVar, MinimumLoopCounterNameLength,
+ IgnoredLoopCounterNames))
+ diag(LoopVar->getLocation(), ErrorMessage)
+ << 3 << LoopVar << MinimumLoopCounterNameLength;
+ return;
}
- const auto *ParamVar = Result.Nodes.getNodeAs<ValueDecl>("paramVar");
- if (ParamVar) {
- if (!ParamVar->getIdentifier())
- return;
-
- const StringRef VarName = ParamVar->getName();
-
- if (VarName.size() >= MinimumParameterNameLength ||
- IgnoredParameterNames.match(VarName))
- return;
-
- if (isShortLived(ParamVar, Result.SourceManager, Result.Context,
- LineCountThreshold))
- return;
-
- diag(ParamVar->getLocation(), ErrorMessage)
- << 4 << ParamVar << MinimumParameterNameLength;
+ if (const auto *ParamVar = Result.Nodes.getNodeAs<ValueDecl>("paramVar")) {
+ if (ShouldWarn(ParamVar, MinimumParameterNameLength, IgnoredParameterNames))
+ diag(ParamVar->getLocation(), ErrorMessage)
+ << 4 << ParamVar << MinimumParameterNameLength;
+ return;
}
}
diff --git a/clang-tools-extra/clang-tidy/readability/IdentifierLengthCheck.h b/clang-tools-extra/clang-tidy/readability/IdentifierLengthCheck.h
index 8fd762971b01b..4c9aa1e355493 100644
--- a/clang-tools-extra/clang-tidy/readability/IdentifierLengthCheck.h
+++ b/clang-tools-extra/clang-tidy/readability/IdentifierLengthCheck.h
@@ -32,19 +32,19 @@ class IdentifierLengthCheck : public ClangTidyCheck {
const unsigned MinimumExceptionNameLength;
const unsigned MinimumParameterNameLength;
- std::string IgnoredVariableNamesInput;
+ llvm::StringRef IgnoredVariableNamesInput;
llvm::Regex IgnoredVariableNames;
- std::string IgnoredBindingNamesInput;
+ llvm::StringRef IgnoredBindingNamesInput;
llvm::Regex IgnoredBindingNames;
- std::string IgnoredLoopCounterNamesInput;
+ llvm::StringRef IgnoredLoopCounterNamesInput;
llvm::Regex IgnoredLoopCounterNames;
- std::string IgnoredExceptionVariableNamesInput;
+ llvm::StringRef IgnoredExceptionVariableNamesInput;
llvm::Regex IgnoredExceptionVariableNames;
- std::string IgnoredParameterNamesInput;
+ llvm::StringRef IgnoredParameterNamesInput;
llvm::Regex IgnoredParameterNames;
const unsigned LineCountThreshold;
diff --git a/clang-tools-extra/docs/clang-tidy/checks/readability/identifier-length.rst b/clang-tools-extra/docs/clang-tidy/checks/readability/identifier-length.rst
index 2e19897bcab66..f08dbb89404c6 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/readability/identifier-length.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/readability/identifier-length.rst
@@ -4,9 +4,9 @@ readability-identifier-length
=============================
This check finds variables and function parameters whose length are too short.
-The desired name length is configurable.
-
-Special cases are supported for loop counters and for exception variable names.
+The desired name length is configurable. Special short names which should be
+ignored can be specified. Local variables with short names can also be ignored
+if they are short-lived. This check offers no automated fix.
Options
-------
@@ -28,14 +28,10 @@ The following options are described below:
`MinimumVariableNameLength` (default is `3`). Setting it to `0` or `1`
disables the check entirely.
-
.. code-block:: c++
int i = 42; // warns that 'i' is too short
- This check does not have any fix suggestions in the general case since
- variable names have semantic value.
-
.. option:: IgnoredVariableNames
Specifies a regular expression for variable names that are
@@ -51,9 +47,6 @@ The following options are described below:
auto [a] = get_result(); // warns that 'a' is too short
- This check does not have any fix suggestions in the general case since
- variable names have semantic value.
-
.. option:: IgnoredBindingNames
Specifies a regular expression for variable names introduced by structured
@@ -66,7 +59,6 @@ The following options are described below:
`MinimumParameterNameLength` (default is `3`). Setting it to `0` or `1`
disables the check entirely.
-
.. code-block:: c++
int doubler(int x) // warns that x is too short
@@ -74,9 +66,6 @@ The following options are described below:
return 2 * x;
}
- This check does not have any fix suggestions in the general case since
- variable names have semantic value.
-
.. option:: IgnoredParameterNames
Specifies a regular expression for parameters that are to be ignored.
@@ -88,7 +77,6 @@ The following options are described below:
`MinimumLoopCounterNameLength` characters (default is `2`). Setting it to
`0` or `1` disables the check entirely.
-
.. code-block:: c++
// This warns that 'q' is too short.
@@ -103,7 +91,6 @@ The following options are described below:
reasons and the last one since it is frequently used as a "don't care"
value, specifically in tools such as Google Benchmark.
-
.. code-block:: c++
// This does not warn by default, for historical reasons.
@@ -117,7 +104,6 @@ The following options are described below:
`MinimumExceptionNameLength` (default is `2`). Setting it to `0` or `1`
disables the check entirely.
-
.. code-block:: c++
try {
``````````
</details>
https://github.com/llvm/llvm-project/pull/194610
More information about the cfe-commits
mailing list