Index: clang-tidy/readability/FunctionSizeCheck.h =================================================================== --- clang-tidy/readability/FunctionSizeCheck.h +++ clang-tidy/readability/FunctionSizeCheck.h @@ -33,6 +33,8 @@ /// level after `NestingThreshold`. This may differ significantly from the /// expected value for macro-heavy code. The default is `-1` (ignore the /// nesting level). +/// * `VariableThreshold` - flag functions having a high number of variable +/// declarations. The default is `-1` (ignore the number of variables). class FunctionSizeCheck : public ClangTidyCheck { public: FunctionSizeCheck(StringRef Name, ClangTidyContext *Context); @@ -47,6 +49,7 @@ const unsigned BranchThreshold; const unsigned ParameterThreshold; const unsigned NestingThreshold; + const unsigned VariableThreshold; }; } // namespace readability Index: clang-tidy/readability/FunctionSizeCheck.cpp =================================================================== --- clang-tidy/readability/FunctionSizeCheck.cpp +++ clang-tidy/readability/FunctionSizeCheck.cpp @@ -22,6 +22,23 @@ using Base = RecursiveASTVisitor; public: + bool VisitVarDecl(VarDecl *VD) { + // Do not count function params. + // Do not count decomposition declarations (C++17's structured bindings). + // Do not count variables declared in macros. + if (StructNesting == 0 && + !(isa(VD) || isa(VD)) && + !VD->getLocation().isMacroID()) + Info.Variables++; + return true; + } + bool VisitBindingDecl(BindingDecl *BD) { + // Do count each of the bindings (in the decomposition declaration). + if (StructNesting == 0 && !BD->getLocation().isMacroID()) + Info.Variables++; + return true; + } + bool TraverseStmt(Stmt *Node) { if (!Node) return Base::TraverseStmt(Node); @@ -74,15 +91,31 @@ return true; } + bool TraverseLambdaExpr(LambdaExpr *Node) { + StructNesting++; + Base::TraverseLambdaExpr(Node); + StructNesting--; + return true; + } + + bool TraverseCXXRecordDecl(CXXRecordDecl *Node) { + StructNesting++; + Base::TraverseCXXRecordDecl(Node); + StructNesting--; + return true; + } + struct FunctionInfo { unsigned Lines = 0; unsigned Statements = 0; unsigned Branches = 0; unsigned NestingThreshold = 0; + unsigned Variables = 0; std::vector NestingThresholders; }; FunctionInfo Info; std::vector TrackedParent; + unsigned StructNesting = 0; unsigned CurrentNestingLevel = 0; }; @@ -94,7 +127,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 +136,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 +168,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 +203,12 @@ DiagnosticIDs::Note) << NestingThreshold + 1 << NestingThreshold; } + + if (FI.Variables > VariableThreshold) { + diag(Func->getLocation(), "%0 variables (threshold %1)", + DiagnosticIDs::Note) + << FI.Variables << VariableThreshold; + } } } // namespace readability Index: docs/ReleaseNotes.rst =================================================================== --- docs/ReleaseNotes.rst +++ docs/ReleaseNotes.rst @@ -134,6 +134,11 @@ ` added. +- Added `VariableThreshold` option to :doc:`readability-function-size + ` check + + Flags functions that have more than a specified number of variables declared in the body. + - The 'misc-forwarding-reference-overload' check was renamed to :doc:`bugprone-forwarding-reference-overload ` Index: docs/clang-tidy/checks/readability-function-size.rst =================================================================== --- docs/clang-tidy/checks/readability-function-size.rst +++ docs/clang-tidy/checks/readability-function-size.rst @@ -36,3 +36,11 @@ Flag compound statements which create next nesting level after `NestingThreshold`. This may differ significantly from the expected value for macro-heavy code. The default is `-1` (ignore the nesting level). + +.. option:: VariableThreshold + + Flag functions exceeding this number of variables declared in the body. + The default is `-1` (ignore the number of variables). + Please do note that function parameters are not counted here. + Also, the variables declared in macros expansion, and in nested lambdas, + nested classes are not counted. Index: test/clang-tidy/readability-function-size-variables-c++17.cpp =================================================================== --- /dev/null +++ test/clang-tidy/readability-function-size-variables-c++17.cpp @@ -0,0 +1,21 @@ +// RUN: %check_clang_tidy %s readability-function-size %t -- -config='{CheckOptions: [{key: readability-function-size.LineThreshold, value: 0}, {key: readability-function-size.StatementThreshold, value: 0}, {key: readability-function-size.BranchThreshold, value: 0}, {key: readability-function-size.ParameterThreshold, value: 5}, {key: readability-function-size.NestingThreshold, value: 2}, {key: readability-function-size.VariableThreshold, value: 1}]}' -- -std=c++17 + +void structured_bindings() { + int a[2] = {1, 2}; + auto [x, y] = a; +} +// CHECK-MESSAGES: :[[@LINE-4]]:6: warning: function 'structured_bindings' exceeds recommended size/complexity thresholds [readability-function-size] +// CHECK-MESSAGES: :[[@LINE-5]]:6: note: 3 lines including whitespace and comments (threshold 0) +// CHECK-MESSAGES: :[[@LINE-6]]:6: note: 2 statements (threshold 0) +// CHECK-MESSAGES: :[[@LINE-7]]:6: note: 3 variables (threshold 1) + +#define SWAP(x, y) ({auto& [x0, x1] = x; __typeof__(x) t = {x0, x1}; auto& [y0, y1] = y; auto& [t0, t1] = t; x0 = y0; x1 = y1; y0 = t0; y1 = t1; }) +void variables_13() { + int a[2] = {1, 2}; + int b[2] = {3, 4}; + SWAP(a, b); +} +// CHECK-MESSAGES: :[[@LINE-5]]:6: warning: function 'variables_13' exceeds recommended size/complexity thresholds [readability-function-size] +// CHECK-MESSAGES: :[[@LINE-6]]:6: note: 4 lines including whitespace and comments (threshold 0) +// CHECK-MESSAGES: :[[@LINE-7]]:6: note: 11 statements (threshold 0) +// CHECK-MESSAGES: :[[@LINE-8]]:6: note: 2 variables (threshold 1) Index: test/clang-tidy/readability-function-size.cpp =================================================================== --- test/clang-tidy/readability-function-size.cpp +++ test/clang-tidy/readability-function-size.cpp @@ -1,4 +1,4 @@ -// RUN: %check_clang_tidy %s readability-function-size %t -- -config='{CheckOptions: [{key: readability-function-size.LineThreshold, value: 0}, {key: readability-function-size.StatementThreshold, value: 0}, {key: readability-function-size.BranchThreshold, value: 0}, {key: readability-function-size.ParameterThreshold, value: 5}, {key: readability-function-size.NestingThreshold, value: 2}]}' -- -std=c++11 +// RUN: %check_clang_tidy %s readability-function-size %t -- -config='{CheckOptions: [{key: readability-function-size.LineThreshold, value: 0}, {key: readability-function-size.StatementThreshold, value: 0}, {key: readability-function-size.BranchThreshold, value: 0}, {key: readability-function-size.ParameterThreshold, value: 5}, {key: readability-function-size.NestingThreshold, value: 2}, {key: readability-function-size.VariableThreshold, value: 1}]}' -- -std=c++11 // Bad formatting is intentional, don't run clang-format over the whole file! @@ -64,7 +64,7 @@ void baz0() { // 1 // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'baz0' exceeds recommended size/complexity - // CHECK-MESSAGES: :[[@LINE-2]]:6: note: 27 lines including whitespace and comments (threshold 0) + // CHECK-MESSAGES: :[[@LINE-2]]:6: note: 28 lines including whitespace and comments (threshold 0) // CHECK-MESSAGES: :[[@LINE-3]]:6: note: 9 statements (threshold 0) int a; { // 2 @@ -87,14 +87,15 @@ } } macro() -// CHECK-MESSAGES: :[[@LINE-1]]:3: note: nesting level 3 starts here (threshold 2) -// CHECK-MESSAGES: :[[@LINE-28]]:25: note: expanded from macro 'macro' + // CHECK-MESSAGES: :[[@LINE-1]]:3: note: nesting level 3 starts here (threshold 2) + // CHECK-MESSAGES: :[[@LINE-28]]:25: note: expanded from macro 'macro' + // CHECK-MESSAGES: :[[@LINE-27]]:6: note: 6 variables (threshold 1) } // check that nested if's are not reported. this was broken initially void nesting_if() { // 1 // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'nesting_if' exceeds recommended size/complexity - // CHECK-MESSAGES: :[[@LINE-2]]:6: note: 22 lines including whitespace and comments (threshold 0) + // CHECK-MESSAGES: :[[@LINE-2]]:6: note: 23 lines including whitespace and comments (threshold 0) // CHECK-MESSAGES: :[[@LINE-3]]:6: note: 18 statements (threshold 0) // CHECK-MESSAGES: :[[@LINE-4]]:6: note: 6 branches (threshold 0) if (true) { // 2 @@ -114,12 +115,13 @@ } else if (true) { // 2 int j; } + // CHECK-MESSAGES: :[[@LINE-22]]:6: note: 6 variables (threshold 1) } // however this should warn void bad_if_nesting() { // 1 // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'bad_if_nesting' exceeds recommended size/complexity -// CHECK-MESSAGES: :[[@LINE-2]]:6: note: 22 lines including whitespace and comments (threshold 0) +// CHECK-MESSAGES: :[[@LINE-2]]:6: note: 23 lines including whitespace and comments (threshold 0) // CHECK-MESSAGES: :[[@LINE-3]]:6: note: 12 statements (threshold 0) // CHECK-MESSAGES: :[[@LINE-4]]:6: note: 4 branches (threshold 0) if (true) { // 2 @@ -139,4 +141,153 @@ } } } + // CHECK-MESSAGES: :[[@LINE-22]]:6: note: 4 variables (threshold 1) } + +void variables_0() { + int i; +} +// CHECK-MESSAGES: :[[@LINE-3]]:6: warning: function 'variables_0' exceeds recommended size/complexity thresholds [readability-function-size] +// CHECK-MESSAGES: :[[@LINE-4]]:6: note: 2 lines including whitespace and comments (threshold 0) +// CHECK-MESSAGES: :[[@LINE-5]]:6: note: 1 statements (threshold 0) +void variables_1(int i) { + int j; +} +// CHECK-MESSAGES: :[[@LINE-3]]:6: warning: function 'variables_1' exceeds recommended size/complexity thresholds [readability-function-size] +// CHECK-MESSAGES: :[[@LINE-4]]:6: note: 2 lines including whitespace and comments (threshold 0) +// CHECK-MESSAGES: :[[@LINE-5]]:6: note: 1 statements (threshold 0) +void variables_2(int i, int j) { + ; +} +// CHECK-MESSAGES: :[[@LINE-3]]:6: warning: function 'variables_2' exceeds recommended size/complexity thresholds [readability-function-size] +// CHECK-MESSAGES: :[[@LINE-4]]:6: note: 2 lines including whitespace and comments (threshold 0) +// CHECK-MESSAGES: :[[@LINE-5]]:6: note: 1 statements (threshold 0) +void variables_3() { + int i[2]; +} +// CHECK-MESSAGES: :[[@LINE-3]]:6: warning: function 'variables_3' exceeds recommended size/complexity thresholds [readability-function-size] +// CHECK-MESSAGES: :[[@LINE-4]]:6: note: 2 lines including whitespace and comments (threshold 0) +// CHECK-MESSAGES: :[[@LINE-5]]:6: note: 1 statements (threshold 0) +void variables_4() { + int i; + int j; +} +// CHECK-MESSAGES: :[[@LINE-4]]:6: warning: function 'variables_4' exceeds recommended size/complexity thresholds [readability-function-size] +// CHECK-MESSAGES: :[[@LINE-5]]:6: note: 3 lines including whitespace and comments (threshold 0) +// CHECK-MESSAGES: :[[@LINE-6]]:6: note: 2 statements (threshold 0) +// CHECK-MESSAGES: :[[@LINE-7]]:6: note: 2 variables (threshold 1) +void variables_5() { + int i, j; +} +// CHECK-MESSAGES: :[[@LINE-3]]:6: warning: function 'variables_5' exceeds recommended size/complexity thresholds [readability-function-size] +// CHECK-MESSAGES: :[[@LINE-4]]:6: note: 2 lines including whitespace and comments (threshold 0) +// CHECK-MESSAGES: :[[@LINE-5]]:6: note: 1 statements (threshold 0) +// CHECK-MESSAGES: :[[@LINE-6]]:6: note: 2 variables (threshold 1) +void variables_6() { + for (int i;;) + for (int j;;) + ; +} +// CHECK-MESSAGES: :[[@LINE-5]]:6: warning: function 'variables_6' exceeds recommended size/complexity thresholds [readability-function-size] +// CHECK-MESSAGES: :[[@LINE-6]]:6: note: 4 lines including whitespace and comments (threshold 0) +// CHECK-MESSAGES: :[[@LINE-7]]:6: note: 5 statements (threshold 0) +// CHECK-MESSAGES: :[[@LINE-8]]:6: note: 2 branches (threshold 0) +// CHECK-MESSAGES: :[[@LINE-9]]:6: note: 2 variables (threshold 1) +void variables_7() { + if (int a = 1) + if (int b = 2) + ; +} +// CHECK-MESSAGES: :[[@LINE-5]]:6: warning: function 'variables_7' exceeds recommended size/complexity thresholds [readability-function-size] +// CHECK-MESSAGES: :[[@LINE-6]]:6: note: 4 lines including whitespace and comments (threshold 0) +// CHECK-MESSAGES: :[[@LINE-7]]:6: note: 7 statements (threshold 0) +// CHECK-MESSAGES: :[[@LINE-8]]:6: note: 2 branches (threshold 0) +// CHECK-MESSAGES: :[[@LINE-9]]:6: note: 2 variables (threshold 1) +void variables_8() { + int a[2]; + for (auto i : a) + for (auto j : a) + ; +} +// CHECK-MESSAGES: :[[@LINE-6]]:6: warning: function 'variables_8' exceeds recommended size/complexity thresholds [readability-function-size] +// CHECK-MESSAGES: :[[@LINE-7]]:6: note: 5 lines including whitespace and comments (threshold 0) +// CHECK-MESSAGES: :[[@LINE-8]]:6: note: 8 statements (threshold 0) +// CHECK-MESSAGES: :[[@LINE-9]]:6: note: 2 branches (threshold 0) +// CHECK-MESSAGES: :[[@LINE-10]]:6: note: 3 variables (threshold 1) +void variables_9() { + int a, b; + struct A { + A(int c, int d) { + int e, f; + } + }; +} +// CHECK-MESSAGES: :[[@LINE-8]]:6: warning: function 'variables_9' exceeds recommended size/complexity thresholds [readability-function-size] +// CHECK-MESSAGES: :[[@LINE-9]]:6: note: 7 lines including whitespace and comments (threshold 0) +// CHECK-MESSAGES: :[[@LINE-10]]:6: note: 3 statements (threshold 0) +// CHECK-MESSAGES: :[[@LINE-11]]:6: note: 2 variables (threshold 1) +// CHECK-MESSAGES: :[[@LINE-9]]:5: warning: function 'A' exceeds recommended size/complexity thresholds [readability-function-size] +// CHECK-MESSAGES: :[[@LINE-10]]:5: note: 2 lines including whitespace and comments (threshold 0) +// CHECK-MESSAGES: :[[@LINE-11]]:5: note: 1 statements (threshold 0) +// CHECK-MESSAGES: :[[@LINE-12]]:5: note: 2 variables (threshold 1) +void variables_10() { + int a, b; + struct A { + int c; + int d; + }; +} +// CHECK-MESSAGES: :[[@LINE-7]]:6: warning: function 'variables_10' exceeds recommended size/complexity thresholds [readability-function-size] +// CHECK-MESSAGES: :[[@LINE-8]]:6: note: 6 lines including whitespace and comments (threshold 0) +// CHECK-MESSAGES: :[[@LINE-9]]:6: note: 2 statements (threshold 0) +// CHECK-MESSAGES: :[[@LINE-10]]:6: note: 2 variables (threshold 1) +void variables_11() { + struct S { + void bar() { + int a, b; + } + }; +} +// CHECK-MESSAGES: :[[@LINE-7]]:6: warning: function 'variables_11' exceeds recommended size/complexity thresholds [readability-function-size] +// CHECK-MESSAGES: :[[@LINE-8]]:6: note: 6 lines including whitespace and comments (threshold 0) +// CHECK-MESSAGES: :[[@LINE-7]]:10: warning: function 'bar' exceeds recommended size/complexity thresholds [readability-function-size] +// CHECK-MESSAGES: :[[@LINE-8]]:10: note: 2 lines including whitespace and comments (threshold 0) +// CHECK-MESSAGES: :[[@LINE-9]]:10: note: 2 variables (threshold 1) +void variables_12() { + int v; + auto test = [](int a, int b) -> void {}; + test({}, {}); +} +// CHECK-MESSAGES: :[[@LINE-5]]:6: warning: function 'variables_12' exceeds recommended size/complexity thresholds [readability-function-size] +// CHECK-MESSAGES: :[[@LINE-6]]:6: note: 4 lines including whitespace and comments (threshold 0) +// CHECK-MESSAGES: :[[@LINE-7]]:6: note: 3 statements (threshold 0) +// CHECK-MESSAGES: :[[@LINE-8]]:6: note: 2 variables (threshold 1) +void variables_13() { + int v; + auto test = []() -> void { + int a; + int b; + }; + test(); +} +// CHECK-MESSAGES: :[[@LINE-8]]:6: warning: function 'variables_13' exceeds recommended size/complexity thresholds [readability-function-size] +// CHECK-MESSAGES: :[[@LINE-9]]:6: note: 7 lines including whitespace and comments (threshold 0) +// CHECK-MESSAGES: :[[@LINE-10]]:6: note: 5 statements (threshold 0) +// CHECK-MESSAGES: :[[@LINE-11]]:6: note: 2 variables (threshold 1) +#define SWAP(x, y) ({__typeof__(x) temp = x; x = y; y = temp; }) +void variables_14() { + int a = 10, b = 12; + SWAP(a, b); +} +// CHECK-MESSAGES: :[[@LINE-4]]:6: warning: function 'variables_14' exceeds recommended size/complexity thresholds [readability-function-size] +// CHECK-MESSAGES: :[[@LINE-5]]:6: note: 3 lines including whitespace and comments (threshold 0) +// CHECK-MESSAGES: :[[@LINE-6]]:6: note: 5 statements (threshold 0) +// CHECK-MESSAGES: :[[@LINE-7]]:6: note: 2 variables (threshold 1) +#define vardecl(type, name) type name; +void variables_15() { + // FIXME: surely we should still warn here? + vardecl(int, a); + vardecl(int, b); +} +// CHECK-MESSAGES: :[[@LINE-5]]:6: warning: function 'variables_15' exceeds recommended size/complexity thresholds [readability-function-size] +// CHECK-MESSAGES: :[[@LINE-6]]:6: note: 4 lines including whitespace and comments (threshold 0)