[clang-tidy] readability-function-size: fix nesting level calculation

Summary:
A followup for D32942.

Malcolm Parsons has provided a valid testcase that the initial version of the check complained about nested `if`'s.
As it turns out, the culprit is the **partially** un-intentional `switch` fallthrough.
So rewrite the NestingThreshold logic without ab-using+mis-using that switch with fallthrough, and add testcases with nested `if`' where there should be a warning and shouldn't be a warning. This results in a cleaner, simpler code, too.

I guess, now it would be actually possible to pick some reasonable default for `NestingThreshold` setting.

Fixes PR33454.

Reviewers: malcolm.parsons, alexfh

Reviewed By: malcolm.parsons

Subscribers: sbenza, xazax.hun, cfe-commits, aaron.ballman

Tags: #clang-tools-extra

Differential Revision: https://reviews.llvm.org/D34202

llvm-svn: 305554

1 files changed, 15 insertions, 10 deletions

diff --git a/clang-tools-extra/clang-tidy/readability/FunctionSizeCheck.cpp b/clang-tools-extra/clang-tidy/readability/FunctionSizeCheck.cpp
index 51b16849239..ea5f7505b69 100644
--- a/clang-tools-extra/clang-tidy/readability/FunctionSizeCheck.cpp
+++ b/clang-tools-extra/clang-tidy/readability/FunctionSizeCheck.cpp
@@ -36,15 +36,8 @@ public:
     case Stmt::ForStmtClass:
     case Stmt::SwitchStmtClass:
       ++Info.Branches;
-    // fallthrough
+      LLVM_FALLTHROUGH;
     case Stmt::CompoundStmtClass:
-      // If this new compound statement is located in a compound statement,
-      // which is already nested NestingThreshold levels deep, record the start
-      // location of this new compound statement
-      if (CurrentNestingLevel == Info.NestingThreshold)
-        Info.NestingThresholders.push_back(Node->getLocStart());
-
-      ++CurrentNestingLevel;
       TrackedParent.push_back(true);
       break;
     default:
@@ -54,13 +47,25 @@ public:
 
     Base::TraverseStmt(Node);
 
-    if (TrackedParent.back())
-      --CurrentNestingLevel;
     TrackedParent.pop_back();
 
     return true;
   }
 
+  bool TraverseCompoundStmt(CompoundStmt *Node) {
+    // If this new compound statement is located in a compound statement, which
+    // is already nested NestingThreshold levels deep, record the start location
+    // of this new compound statement.
+    if (CurrentNestingLevel == Info.NestingThreshold)
+      Info.NestingThresholders.push_back(Node->getLocStart());
+
+    ++CurrentNestingLevel;
+    Base::TraverseCompoundStmt(Node);
+    --CurrentNestingLevel;
+
+    return true;
+  }
+
   bool TraverseDecl(Decl *Node) {
     TrackedParent.push_back(false);
     Base::TraverseDecl(Node);