[clang-tidy] Add parametercount for readibility-function-size

[JonasToth]

Summary

Add an option to function-size to warn about high parameter counts.

This might be relevant for cppcoreguidelines and the safety module as well. Since the safety module is not landed in master already, i did not create an alias, but that can be done later as well.

[alexfh]

Looks good in general, but please revert unrelated changes (see the comments inline).

test/clang-tidy/readability-function-size.cpp

6

I suppose, weird formatting is a part of the test: it needs to count empty statements, empty lines, etc. (and on the opposite: not count lines when multiple statements are on the same line). Please revert formatting changes. You can add a comment about bad formatting being intentional.

13

The patterns are truncated on purpose: there's not much value in repeating to much static text, so we recommend to leave only what's needed to keep the test readable and unambiguous (ideally, this would also fit into 80 characters).

[JonasToth]

Reverted formatting change to testing code

[alexfh]

LG

[JonasToth]

i dont have commit rights, can you land it for me please?

[hokein]

Could you please also update the document in docs/clang-tidy/checks/readability-function-size.rst?

[JonasToth]

• [Doc] adjust doc for the checker

[JonasToth]

• Merge branch 'master' of http://llvm.org/git/clang-tools-extra into feature_argument_count
• [Release Note] added parameterthreshold to release notes

[JonasToth]

i adjusted the docs and release notes.

[hokein]

LGTM with one nit.

docs/ReleaseNotes.rst

72 ↗

(On Diff #89808)

s/function/functions

[JonasToth]

• [Fix] typo fixed

[JonasToth]

fixed typo, updated diff

clang-tidy/readability/FunctionSizeCheck.h

 ///
 ///   * `LineThreshold` - flag functions exceeding this number of lines. The
 ///     default is `-1` (ignore the number of lines).
 ///   * `StatementThreshold` - flag functions exceeding this number of
 ///     statements. This may differ significantly from the number of lines for
 ///     macro-heavy code. The default is `800`.
 ///   * `BranchThreshold` - flag functions exceeding this number of control
 ///     statements. The default is `-1` (ignore the number of branches).
+///   * `ParameterThreshold` - flag functions having a high number of parameters.
+///     The default is `6`.
 class FunctionSizeCheck : public ClangTidyCheck {
 public:
   FunctionSizeCheck(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 LineThreshold;
   const unsigned StatementThreshold;
   const unsigned BranchThreshold;
+  const unsigned ParameterThreshold;
 };
 
 } // namespace readability
 } // namespace tidy
 } // namespace clang
 
 #endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_FUNCTIONSIZECHECK_H

clang-tidy/readability/FunctionSizeCheck.cpp

public:
   FunctionInfo Info;
   std::vector<bool> TrackedParent;
 };
 
 FunctionSizeCheck::FunctionSizeCheck(StringRef Name, ClangTidyContext *Context)
     : ClangTidyCheck(Name, Context),
       LineThreshold(Options.get("LineThreshold", -1U)),
       StatementThreshold(Options.get("StatementThreshold", 800U)),
-      BranchThreshold(Options.get("BranchThreshold", -1U)) {}
+      BranchThreshold(Options.get("BranchThreshold", -1U)),
+      ParameterThreshold(Options.get("ParameterThreshold", 6)) {}
 
 void FunctionSizeCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
   Options.store(Opts, "LineThreshold", LineThreshold);
   Options.store(Opts, "StatementThreshold", StatementThreshold);
   Options.store(Opts, "BranchThreshold", BranchThreshold);
+  Options.store(Opts, "ParameterThreshold", ParameterThreshold);
 }
 
 void FunctionSizeCheck::registerMatchers(MatchFinder *Finder) {
   Finder->addMatcher(functionDecl(unless(isInstantiated())).bind("func"), this);
 }
 
 void FunctionSizeCheck::check(const MatchFinder::MatchResult &Result) {
   const auto *Func = Result.Nodes.getNodeAs<FunctionDecl>("func");
void FunctionSizeCheck::check(const MatchFinder::MatchResult &Result) {
   if (const Stmt *Body = Func->getBody()) {
     SourceManager *SM = Result.SourceManager;
     if (SM->isWrittenInSameFile(Body->getLocStart(), Body->getLocEnd())) {
       FI.Lines = SM->getSpellingLineNumber(Body->getLocEnd()) -
                  SM->getSpellingLineNumber(Body->getLocStart());
     }
   }
 
+  unsigned ActualNumberParameters = Func->getNumParams();
+
   if (FI.Lines > LineThreshold || FI.Statements > StatementThreshold ||
-      FI.Branches > BranchThreshold) {
+      FI.Branches > BranchThreshold ||
+      ActualNumberParameters > ParameterThreshold) {
     diag(Func->getLocation(),
          "function %0 exceeds recommended size/complexity thresholds")
         << Func;
   }
 
   if (FI.Lines > LineThreshold) {
     diag(Func->getLocation(),
          "%0 lines including whitespace and comments (threshold %1)",
          DiagnosticIDs::Note)
         << FI.Lines << LineThreshold;
   }
 
   if (FI.Statements > StatementThreshold) {
     diag(Func->getLocation(), "%0 statements (threshold %1)",
          DiagnosticIDs::Note)
         << FI.Statements << StatementThreshold;
   }
 
   if (FI.Branches > BranchThreshold) {
     diag(Func->getLocation(), "%0 branches (threshold %1)", DiagnosticIDs::Note)
         << FI.Branches << BranchThreshold;
   }
+
+  if (ActualNumberParameters > ParameterThreshold) {
+    diag(Func->getLocation(), "%0 parameters (threshold %1)",
+         DiagnosticIDs::Note)
+        << ActualNumberParameters << ParameterThreshold;
+  }
 }
 
 } // namespace readability
 } // namespace tidy
 } // namespace clang

test/clang-tidy/readability-function-size.cpp

-// 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}]}' -- -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}]}' -- -std=c++11
 
 void foo1() {
 }
 
-void foo2() {;}
+void foo2() { ; }
 // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'foo2' exceeds recommended size/complexity thresholds [readability-function-size]
 // CHECK-MESSAGES: :[[@LINE-2]]:6: note: 1 statements (threshold 0)
 
 void foo3() {
   ;
-
 }
-// CHECK-MESSAGES: :[[@LINE-4]]:6: warning: function 'foo3' exceeds recommended size/complexity
-// CHECK-MESSAGES: :[[@LINE-5]]:6: note: 3 lines including whitespace and comments (threshold 0)
-// CHECK-MESSAGES: :[[@LINE-6]]:6: note: 1 statements (threshold 0)
+// CHECK-MESSAGES: :[[@LINE-3]]:6: warning: function 'foo3' 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 foo4(int i) { if (i) {} else; {}
+void foo4(int i) {
+  if (i) {
+  } else
+    ;
+  {}
 }
-// CHECK-MESSAGES: :[[@LINE-2]]:6: warning: function 'foo4' exceeds recommended size/complexity
-// CHECK-MESSAGES: :[[@LINE-3]]:6: note: 1 lines including whitespace and comments (threshold 0)
-// CHECK-MESSAGES: :[[@LINE-4]]:6: note: 3 statements (threshold 0)
-// CHECK-MESSAGES: :[[@LINE-5]]:6: note: 1 branches (threshold 0)
-
-void foo5(int i) {for(;i;)while(i)
-do;while(i);
+// CHECK-MESSAGES: :[[@LINE-6]]:6: warning: function 'foo4' 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: 3 statements (threshold 0)
+// CHECK-MESSAGES: :[[@LINE-9]]:6: note: 1 branches (threshold 0)
+
+void foo5(int i) {
+  for (; i;)
+    while (i)
+      do
+        ;
+      while (i);
 }
-// CHECK-MESSAGES: :[[@LINE-3]]:6: warning: function 'foo5' exceeds recommended size/complexity
-// CHECK-MESSAGES: :[[@LINE-4]]:6: note: 2 lines including whitespace and comments (threshold 0)
-// CHECK-MESSAGES: :[[@LINE-5]]:6: note: 7 statements (threshold 0)
-// CHECK-MESSAGES: :[[@LINE-6]]:6: note: 3 branches (threshold 0)
-
-template <typename T> T foo6(T i) {return i;
+// CHECK-MESSAGES: :[[@LINE-7]]:6: warning: function 'foo5' 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: 7 statements (threshold 0)
+// CHECK-MESSAGES: :[[@LINE-10]]:6: note: 3 branches (threshold 0)
+
+template <typename T>
+T foo6(T i) {
+  return i;
 }
 int x = foo6(0);
-// CHECK-MESSAGES: :[[@LINE-3]]:25: warning: function 'foo6' exceeds recommended size/complexity
-// CHECK-MESSAGES: :[[@LINE-4]]:25: note: 1 lines including whitespace and comments (threshold 0)
-// CHECK-MESSAGES: :[[@LINE-5]]:25: note: 1 statements (threshold 0)
-
-void bar1() { [](){;;;;;;;;;;;if(1){}}();
+// CHECK-MESSAGES: :[[@LINE-4]]:3: warning: function 'foo6' exceeds recommended size/complexity thresholds [readability-function-size]
+// CHECK-MESSAGES: :[[@LINE-5]]:3: note: 2 lines including whitespace and comments (threshold 0)
+// CHECK-MESSAGES: :[[@LINE-6]]:3: note: 1 statements (threshold 0)
 
+void foo7(int p1, int p2, int p3, int p4, int p5, int p6, int p7) {
+  ;
+}
+// CHECK-MESSAGES: :[[@LINE-3]]:6: warning: function 'foo7' 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: 7 parameters (threshold 5)
 
+void bar1() {
+  []() {;;;;;;;;;;;if(1){} }();
 }
-// CHECK-MESSAGES: :[[@LINE-4]]:6: warning: function 'bar1' exceeds recommended size/complexity
-// CHECK-MESSAGES: :[[@LINE-5]]:6: note: 3 lines including whitespace and comments (threshold 0)
-// CHECK-MESSAGES: :[[@LINE-6]]:6: note: 14 statements (threshold 0)
-// CHECK-MESSAGES: :[[@LINE-7]]:6: note: 1 branches (threshold 0)
+// CHECK-MESSAGES: :[[@LINE-3]]:6: warning: function 'bar1' 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: 14 statements (threshold 0)
+// CHECK-MESSAGES: :[[@LINE-6]]:6: note: 1 branches (threshold 0)
 
-void bar2() { class A { void barx() {;;} }; }
-// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'bar2' exceeds recommended size/complexity
-// CHECK-MESSAGES: :[[@LINE-2]]:6: note: 3 statements (threshold 0)
+void bar2() {
+  class A {
+    void barx() {
+      ;
+      ;
+    }
+  };
+}
+// CHECK-MESSAGES: :[[@LINE-8]]:6: warning: function 'bar2' 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-4]]:30: warning: function 'barx' exceeds recommended size/complexity
-// CHECK-MESSAGES: :[[@LINE-5]]:30: note: 2 statements (threshold 0)
+// CHECK-MESSAGES: :[[@LINE-10]]:10: warning: function 'barx' exceeds recommended size/complexity thresholds [readability-function-size]
+// CHECK-MESSAGES: :[[@LINE-11]]:10: note: 3 lines including whitespace and comments (threshold 0)
+// CHECK-MESSAGES: :[[@LINE-12]]:10: note: 2 statements (threshold 0)