- Replace CFG construction with a simple branch counter as suggested by Manuel.

http://reviews.llvm.org/D4986

Files:
  clang-tidy/misc/CMakeLists.txt
  clang-tidy/misc/FunctionLength.cpp
  clang-tidy/misc/FunctionLength.h
  clang-tidy/misc/MiscTidyModule.cpp
  unittests/clang-tidy/CMakeLists.txt
  unittests/clang-tidy/FunctionLengthCheckTest.cpp
Index: clang-tidy/misc/CMakeLists.txt
===================================================================
--- clang-tidy/misc/CMakeLists.txt
+++ clang-tidy/misc/CMakeLists.txt
@@ -3,6 +3,7 @@
 add_clang_library(clangTidyMiscModule
   ArgumentCommentCheck.cpp
   BoolPointerImplicitConversion.cpp
+  FunctionLength.cpp
   MiscTidyModule.cpp
   RedundantSmartptrGet.cpp
   SwappedArgumentsCheck.cpp
Index: clang-tidy/misc/FunctionLength.cpp
===================================================================
--- /dev/null
+++ clang-tidy/misc/FunctionLength.cpp
@@ -0,0 +1,87 @@
+//===--- FunctionLength.cpp - clang-tidy ----------------------------------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include "FunctionLength.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+
+void FunctionLengthCheck::registerMatchers(MatchFinder *Finder) {
+  auto InTemplateInstantiation = hasAncestor(
+      decl(anyOf(recordDecl(ast_matchers::isTemplateInstantiation()),
+                 functionDecl(ast_matchers::isTemplateInstantiation()))));
+  Finder->addMatcher(
+      functionDecl(
+          unless(InTemplateInstantiation),
+          forEachDescendant(
+              stmt(unless(compoundStmt()),
+                   hasParent(stmt(anyOf(compoundStmt(), ifStmt(),
+                                        anyOf(whileStmt(), doStmt(),
+                                              forRangeStmt(), forStmt())))))
+                  .bind("stmt"))).bind("func"),
+      this);
+}
+
+void FunctionLengthCheck::check(const MatchFinder::MatchResult &Result) {
+  const auto *Func = Result.Nodes.getNodeAs<FunctionDecl>("func");
+
+  FunctionInfo &FI = FunctionInfos[Func];
+
+  // Count the lines including whitespace and comments. Really simple.
+  if (!FI.Lines) {
+    if (const Stmt *Body = Func->getBody()) {
+      SourceManager *SM = Result.SourceManager;
+      if (SM->isWrittenInSameFile(Body->getLocStart(), Body->getLocEnd())) {
+        FI.Lines = SM->getPresumedLineNumber(Body->getLocEnd()) -
+                   SM->getPresumedLineNumber(Body->getLocStart());
+      }
+    }
+  }
+
+  const auto *Statement = Result.Nodes.getNodeAs<Stmt>("stmt");
+  ++FI.Statements;
+
+  // TODO: switch cases, gotos
+  if (isa<IfStmt>(Statement) || isa<WhileStmt>(Statement) ||
+      isa<ForStmt>(Statement) || isa<SwitchStmt>(Statement) ||
+      isa<DoStmt>(Statement) || isa<CXXForRangeStmt>(Statement))
+    ++FI.Branches;
+}
+
+void FunctionLengthCheck::onEndOfTranslationUnit() {
+  // If we're above the limit emit a warning.
+  for (const auto &P : FunctionInfos) {
+    const FunctionInfo &FI = P.second;
+    if (FI.Lines > LineThreshold) {
+      diag(P.first->getLocation(), "function is very large, %0 lines including "
+                                   "whitespace and comments (threshold %1)")
+          << FI.Lines << LineThreshold;
+    }
+
+    if (FI.Statements > StatementThreshold) {
+      diag(P.first->getLocation(),
+           "function is very large, %0 statements (threshold %1)")
+          << FI.Statements << StatementThreshold;
+    }
+
+    if (FI.Branches > BranchThreshold) {
+      diag(P.first->getLocation(),
+           "function is very large, %0 branches (threshold %1)")
+          << FI.Branches << BranchThreshold;
+    }
+  }
+
+  FunctionInfos.clear();
+}
+
+} // namespace tidy
+} // namespace clang
Index: clang-tidy/misc/FunctionLength.h
===================================================================
--- /dev/null
+++ clang-tidy/misc/FunctionLength.h
@@ -0,0 +1,57 @@
+//===--- FunctionLength.h - clang-tidy --------------------------*- C++ -*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_FUNCTIONLENGTH_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_FUNCTIONLENGTH_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+
+/// \brief Checks for large functions based on various metrics.
+class FunctionLengthCheck : public ClangTidyCheck {
+  struct FunctionInfo {
+    FunctionInfo() : Lines(0), Statements(0), Branches(0) {}
+    unsigned Lines;
+    unsigned Statements;
+    unsigned Branches;
+  };
+
+  const unsigned LineThreshold;
+  const unsigned StatementThreshold;
+  const unsigned BranchThreshold;
+
+  llvm::DenseMap<const FunctionDecl *, FunctionInfo> FunctionInfos;
+
+public:
+  /// \brief Creates a new function length check.
+  ///
+  /// \param LineThreshold      The maxmimum number of lines a function can
+  ///                           contain before a warning is emitted.
+  /// \param StatementThreshold The maximum number of statements a function can
+  ///                           contain before a warning is emitted.
+  /// \param BranchThreshold    The maximum number of branches a function can
+  ///                           contain before a warning is emitted.
+  FunctionLengthCheck(unsigned LineThreshold = -1U,
+                      unsigned StatementThreshold = 800,
+                      unsigned BranchThreshold = -1U)
+      : LineThreshold(LineThreshold), StatementThreshold(StatementThreshold),
+        BranchThreshold(BranchThreshold) {}
+
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+  void onEndOfTranslationUnit() override;
+};
+
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_FUNCTIONLENGTH_H
+
Index: clang-tidy/misc/MiscTidyModule.cpp
===================================================================
--- clang-tidy/misc/MiscTidyModule.cpp
+++ clang-tidy/misc/MiscTidyModule.cpp
@@ -12,6 +12,7 @@
 #include "../ClangTidyModuleRegistry.h"
 #include "ArgumentCommentCheck.h"
 #include "BoolPointerImplicitConversion.h"
+#include "FunctionLength.h"
 #include "RedundantSmartptrGet.h"
 #include "SwappedArgumentsCheck.h"
 #include "UndelegatedConstructor.h"
@@ -31,6 +32,9 @@
         "misc-bool-pointer-implicit-conversion",
         new ClangTidyCheckFactory<BoolPointerImplicitConversion>());
     CheckFactories.addCheckFactory(
+        "misc-function-length",
+        new ClangTidyCheckFactory<FunctionLengthCheck>());
+    CheckFactories.addCheckFactory(
         "misc-redundant-smartptr-get",
         new ClangTidyCheckFactory<RedundantSmartptrGet>());
     CheckFactories.addCheckFactory(
Index: unittests/clang-tidy/CMakeLists.txt
===================================================================
--- unittests/clang-tidy/CMakeLists.txt
+++ unittests/clang-tidy/CMakeLists.txt
@@ -9,8 +9,9 @@
 add_extra_unittest(ClangTidyTests
   ClangTidyDiagnosticConsumerTest.cpp
   ClangTidyOptionsTest.cpp
-  LLVMModuleTest.cpp
+  FunctionLengthCheckTest.cpp
   GoogleModuleTest.cpp
+  LLVMModuleTest.cpp
   MiscModuleTest.cpp)
 
 target_link_libraries(ClangTidyTests
Index: unittests/clang-tidy/FunctionLengthCheckTest.cpp
===================================================================
--- /dev/null
+++ unittests/clang-tidy/FunctionLengthCheckTest.cpp
@@ -0,0 +1,98 @@
+#include "ClangTidyTest.h"
+#include "misc/FunctionLength.h"
+#include "gtest/gtest.h"
+
+namespace clang {
+namespace tidy {
+namespace test {
+
+namespace {
+std::string getWarning(ClangTidyCheck &Check, StringRef Code) {
+  ClangTidyContext Context(
+      new DefaultOptionsProvider(ClangTidyGlobalOptions(), ClangTidyOptions()));
+  ClangTidyDiagnosticConsumer DiagConsumer(Context);
+  Check.setContext(&Context);
+  std::vector<std::string> Args;
+
+  ast_matchers::MatchFinder Finder;
+  Check.registerMatchers(&Finder);
+  std::unique_ptr<tooling::FrontendActionFactory> Factory(
+      tooling::newFrontendActionFactory(&Finder));
+  if (!tooling::runToolOnCodeWithArgs(Factory->create(), Code, Args))
+    return "tooling error";
+  DiagConsumer.finish();
+
+  if (Context.getErrors().size() == 0)
+    return "no errors";
+
+  if (Context.getErrors().size() != 1)
+    return "wrong number of errors";
+
+  return StringRef(Context.getErrors().back().Message.Message).rtrim(" []");
+}
+} // namespace
+
+static const char *Code1 = "void foo() {\n}";
+static const char *Code2 = "void foo() {;}";
+static const char *Code3 = "void foo() {\n;\n\n}";
+static const char *Code4 = "void foo(int i) { if (i) {} else; {}\n}";
+static const char *Code5 = "void foo(int i) {for(;i;)while(i)\ndo;while(i);\n}";
+static const char *Code6 = "template <typename T> T foo(T i) {return i;\n}\n"
+                           "int x = foo(0);";
+
+TEST(FunctionLengthCheckTest, LinesTest) {
+  FunctionLengthCheck C(0U, -1U, -1U);
+
+  EXPECT_EQ("no errors", getWarning(C, Code1));
+  EXPECT_EQ("no errors", getWarning(C, Code2));
+  EXPECT_EQ("function is very large, 3 lines including whitespace and comments "
+            "(threshold 0)",
+            getWarning(C, Code3));
+  EXPECT_EQ("function is very large, 1 lines including whitespace and comments "
+            "(threshold 0)",
+            getWarning(C, Code4));
+  EXPECT_EQ("function is very large, 2 lines including whitespace and comments "
+            "(threshold 0)",
+            getWarning(C, Code5));
+  EXPECT_EQ("function is very large, 1 lines including whitespace and comments "
+            "(threshold 0)",
+            getWarning(C, Code6));
+}
+
+TEST(FunctionLengthCheckTest, StatementsTest) {
+  FunctionLengthCheck C(-1U, 0U, -1U);
+
+  EXPECT_EQ("no errors",
+            getWarning(C, Code1));
+  EXPECT_EQ("function is very large, 1 statements (threshold 0)",
+            getWarning(C, Code2));
+  EXPECT_EQ("function is very large, 1 statements (threshold 0)",
+            getWarning(C, Code3));
+  EXPECT_EQ("function is very large, 3 statements (threshold 0)",
+            getWarning(C, Code4));
+  EXPECT_EQ("function is very large, 7 statements (threshold 0)",
+            getWarning(C, Code5));
+  EXPECT_EQ("function is very large, 1 statements (threshold 0)",
+            getWarning(C, Code6));
+}
+
+TEST(FunctionLengthCheckTest, BranchesTest) {
+  FunctionLengthCheck C(-1U, -1U, 0U);
+
+  EXPECT_EQ("no errors",
+            getWarning(C, Code1));
+  EXPECT_EQ("no errors",
+            getWarning(C, Code2));
+  EXPECT_EQ("no errors",
+            getWarning(C, Code3));
+  EXPECT_EQ("function is very large, 1 branches (threshold 0)",
+            getWarning(C, Code4));
+  EXPECT_EQ("function is very large, 3 branches (threshold 0)",
+            getWarning(C, Code5));
+  EXPECT_EQ("no errors",
+            getWarning(C, Code6));
+}
+
+} // namespace test
+} // namespace tidy
+} // namespace clang
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

Reply via email to