Index: clang-tools-extra/clang-tidy/readability/CMakeLists.txt =================================================================== --- clang-tools-extra/clang-tidy/readability/CMakeLists.txt +++ clang-tools-extra/clang-tidy/readability/CMakeLists.txt @@ -13,6 +13,7 @@ ElseAfterReturnCheck.cpp FunctionCognitiveComplexityCheck.cpp FunctionSizeCheck.cpp + IdentifierLengthCheck.cpp IdentifierNamingCheck.cpp ImplicitBoolConversionCheck.cpp InconsistentDeclarationParameterNameCheck.cpp Index: clang-tools-extra/clang-tidy/readability/IdentifierLengthCheck.h =================================================================== --- /dev/null +++ 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 Index: clang-tools-extra/clang-tidy/readability/IdentifierLengthCheck.cpp =================================================================== --- /dev/null +++ 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("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("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("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("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 Index: clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp =================================================================== --- clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp +++ 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 @@ "readability-function-cognitive-complexity"); CheckFactories.registerCheck( "readability-function-size"); + CheckFactories.registerCheck( + "readability-identifier-length"); CheckFactories.registerCheck( "readability-identifier-naming"); CheckFactories.registerCheck( Index: clang-tools-extra/docs/ReleaseNotes.rst =================================================================== --- clang-tools-extra/docs/ReleaseNotes.rst +++ clang-tools-extra/docs/ReleaseNotes.rst @@ -72,6 +72,11 @@ New checks ^^^^^^^^^^ +- New :doc:`readability-identifier-length + ` check. + + Reports identifiers whose names are too short. Currently checks local variables and function parameters only. + New check aliases ^^^^^^^^^^^^^^^^^ Index: clang-tools-extra/docs/clang-tidy/checks/list.rst =================================================================== --- clang-tools-extra/docs/clang-tidy/checks/list.rst +++ clang-tools-extra/docs/clang-tidy/checks/list.rst @@ -288,6 +288,7 @@ `readability-else-after-return `_, "Yes" `readability-function-cognitive-complexity `_, `readability-function-size `_, + `readability-identifier-length `_, `readability-identifier-naming `_, "Yes" `readability-implicit-bool-conversion `_, "Yes" `readability-inconsistent-declaration-parameter-name `_, "Yes" Index: clang-tools-extra/docs/clang-tidy/checks/readability-identifier-length.rst =================================================================== --- /dev/null +++ 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) { + // ... + } Index: clang-tools-extra/test/clang-tidy/checkers/readability-identifier-length.cpp =================================================================== --- /dev/null +++ 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 +}