[clang-tidy] readability-function-size: add VariableThreshold param.
Summary:
Pretty straight-forward, just count all the variable declarations in the function's body, and if more than the configured threshold - do complain.
Note that this continues perverse practice of disabling the new option by default.
I'm not certain where is the balance point between not being too noisy, and actually enforcing the good practice.
If we really want to not disable this by default, but also to not cause too many new warnings, we could default to 50 or so.
But that is a lot of variables too...
I was able to find one coding style referencing variable count:
- https://www.kernel.org/doc/html/v4.15/process/coding-style.html#functions
> Another measure of the function is the number of local variables. They shouldn’t exceed 5-10, or you’re doing something wrong.
Reviewers: hokein, xazax.hun, JonasToth, aaron.ballman, alexfh
Reviewed By: aaron.ballman
Subscribers: kimgr, Eugene.Zelenko, rnkovacs, cfe-commits
Tags: #clang-tools-extra
Differential Revision: https://reviews.llvm.org/D44602
llvm-svn: 329902
diff --git a/clang-tools-extra/clang-tidy/readability/FunctionSizeCheck.cpp b/clang-tools-extra/clang-tidy/readability/FunctionSizeCheck.cpp
index e6410ce..8ed6b90 100644
--- a/clang-tools-extra/clang-tidy/readability/FunctionSizeCheck.cpp
+++ b/clang-tools-extra/clang-tidy/readability/FunctionSizeCheck.cpp
@@ -22,6 +22,21 @@
using Base = RecursiveASTVisitor<FunctionASTVisitor>;
public:
+ bool VisitVarDecl(VarDecl *VD) {
+ // Do not count function params.
+ // Do not count decomposition declarations (C++17's structured bindings).
+ if (StructNesting == 0 &&
+ !(isa<ParmVarDecl>(VD) || isa<DecompositionDecl>(VD)))
+ ++Info.Variables;
+ return true;
+ }
+ bool VisitBindingDecl(BindingDecl *BD) {
+ // Do count each of the bindings (in the decomposition declaration).
+ if (StructNesting == 0)
+ ++Info.Variables;
+ return true;
+ }
+
bool TraverseStmt(Stmt *Node) {
if (!Node)
return Base::TraverseStmt(Node);
@@ -74,15 +89,38 @@
return true;
}
+ bool TraverseLambdaExpr(LambdaExpr *Node) {
+ ++StructNesting;
+ Base::TraverseLambdaExpr(Node);
+ --StructNesting;
+ return true;
+ }
+
+ bool TraverseCXXRecordDecl(CXXRecordDecl *Node) {
+ ++StructNesting;
+ Base::TraverseCXXRecordDecl(Node);
+ --StructNesting;
+ return true;
+ }
+
+ bool TraverseStmtExpr(StmtExpr *SE) {
+ ++StructNesting;
+ Base::TraverseStmtExpr(SE);
+ --StructNesting;
+ return true;
+ }
+
struct FunctionInfo {
unsigned Lines = 0;
unsigned Statements = 0;
unsigned Branches = 0;
unsigned NestingThreshold = 0;
+ unsigned Variables = 0;
std::vector<SourceLocation> NestingThresholders;
};
FunctionInfo Info;
std::vector<bool> TrackedParent;
+ unsigned StructNesting = 0;
unsigned CurrentNestingLevel = 0;
};
@@ -94,7 +132,8 @@
StatementThreshold(Options.get("StatementThreshold", 800U)),
BranchThreshold(Options.get("BranchThreshold", -1U)),
ParameterThreshold(Options.get("ParameterThreshold", -1U)),
- NestingThreshold(Options.get("NestingThreshold", -1U)) {}
+ NestingThreshold(Options.get("NestingThreshold", -1U)),
+ VariableThreshold(Options.get("VariableThreshold", -1U)) {}
void FunctionSizeCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
Options.store(Opts, "LineThreshold", LineThreshold);
@@ -102,6 +141,7 @@
Options.store(Opts, "BranchThreshold", BranchThreshold);
Options.store(Opts, "ParameterThreshold", ParameterThreshold);
Options.store(Opts, "NestingThreshold", NestingThreshold);
+ Options.store(Opts, "VariableThreshold", VariableThreshold);
}
void FunctionSizeCheck::registerMatchers(MatchFinder *Finder) {
@@ -133,7 +173,7 @@
if (FI.Lines > LineThreshold || FI.Statements > StatementThreshold ||
FI.Branches > BranchThreshold ||
ActualNumberParameters > ParameterThreshold ||
- !FI.NestingThresholders.empty()) {
+ !FI.NestingThresholders.empty() || FI.Variables > VariableThreshold) {
diag(Func->getLocation(),
"function %0 exceeds recommended size/complexity thresholds")
<< Func;
@@ -168,6 +208,12 @@
DiagnosticIDs::Note)
<< NestingThreshold + 1 << NestingThreshold;
}
+
+ if (FI.Variables > VariableThreshold) {
+ diag(Func->getLocation(), "%0 variables (threshold %1)",
+ DiagnosticIDs::Note)
+ << FI.Variables << VariableThreshold;
+ }
}
} // namespace readability