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 @@ -44,6 +44,7 @@ UniqueptrDeleteReleaseCheck.cpp UppercaseLiteralSuffixCheck.cpp UseAnyOfAllOfCheck.cpp + VariableLengthCheck.cpp LINK_LIBS clangTidy 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 @@ -47,6 +47,7 @@ #include "UniqueptrDeleteReleaseCheck.h" #include "UppercaseLiteralSuffixCheck.h" #include "UseAnyOfAllOfCheck.h" +#include "VariableLengthCheck.h" namespace clang { namespace tidy { @@ -131,6 +132,8 @@ "readability-uppercase-literal-suffix"); CheckFactories.registerCheck( "readability-use-anyofallof"); + CheckFactories.registerCheck( + "readability-variable-length"); } }; Index: clang-tools-extra/clang-tidy/readability/VariableLengthCheck.h =================================================================== --- /dev/null +++ clang-tools-extra/clang-tidy/readability/VariableLengthCheck.h @@ -0,0 +1,43 @@ +//===--- VariableLengthCheck.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_VARIABLELENGTHCHECK_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_VARIABLELENGTHCHECK_H + +#include "../ClangTidyCheck.h" + +#include + +namespace clang { +namespace tidy { +namespace readability { + +/// Warns about variable names whose length is too short +/// +/// For the user-facing documentation see: +/// http://clang.llvm.org/extra/clang-tidy/checks/readability-variable-length.html +class VariableLengthCheck : public ClangTidyCheck { +public: + VariableLengthCheck(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; + + std::unordered_set IgnoredLoopCounterNames; +}; + +} // namespace readability +} // namespace tidy +} // namespace clang + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_VARIABLELENGTHCHECK_H Index: clang-tools-extra/clang-tidy/readability/VariableLengthCheck.cpp =================================================================== --- /dev/null +++ clang-tools-extra/clang-tidy/readability/VariableLengthCheck.cpp @@ -0,0 +1,111 @@ +//===--- VariableLengthCheck.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 "VariableLengthCheck.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 char DefaultIgnoredLoopCounterNames[] = "i;j;k;"; + +VariableLengthCheck::VariableLengthCheck(StringRef Name, + ClangTidyContext *Context) + : ClangTidyCheck(Name, Context), MinimumVariableNameLength{Options.get( + "MinimumVariableNameLength", + DefaultMinimumVariableNameLength)}, + MinimumLoopCounterNameLength{Options.get( + "MinimumLoopCounterNameLength", DefaultMinimumLoopCounterNameLength)}, + MinimumExceptionNameLength{Options.get( + "MinimumExceptionNameLength", DefaultMinimumExceptionNameLength)} { + const auto Values = utils::options::parseStringList( + Options.get("IgnoredLoopCounterNames", DefaultIgnoredLoopCounterNames)); + IgnoredLoopCounterNames.reserve(Values.size()); + for (const auto &Value : Values) { + IgnoredLoopCounterNames.insert(Value); + } +} + +void VariableLengthCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) { + Options.store(Opts, "MinimumVariableNameLength", + DefaultMinimumVariableNameLength); + Options.store(Opts, "MinimumLoopCounterNameLength", + DefaultMinimumLoopCounterNameLength); + Options.store(Opts, "MinimumExceptionNameLength", + DefaultMinimumExceptionNameLength); + Options.store(Opts, "IgnoredLoopCounterNames", + DefaultIgnoredLoopCounterNames); +} + +void VariableLengthCheck::registerMatchers(MatchFinder *Finder) { + Finder->addMatcher( + varDecl(hasParent(declStmt(hasParent(forStmt())))).bind("loopVar"), this); + Finder->addMatcher(varDecl(hasParent(cxxCatchStmt())).bind("exceptionVar"), + this); + Finder->addMatcher( + varDecl(unless(anyOf(hasParent(declStmt(hasParent(forStmt()))), + hasParent(cxxCatchStmt())))) + .bind("standaloneVar"), + this); +} + +void VariableLengthCheck::check(const MatchFinder::MatchResult &Result) { + const auto *StandaloneVar = Result.Nodes.getNodeAs("standaloneVar"); + if (StandaloneVar) { + if (!StandaloneVar->getIdentifier() || + StandaloneVar->getName().size() >= MinimumVariableNameLength) + return; + + diag(StandaloneVar->getLocation(), + "variable name %0 is too short, expected at least %1 characters") + << StandaloneVar << MinimumVariableNameLength; + } + + const auto *ExceptionVar = Result.Nodes.getNodeAs("exceptionVar"); + if (ExceptionVar) { + if (!ExceptionVar->getIdentifier() || + ExceptionVar->getName().size() >= MinimumExceptionNameLength) + return; + + diag(ExceptionVar->getLocation(), "exception variable name %0 is too " + "short, expected at least %1 characters") + << ExceptionVar << MinimumExceptionNameLength; + } + + const auto *LoopVar = Result.Nodes.getNodeAs("loopVar"); + if (LoopVar) { + if (!LoopVar->getIdentifier()) + return; + + const auto LoopVarName = LoopVar->getName(); + + if (LoopVarName.size() >= MinimumLoopCounterNameLength) + return; + + const std::string LoopVarNameString{LoopVarName.data(), LoopVarName.size()}; + if (IgnoredLoopCounterNames.find(LoopVarNameString) != + IgnoredLoopCounterNames.end()) + return; + + diag(LoopVar->getLocation(), + "loop variable name %0 is too short, expected at least %1 characters") + << LoopVar << MinimumLoopCounterNameLength; + } +} + +} // namespace readability +} // namespace tidy +} // namespace clang Index: clang-tools-extra/docs/ReleaseNotes.rst =================================================================== --- clang-tools-extra/docs/ReleaseNotes.rst +++ clang-tools-extra/docs/ReleaseNotes.rst @@ -89,6 +89,11 @@ Finds member initializations in the constructor body which can be placed into the initialization list instead. +- New :doc:`readability-variable-length + ` check. + + Finds variables whose names are too short. + 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 @@ -312,6 +312,7 @@ `readability-uniqueptr-delete-release `_, "Yes" `readability-uppercase-literal-suffix `_, "Yes" `readability-use-anyofallof `_, + `readability-variable-length `_, `zircon-temporary-objects `_, Index: clang-tools-extra/docs/clang-tidy/checks/readability-variable-length.rst =================================================================== --- /dev/null +++ clang-tools-extra/docs/clang-tidy/checks/readability-variable-length.rst @@ -0,0 +1,18 @@ +.. title:: clang-tidy - readability-variable-length + +readability-variable-length +=========================== + +Checks that the variables and function parameters have at least a minimum +number of characters in the name. + +Loop counter variables are expected to have a length of at least +`MinimumLoopCounterNameLength` characters (default is 2). Additionally, `i`, +`j` and `k` are accepted as legacy values (this is the default value for +`IgnoredLoopCounterNames`). + +Exception clause variables are expected to have a length of at least +`MinimumExceptionNameLength` (default is 2). + +All other variables are expected to have at least a length of +`MinimumVariableNameLength` (default is 3). Index: clang-tools-extra/test/clang-tidy/checkers/readability-variable-length.cpp =================================================================== --- /dev/null +++ clang-tools-extra/test/clang-tidy/checkers/readability-variable-length.cpp @@ -0,0 +1,57 @@ +// RUN: %check_clang_tidy %s readability-variable-length %t + +struct myexcept +{ + int val; +}; + +void doIt(); + +void tooShortVariableNames() +{ + int i = 5; +// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: variable name 'i' is too short, expected at least 3 characters [readability-variable-length] + + int jj = 6; +// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: variable name 'jj' is too short, expected at least 3 characters [readability-variable-length] + + for (int m = 0; m < 5; ++ m) +// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: loop variable name 'm' is too short, expected at least 2 characters [readability-variable-length] + { + doIt(); + } + + try + { + doIt(); + } + catch (const myexcept& x) +// CHECK-MESSAGES: :[[@LINE-1]]:27: warning: exception variable name 'x' is too short, expected at least 2 characters [readability-variable-length] + { + doIt(); + } +} + +void longEnoughVariableNames() +{ + 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 myexcept& ex) + { + doIt(); + } +}