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 +} _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
