[PATCH] [clang-tidy] Add a checker for long functions.

[Alexander Kornienko]

Counting control statements is much more straightforward, thanks. A few comments inline.

================
Comment at: clang-tidy/misc/FunctionLength.cpp:19
@@ +18,3 @@
+void FunctionLengthCheck::registerMatchers(MatchFinder *Finder) {
+  auto InTemplateInstantiation = hasAncestor(
+      decl(anyOf(recordDecl(ast_matchers::isTemplateInstantiation()),
----------------
Move it to ASTMatchers.h? It's used in enough places already. IIUC, Manuel considers this a good enough way to filter out matches in template instantiations.

================
Comment at: clang-tidy/misc/FunctionLength.cpp:44
@@ +43,3 @@
+      if (SM->isWrittenInSameFile(Body->getLocStart(), Body->getLocEnd())) {
+        FI.Lines = SM->getPresumedLineNumber(Body->getLocEnd()) -
+                   SM->getPresumedLineNumber(Body->getLocStart());
----------------
I wonder if you see specific reasons to use presumed locations here?

================
Comment at: clang-tidy/misc/FunctionLength.cpp:65
@@ +64,3 @@
+    if (FI.Lines > LineThreshold) {
+      diag(P.first->getLocation(), "function is very large, %0 lines including "
+                                   "whitespace and comments (threshold %1)")
----------------
If someone decides to configure several thresholds, they will get duplicate "function is very large" warnings. I'd make a single "function is very large" (or maybe "function exceeds recommended size/complexity thresholds") warning, and add specific metrics as notes. Also please insert the function name to the message to make it obvious what it complains about.

================
Comment at: unittests/clang-tidy/FunctionLengthCheckTest.cpp:1
@@ +1,2 @@
+#include "ClangTidyTest.h"
+#include "misc/FunctionLength.h"
----------------
It would be nice to test: multiple functions, lambdas, local classes/functions.


================
Comment at: unittests/clang-tidy/FunctionLengthCheckTest.cpp:10
@@ +9,3 @@
+namespace {
+std::string getWarning(ClangTidyCheck &Check, StringRef Code) {
+  ClangTidyContext Context(
----------------
I'd make this function a bit more generic (e.g. return llvm::ErrorOr<std::vector<std::string>>) and move it to ClangTidyTest.h. Or maybe even extend runCheckOnCode.

http://reviews.llvm.org/D4986