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.

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

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

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

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 @@ public:
         "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>(

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
 ^^^^^^^^^^^^^^^^^
 

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"

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) {
+          // ...
+      }

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
+}