[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 ↗

(On Diff #87137)

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 ↗

(On Diff #87137)

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-tools-extra/trunk/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-tools-extra/trunk/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

clang-tools-extra/trunk/docs/ReleaseNotes.rst

- New `cert-dcl58-cpp
 
   Finds modification of the ``std`` or ``posix`` namespace.
 
 - New `readability-misleading-indentation
   <http://clang.llvm.org/extra/clang-tidy/checks/readability-misleading-indentation.html>`_ check
 
   Finds misleading indentation where braces should be introduced or the code should be reformatted.
 
+- Added `ParameterThreshold` to `readability-function-size`.
+
+  Finds functions that have more then `ParameterThreshold` parameters and emits a warning.
+
 - New `safety-no-assembler
   <http://clang.llvm.org/extra/clang-tidy/checks/safety-no-assembler.html>`_ check
 
   Finds uses of inline assembler.
 
 - New `modernize-return-braced-init-list
   <http://clang.llvm.org/extra/clang-tidy/checks/modernize-return-braced-init-list.html>`_ check
 

clang-tools-extra/trunk/docs/clang-tidy/checks/readability-function-size.rst

.. option:: 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`.
 
 .. option:: BranchThreshold
 
    Flag functions exceeding this number of control statements. The default is
    `-1` (ignore the number of branches).
+
+.. option:: ParameterThreshold
+
+   Flag functions that exceed a specified number of parameters. The default 
+   is 6.

clang-tools-extra/trunk/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
+
+// Bad formatting is intentional, don't run clang-format over the whole file!
 
 void foo1() {
 }
 
 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)
 

 
 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 foo7(int p1, int p2, int p3, int p4, int p5, int p6) {;}
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'foo7' exceeds recommended size/complexity
+// CHECK-MESSAGES: :[[@LINE-2]]:6: note: 1 statements (threshold 0)
+// CHECK-MESSAGES: :[[@LINE-3]]:6: note: 6 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)
 
 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)
 //
 // CHECK-MESSAGES: :[[@LINE-4]]:30: warning: function 'barx' exceeds recommended size/complexity
 // CHECK-MESSAGES: :[[@LINE-5]]:30: note: 2 statements (threshold 0)