[clang-tools-extra] d2c5cbc - Add a check for enforcing minimum length for variable names
Aaron Ballman via cfe-commits
cfe-commits at lists.llvm.org
Thu Aug 12 08:32:11 PDT 2021
Author: Florin Iucha
Date: 2021-08-12T11:31:26-04:00
New Revision: d2c5cbc3a856c2f8bbda05540fa761bb94ba32f6
URL: https://github.com/llvm/llvm-project/commit/d2c5cbc3a856c2f8bbda05540fa761bb94ba32f6
DIFF: https://github.com/llvm/llvm-project/commit/d2c5cbc3a856c2f8bbda05540fa761bb94ba32f6.diff
LOG: Add a check for enforcing minimum length for variable names
Add a check for enforcing minimum length for variable names. A default
minimum length of three characters is applied to regular variables
(including function parameters). Loop counters and exception variables
have a minimum of two characters. Additionally, the 'i', 'j' and 'k'
are accepted as legacy values.
All three sizes, as well as the list of accepted legacy loop counter
names are configurable.
Added:
clang-tools-extra/clang-tidy/readability/IdentifierLengthCheck.cpp
clang-tools-extra/clang-tidy/readability/IdentifierLengthCheck.h
clang-tools-extra/docs/clang-tidy/checks/readability-identifier-length.rst
clang-tools-extra/test/clang-tidy/checkers/readability-identifier-length.cpp
Modified:
clang-tools-extra/clang-tidy/readability/CMakeLists.txt
clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/docs/clang-tidy/checks/list.rst
Removed:
################################################################################
diff --git a/clang-tools-extra/clang-tidy/readability/CMakeLists.txt b/clang-tools-extra/clang-tidy/readability/CMakeLists.txt
index 0d35991d24799..78256d6f73251 100644
--- a/clang-tools-extra/clang-tidy/readability/CMakeLists.txt
+++ b/clang-tools-extra/clang-tidy/readability/CMakeLists.txt
@@ -13,6 +13,7 @@ add_clang_library(clangTidyReadabilityModule
ElseAfterReturnCheck.cpp
FunctionCognitiveComplexityCheck.cpp
FunctionSizeCheck.cpp
+ IdentifierLengthCheck.cpp
IdentifierNamingCheck.cpp
ImplicitBoolConversionCheck.cpp
InconsistentDeclarationParameterNameCheck.cpp
diff --git a/clang-tools-extra/clang-tidy/readability/IdentifierLengthCheck.cpp b/clang-tools-extra/clang-tidy/readability/IdentifierLengthCheck.cpp
new file mode 100644
index 0000000000000..1fd8624fcf4b2
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/readability/IdentifierLengthCheck.cpp
@@ -0,0 +1,156 @@
+//===--- IdentifierLengthCheck.cpp - clang-tidy
+//-----------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "IdentifierLengthCheck.h"
+#include "../utils/OptionsUtils.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace readability {
+
+const unsigned DefaultMinimumVariableNameLength = 3;
+const unsigned DefaultMinimumLoopCounterNameLength = 2;
+const unsigned DefaultMinimumExceptionNameLength = 2;
+const unsigned DefaultMinimumParameterNameLength = 3;
+const char DefaultIgnoredLoopCounterNames[] = "^[ijk_]$";
+const char DefaultIgnoredVariableNames[] = "";
+const char DefaultIgnoredExceptionVariableNames[] = "^[e]$";
+const char DefaultIgnoredParameterNames[] = "^[n]$";
+
+const char ErrorMessage[] =
+ "%select{variable|exception variable|loop variable|"
+ "parameter}0 name %1 is too short, expected at least %2 characters";
+
+IdentifierLengthCheck::IdentifierLengthCheck(StringRef Name,
+ ClangTidyContext *Context)
+ : ClangTidyCheck(Name, Context),
+ MinimumVariableNameLength(Options.get("MinimumVariableNameLength",
+ DefaultMinimumVariableNameLength)),
+ MinimumLoopCounterNameLength(Options.get(
+ "MinimumLoopCounterNameLength", DefaultMinimumLoopCounterNameLength)),
+ MinimumExceptionNameLength(Options.get(
+ "MinimumExceptionNameLength", DefaultMinimumExceptionNameLength)),
+ MinimumParameterNameLength(Options.get(
+ "MinimumParameterNameLength", DefaultMinimumParameterNameLength)),
+ IgnoredVariableNamesInput(
+ Options.get("IgnoredVariableNames", DefaultIgnoredVariableNames)),
+ IgnoredVariableNames(IgnoredVariableNamesInput),
+ IgnoredLoopCounterNamesInput(Options.get("IgnoredLoopCounterNames",
+ DefaultIgnoredLoopCounterNames)),
+ IgnoredLoopCounterNames(IgnoredLoopCounterNamesInput),
+ IgnoredExceptionVariableNamesInput(
+ Options.get("IgnoredExceptionVariableNames",
+ DefaultIgnoredExceptionVariableNames)),
+ IgnoredExceptionVariableNames(IgnoredExceptionVariableNamesInput),
+ IgnoredParameterNamesInput(
+ Options.get("IgnoredParameterNames", DefaultIgnoredParameterNames)),
+ IgnoredParameterNames(IgnoredParameterNamesInput) {}
+
+void IdentifierLengthCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
+ Options.store(Opts, "MinimumVariableNameLength", MinimumVariableNameLength);
+ Options.store(Opts, "MinimumLoopCounterNameLength",
+ MinimumLoopCounterNameLength);
+ Options.store(Opts, "MinimumExceptionNameLength", MinimumExceptionNameLength);
+ Options.store(Opts, "MinimumParameterNameLength", MinimumParameterNameLength);
+ Options.store(Opts, "IgnoredLoopCounterNames", IgnoredLoopCounterNamesInput);
+ Options.store(Opts, "IgnoredVariableNames", IgnoredVariableNamesInput);
+ Options.store(Opts, "IgnoredExceptionVariableNames",
+ IgnoredExceptionVariableNamesInput);
+ Options.store(Opts, "IgnoredParameterNames", IgnoredParameterNamesInput);
+}
+
+void IdentifierLengthCheck::registerMatchers(MatchFinder *Finder) {
+ if (MinimumLoopCounterNameLength > 1)
+ Finder->addMatcher(
+ forStmt(hasLoopInit(declStmt(forEach(varDecl().bind("loopVar"))))),
+ this);
+
+ if (MinimumExceptionNameLength > 1)
+ Finder->addMatcher(varDecl(hasParent(cxxCatchStmt())).bind("exceptionVar"),
+ this);
+
+ if (MinimumParameterNameLength > 1)
+ Finder->addMatcher(parmVarDecl().bind("paramVar"), this);
+
+ if (MinimumVariableNameLength > 1)
+ Finder->addMatcher(
+ varDecl(unless(anyOf(hasParent(declStmt(hasParent(forStmt()))),
+ hasParent(cxxCatchStmt()), parmVarDecl())))
+ .bind("standaloneVar"),
+ this);
+}
+
+void IdentifierLengthCheck::check(const MatchFinder::MatchResult &Result) {
+ const auto *StandaloneVar = Result.Nodes.getNodeAs<VarDecl>("standaloneVar");
+ if (StandaloneVar) {
+ if (!StandaloneVar->getIdentifier())
+ return;
+
+ StringRef VarName = StandaloneVar->getName();
+
+ if (VarName.size() >= MinimumVariableNameLength ||
+ IgnoredVariableNames.match(VarName))
+ return;
+
+ diag(StandaloneVar->getLocation(), ErrorMessage)
+ << 0 << StandaloneVar << MinimumVariableNameLength;
+ }
+
+ auto *ExceptionVarName = Result.Nodes.getNodeAs<VarDecl>("exceptionVar");
+ if (ExceptionVarName) {
+ if (!ExceptionVarName->getIdentifier())
+ return;
+
+ StringRef VarName = ExceptionVarName->getName();
+ if (VarName.size() >= MinimumExceptionNameLength ||
+ IgnoredExceptionVariableNames.match(VarName))
+ return;
+
+ diag(ExceptionVarName->getLocation(), ErrorMessage)
+ << 1 << ExceptionVarName << MinimumExceptionNameLength;
+ }
+
+ const auto *LoopVar = Result.Nodes.getNodeAs<VarDecl>("loopVar");
+ if (LoopVar) {
+ if (!LoopVar->getIdentifier())
+ return;
+
+ StringRef VarName = LoopVar->getName();
+
+ if (VarName.size() >= MinimumLoopCounterNameLength ||
+ IgnoredLoopCounterNames.match(VarName))
+ return;
+
+ diag(LoopVar->getLocation(), ErrorMessage)
+ << 2 << LoopVar << MinimumLoopCounterNameLength;
+ }
+
+ const auto *ParamVar = Result.Nodes.getNodeAs<VarDecl>("paramVar");
+ if (ParamVar) {
+ if (!ParamVar->getIdentifier())
+ return;
+
+ StringRef VarName = ParamVar->getName();
+
+ if (VarName.size() >= MinimumParameterNameLength ||
+ IgnoredParameterNames.match(VarName))
+ return;
+
+ diag(ParamVar->getLocation(), ErrorMessage)
+ << 3 << ParamVar << MinimumParameterNameLength;
+ }
+}
+
+} // namespace readability
+} // namespace tidy
+} // namespace clang
diff --git a/clang-tools-extra/clang-tidy/readability/IdentifierLengthCheck.h b/clang-tools-extra/clang-tidy/readability/IdentifierLengthCheck.h
new file mode 100644
index 0000000000000..ade722bba837a
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/readability/IdentifierLengthCheck.h
@@ -0,0 +1,54 @@
+//===--- IdentifierLengthCheck.h - clang-tidy ---------------------*- C++
+//-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_IDENTIFIERLENGTHCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_IDENTIFIERLENGTHCHECK_H
+
+#include "../ClangTidyCheck.h"
+#include "llvm/Support/Regex.h"
+
+namespace clang {
+namespace tidy {
+namespace readability {
+
+/// Warns about identifiers names whose length is too short.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/readability-identifier-length.html
+class IdentifierLengthCheck : public ClangTidyCheck {
+public:
+ IdentifierLengthCheck(StringRef Name, ClangTidyContext *Context);
+ void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
+ void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+ void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+
+private:
+ const unsigned MinimumVariableNameLength;
+ const unsigned MinimumLoopCounterNameLength;
+ const unsigned MinimumExceptionNameLength;
+ const unsigned MinimumParameterNameLength;
+
+ std::string IgnoredVariableNamesInput;
+ llvm::Regex IgnoredVariableNames;
+
+ std::string IgnoredLoopCounterNamesInput;
+ llvm::Regex IgnoredLoopCounterNames;
+
+ std::string IgnoredExceptionVariableNamesInput;
+ llvm::Regex IgnoredExceptionVariableNames;
+
+ std::string IgnoredParameterNamesInput;
+ llvm::Regex IgnoredParameterNames;
+};
+
+} // namespace readability
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_IDENTIFIERLENGTHCHECK_H
diff --git a/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp b/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp
index a05b70826e669..366541a0ed487 100644
--- a/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp
+++ b/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp
@@ -18,6 +18,7 @@
#include "ElseAfterReturnCheck.h"
#include "FunctionCognitiveComplexityCheck.h"
#include "FunctionSizeCheck.h"
+#include "IdentifierLengthCheck.h"
#include "IdentifierNamingCheck.h"
#include "ImplicitBoolConversionCheck.h"
#include "InconsistentDeclarationParameterNameCheck.h"
@@ -73,6 +74,8 @@ class ReadabilityModule : public ClangTidyModule {
"readability-function-cognitive-complexity");
CheckFactories.registerCheck<FunctionSizeCheck>(
"readability-function-size");
+ CheckFactories.registerCheck<IdentifierLengthCheck>(
+ "readability-identifier-length");
CheckFactories.registerCheck<IdentifierNamingCheck>(
"readability-identifier-naming");
CheckFactories.registerCheck<ImplicitBoolConversionCheck>(
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index afa685d1fbdd9..1cbb79fbab2c6 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -72,6 +72,11 @@ The improvements are...
New checks
^^^^^^^^^^
+- New :doc:`readability-identifier-length
+ <clang-tidy/checks/readability-identifier-length>` check.
+
+ Reports identifiers whose names are too short. Currently checks local variables and function parameters only.
+
New check aliases
^^^^^^^^^^^^^^^^^
diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst
index ebc99ca8f0eec..ee0c6e3c55a01 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -288,6 +288,7 @@ Clang-Tidy Checks
`readability-else-after-return <readability-else-after-return.html>`_, "Yes"
`readability-function-cognitive-complexity <readability-function-cognitive-complexity.html>`_,
`readability-function-size <readability-function-size.html>`_,
+ `readability-identifier-length <readability-identifier-length.html>`_,
`readability-identifier-naming <readability-identifier-naming.html>`_, "Yes"
`readability-implicit-bool-conversion <readability-implicit-bool-conversion.html>`_, "Yes"
`readability-inconsistent-declaration-parameter-name <readability-inconsistent-declaration-parameter-name.html>`_, "Yes"
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
new file mode 100644
index 0000000000000..8e31c997ceb70
--- /dev/null
+++ b/clang-tools-extra/docs/clang-tidy/checks/readability-identifier-length.rst
@@ -0,0 +1,122 @@
+.. title:: clang-tidy - readability-identifier-length
+
+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.
+
+Options
+-------
+
+The following options are described below:
+
+ - :option:`MinimumVariableNameLength`, :option:`IgnoredVariableNames`
+ - :option:`MinimumParameterNameLength`, :option:`IgnoredParameterNames`
+ - :option:`MinimumLoopCounterNameLength`, :option:`IgnoredLoopCounterNames`
+ - :option:`MinimumExceptionNameLength`, :option:`IgnoredExceptionVariableNames`
+
+.. option:: MinimumVariableNameLength
+
+ All variables (other than loop counter, exception names and function
+ parameters) are expected to have at least a length of
+ `MinimumVariableNameLength` (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
+ {
+ return 2 * x;
+ }
+
+ 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
+ to be ignored. The default value is empty, thus no names are ignored.
+
+.. option:: MinimumParameterNameLength
+
+ All function parameter names are expected to have a length of at least
+ `MinimumParameterNameLength` (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:: IgnoredParameterNames
+
+ Specifies a regular expression for parameters that are to be ignored.
+ The default value is `^[n]$` for historical reasons.
+
+.. option:: MinimumLoopCounterNameLength
+
+ Loop counter variables are expected to have a length of at least
+ `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.
+ for (int q = 0; q < size; ++ q) {
+ // ...
+ }
+
+.. option:: IgnoredLoopCounterNames
+
+ Specifies a regular expression for counter names that are to be ignored.
+ The default value is `^[ijk_]$`; the first three symbols for historical
+ 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.
+ for (int i = 0; i < size; ++ i) {
+ // ...
+ }
+
+.. option:: MinimumExceptionNameLength
+
+ Exception clause variables are expected to have a length of at least
+ `MinimumExceptionNameLength` (default is `2`). Setting it to `0` or `1`
+ disables the check entirely.
+
+
+ .. code-block:: c++
+
+ try {
+ // ...
+ }
+ // This warns that 'e' is too short.
+ catch (const std::exception& x) {
+ // ...
+ }
+
+.. option:: IgnoredExceptionVariableNames
+
+ Specifies a regular expression for exception variable names that are to
+ be ignored. The default value is `^[e]$` mainly for historical reasons.
+
+ .. code-block:: c++
+
+ try {
+ // ...
+ }
+ // This does not warn by default, for historical reasons.
+ catch (const std::exception& e) {
+ // ...
+ }
diff --git a/clang-tools-extra/test/clang-tidy/checkers/readability-identifier-length.cpp b/clang-tools-extra/test/clang-tidy/checkers/readability-identifier-length.cpp
new file mode 100644
index 0000000000000..b9e2e756dd78c
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/checkers/readability-identifier-length.cpp
@@ -0,0 +1,63 @@
+// RUN: %check_clang_tidy %s readability-identifier-length %t \
+// RUN: -config='{CheckOptions: \
+// RUN: [{key: readability-identifier-length.IgnoredVariableNames, value: "^[xy]$"}]}' \
+// RUN: --
+
+struct myexcept {
+ int val;
+};
+
+struct simpleexcept {
+ int other;
+};
+
+void doIt();
+
+void tooShortVariableNames(int z)
+// CHECK-MESSAGES: :[[@LINE-1]]:32: warning: parameter name 'z' is too short, expected at least 3 characters [readability-identifier-length]
+{
+ int i = 5;
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: variable name 'i' is too short, expected at least 3 characters [readability-identifier-length]
+
+ int jj = z;
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: variable name 'jj' is too short, expected at least 3 characters [readability-identifier-length]
+
+ for (int m = 0; m < 5; ++m)
+ // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: loop variable name 'm' is too short, expected at least 2 characters [readability-identifier-length]
+ {
+ doIt();
+ }
+
+ try {
+ doIt();
+ } catch (const myexcept &x)
+ // CHECK-MESSAGES: :[[@LINE-1]]:28: warning: exception variable name 'x' is too short, expected at least 2 characters [readability-identifier-length]
+ {
+ doIt();
+ }
+}
+
+void longEnoughVariableNames(int n) // argument 'n' ignored by default configuration
+{
+ int var = 5;
+
+ for (int i = 0; i < 42; ++i) // 'i' is default allowed, for historical reasons
+ {
+ doIt();
+ }
+
+ for (int kk = 0; kk < 42; ++kk) {
+ doIt();
+ }
+
+ try {
+ doIt();
+ } catch (const simpleexcept &e) // ignored by default configuration
+ {
+ doIt();
+ } catch (const myexcept &ex) {
+ doIt();
+ }
+
+ int x = 5; // ignored by configuration
+}
More information about the cfe-commits
mailing list