[clang-tidy] Add IgnoreMacros to readability-function-size

[PiotrZSL]

Extend readability-function-size check by adding
IgnoreMacros option.

Fixes #112835

[llvmbot]

@llvm/pr-subscribers-clang-tools-extra

@llvm/pr-subscribers-clang-tidy

Author: Piotr Zegar (PiotrZSL)

Changes

Extend readability-function-size check by adding
IgnoreMacros option.

Fixes #112835

---

Full diff: https://github.com/llvm/llvm-project/pull/131406.diff

5 Files Affected:

• (modified) clang-tools-extra/clang-tidy/readability/FunctionSizeCheck.cpp (+13-2)
• (modified) clang-tools-extra/clang-tidy/readability/FunctionSizeCheck.h (+2)
• (modified) clang-tools-extra/docs/ReleaseNotes.rst (+4)
• (modified) clang-tools-extra/docs/clang-tidy/checks/readability/function-size.rst (+7)
• (modified) clang-tools-extra/test/clang-tidy/checkers/readability/function-size.cpp (+14-1)

diff --git a/clang-tools-extra/clang-tidy/readability/FunctionSizeCheck.cpp b/clang-tools-extra/clang-tidy/readability/FunctionSizeCheck.cpp
index 3313bcb39b7f3..9535211413be6 100644
--- a/clang-tools-extra/clang-tidy/readability/FunctionSizeCheck.cpp
+++ b/clang-tools-extra/clang-tidy/readability/FunctionSizeCheck.cpp
@@ -20,6 +20,8 @@ class FunctionASTVisitor : public RecursiveASTVisitor<FunctionASTVisitor> {
   using Base = RecursiveASTVisitor<FunctionASTVisitor>;
 
 public:
+  FunctionASTVisitor(bool IgnoreMacros) : IgnoreMacros(IgnoreMacros) {}
+
   bool VisitVarDecl(VarDecl *VD) {
     // Do not count function params.
     // Do not count decomposition declarations (C++17's structured bindings).
@@ -39,6 +41,9 @@ class FunctionASTVisitor : public RecursiveASTVisitor<FunctionASTVisitor> {
     if (!Node)
       return Base::TraverseStmt(Node);
 
+    if (IgnoreMacros && Node->getBeginLoc().isMacroID())
+      return true;
+
     if (TrackedParent.back() && !isa<CompoundStmt>(Node))
       ++Info.Statements;
 
@@ -120,6 +125,9 @@ class FunctionASTVisitor : public RecursiveASTVisitor<FunctionASTVisitor> {
   llvm::BitVector TrackedParent;
   unsigned StructNesting = 0;
   unsigned CurrentNestingLevel = 0;
+
+private:
+  const bool IgnoreMacros;
 };
 
 } // namespace
@@ -135,7 +143,9 @@ FunctionSizeCheck::FunctionSizeCheck(StringRef Name, ClangTidyContext *Context)
       NestingThreshold(
           Options.get("NestingThreshold", DefaultNestingThreshold)),
       VariableThreshold(
-          Options.get("VariableThreshold", DefaultVariableThreshold)) {}
+          Options.get("VariableThreshold", DefaultVariableThreshold)),
+      IgnoreMacros(
+          Options.getLocalOrGlobal("IgnoreMacros", DefaultIgnoreMacros)) {}
 
 void FunctionSizeCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
   Options.store(Opts, "LineThreshold", LineThreshold);
@@ -144,6 +154,7 @@ void FunctionSizeCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
   Options.store(Opts, "ParameterThreshold", ParameterThreshold);
   Options.store(Opts, "NestingThreshold", NestingThreshold);
   Options.store(Opts, "VariableThreshold", VariableThreshold);
+  Options.store(Opts, "IgnoreMacros", IgnoreMacros);
 }
 
 void FunctionSizeCheck::registerMatchers(MatchFinder *Finder) {
@@ -158,7 +169,7 @@ void FunctionSizeCheck::registerMatchers(MatchFinder *Finder) {
 void FunctionSizeCheck::check(const MatchFinder::MatchResult &Result) {
   const auto *Func = Result.Nodes.getNodeAs<FunctionDecl>("func");
 
-  FunctionASTVisitor Visitor;
+  FunctionASTVisitor Visitor(IgnoreMacros);
   Visitor.Info.NestingThreshold = NestingThreshold.value_or(-1);
   Visitor.TraverseDecl(const_cast<FunctionDecl *>(Func));
   auto &FI = Visitor.Info;
diff --git a/clang-tools-extra/clang-tidy/readability/FunctionSizeCheck.h b/clang-tools-extra/clang-tidy/readability/FunctionSizeCheck.h
index 106c69ff07393..b36de68f9583c 100644
--- a/clang-tools-extra/clang-tidy/readability/FunctionSizeCheck.h
+++ b/clang-tools-extra/clang-tidy/readability/FunctionSizeCheck.h
@@ -47,6 +47,7 @@ class FunctionSizeCheck : public ClangTidyCheck {
   const std::optional<unsigned> ParameterThreshold;
   const std::optional<unsigned> NestingThreshold;
   const std::optional<unsigned> VariableThreshold;
+  const bool IgnoreMacros;
 
   static constexpr std::optional<unsigned> DefaultLineThreshold = std::nullopt;
   static constexpr std::optional<unsigned> DefaultStatementThreshold = 800U;
@@ -58,6 +59,7 @@ class FunctionSizeCheck : public ClangTidyCheck {
       std::nullopt;
   static constexpr std::optional<unsigned> DefaultVariableThreshold =
       std::nullopt;
+  static constexpr bool DefaultIgnoreMacros = false;
 };
 
 } // namespace clang::tidy::readability
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 0ad52f83fad85..6ae117ca57ed4 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -178,6 +178,10 @@ Changes in existing checks
   tolerating fix-it breaking compilation when functions is used as pointers
   to avoid matching usage of functions within the current compilation unit.
 
+- Improved :doc:`readability-function-size
+  <clang-tidy/checks/readability/function-size>` check by adding the option
+  `IgnoreMacros` to allow ignoring code inside macros.
+
 Removed checks
 ^^^^^^^^^^^^^^
 
diff --git a/clang-tools-extra/docs/clang-tidy/checks/readability/function-size.rst b/clang-tools-extra/docs/clang-tidy/checks/readability/function-size.rst
index 133bd3e9c8cbe..4d7c8e9ed47f2 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/readability/function-size.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/readability/function-size.rst
@@ -43,3 +43,10 @@ Options
    The default is `none` (ignore the number of variables).
    Please note that function parameters and variables declared in lambdas,
    GNU Statement Expressions, and nested class inline functions are not counted.
+
+.. option:: IgnoreMacros
+
+   If set to `true`, the check will ignore code inside macros. Note, that also
+   any macro arguments are ignored, even if they should count to the complexity.
+   As this might change in the future, this option isn't guaranteed to be
+   forward-compatible. Default is `false`.
diff --git a/clang-tools-extra/test/clang-tidy/checkers/readability/function-size.cpp b/clang-tools-extra/test/clang-tidy/checkers/readability/function-size.cpp
index 45b2604b43d03..f7fe5509fed99 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/readability/function-size.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/readability/function-size.cpp
@@ -15,7 +15,8 @@
 // RUN:         readability-function-size.BranchThreshold: "5", \
 // RUN:         readability-function-size.ParameterThreshold: "none", \
 // RUN:         readability-function-size.NestingThreshold: "", \
-// RUN:         readability-function-size.VariableThreshold: "" \
+// RUN:         readability-function-size.VariableThreshold: "", \
+// RUN:         readability-function-size.IgnoreMacros: "true" \
 // RUN:     }}'
 
 // Bad formatting is intentional, don't run clang-format over the whole file!
@@ -319,3 +320,15 @@ void variables_16() {
 // CHECK-MESSAGES: :[[@LINE-5]]:6: note: 3 lines including whitespace and comments (threshold 0)
 // CHECK-MESSAGES: :[[@LINE-6]]:6: note: 4 statements (threshold 0)
 // CHECK-MESSAGES: :[[@LINE-7]]:6: note: 2 variables (threshold 1)
+
+#define CONDITION(value) if (value) { return value; }
+
+int macro_test(int v) {
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: function 'macro_test' exceeds recommended size/complexity thresholds [readability-function-size]
+// CHECK-MESSAGES: :[[@LINE-2]]:5: note: 8 lines including whitespace and comments (threshold 0)
+// CHECK-MESSAGES: :[[@LINE-3]]:5: note: 31 statements (threshold 0)
+// CHECK-MESSAGES: :[[@LINE-4]]:5: note: 8 branches (threshold 0)
+
+  CONDITION(v);CONDITION(v);CONDITION(v);CONDITION(v);CONDITION(v);CONDITION(v);CONDITION(v);
+  if (v) { return v; }
+}

[HerrCai0907]

could we check the whole Stmt is macro here. It will cause some unexpected behavior when the stmt is start with macro.
btw. I am not sure whether we should let macro increase statement count.