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.h =================================================================== --- /dev/null +++ clang-tidy/misc/FunctionLength.h @@ -0,0 +1,46 @@ +//===--- 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 { +public: + enum AnalysisType { + FL_LINES, ///< Count lines, including comments and whitespace. + FL_STATEMENTS, ///< Count statements in the function. + FL_CYCLOMATIC ///< Calculate cyclomatic complexity for the function. + }; + +private: + const AnalysisType Type; + const unsigned Threshold; + llvm::DenseMap FunctionLengths; + +public: + FunctionLengthCheck(AnalysisType Type = FL_STATEMENTS, + unsigned Threshold = 800) + : Type(Type), Threshold(Threshold) {} + + 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/FunctionLength.cpp =================================================================== --- /dev/null +++ clang-tidy/misc/FunctionLength.cpp @@ -0,0 +1,115 @@ +//===--- 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" +#include "clang/Analysis/CFG.h" + +using namespace clang::ast_matchers; + +namespace clang { +namespace tidy { + +void FunctionLengthCheck::registerMatchers(MatchFinder *Finder) { + if (Type == FL_STATEMENTS) { + auto InTemplateInstantiation = hasAncestor( + decl(anyOf(recordDecl(ast_matchers::isTemplateInstantiation()), + functionDecl(ast_matchers::isTemplateInstantiation())))); + Finder->addMatcher(functionDecl(unless(InTemplateInstantiation), + forEachDescendant(compoundStmt().bind( + "block"))).bind("func"), + this); + } else { + Finder->addMatcher(functionDecl().bind("func"), this); + } +} + +void FunctionLengthCheck::check(const MatchFinder::MatchResult &Result) { + const auto *Func = Result.Nodes.getNodeAs("func"); + + // Count the lines including whitespace and comments. Really simple. + if (Type == FL_LINES) { + if (const Stmt *Body = Func->getBody()) { + SourceManager *SM = Result.SourceManager; + if (SM->isWrittenInSameFile(Body->getLocStart(), Body->getLocEnd())) { + FunctionLengths[Func] = SM->getPresumedLineNumber(Body->getLocEnd()) - + SM->getPresumedLineNumber(Body->getLocStart()); + } + } + return; + } + + // Calculate the number of statements in this function. + if (Type == FL_STATEMENTS) { + // Add the number of statements in this block. We'll automatically recurse + // into it with the ast matcher. + const auto *Block = Result.Nodes.getNodeAs("block"); + FunctionLengths[Func] += + std::distance(Block->child_begin(), Block->child_end()); + return; + } + + // Now a more complex analysis. Create a CFG and use it to calculate + // cyclomatic complexity. The CFG analysis does not work on template + // definitions so do the analysis at instantiation time. + if (!Func->getBody() || Func->isDependentContext()) + return; + + // Generate a CFG for the function. + // FIXME: Can we reuse the CFG from other analyses somehow? + CFG::BuildOptions BO; + BO.PruneTriviallyFalseEdges = false; + BO.AddEHEdges = false; + BO.AddInitializers = true; + BO.AddImplicitDtors = false; + BO.AddTemporaryDtors = false; + BO.AddCXXNewAllocator = false; + BO.setAllAlwaysAdd(); + std::unique_ptr G( + CFG::buildCFG(Func, Func->getBody(), Result.Context, BO)); + + // Count the number of blocks and edges in this graph. + unsigned NumBlocks = 0; + unsigned NumEdges = 0; + for (CFG::iterator I = G->begin(), E = G->end(); I != E; ++I) { + ++NumBlocks; + NumEdges += (*I)->succ_size(); + } + + FunctionLengths[Func] = NumEdges - NumBlocks + 2; +} + +void FunctionLengthCheck::onEndOfTranslationUnit() { + // If we're above the limit emit a warning. + for (const auto &P : FunctionLengths) { + if (P.second > Threshold) { + const char *T; + switch (Type) { + case FL_LINES: + T = "lines including whitespace"; + break; + case FL_STATEMENTS: + T = "statements"; + break; + case FL_CYCLOMATIC: + T = "in cyclomatic complexity"; + break; + } + + diag(P.first->getLocation(), + "function is very large, %0 %1 (threshold %2)") + << P.second << T << Threshold; + } + } + + FunctionLengths.clear(); +} + +} // namespace tidy +} // namespace clang 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()); CheckFactories.addCheckFactory( + "misc-function-length", + new ClangTidyCheckFactory()); + CheckFactories.addCheckFactory( "misc-redundant-smartptr-get", new ClangTidyCheckFactory()); 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,97 @@ +#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 Args; + + ast_matchers::MatchFinder Finder; + Check.registerMatchers(&Finder); + std::unique_ptr 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) { switch (i) { case 1: break;\n" + "case 2: break;}\n}"; +static const char *Code6 = "template T foo(T i) {return i;\n}\n" + "int x = foo(0);"; + +TEST(FunctionLengthCheckTest, LinesTest) { + FunctionLengthCheck C(FunctionLengthCheck::FL_LINES, 0); + + EXPECT_EQ("function is very large, 1 lines including whitespace (threshold 0)", + getWarning(C, Code1)); + EXPECT_EQ("no errors", + getWarning(C, Code2)); + EXPECT_EQ("function is very large, 3 lines including whitespace (threshold 0)", + getWarning(C, Code3)); + EXPECT_EQ("function is very large, 1 lines including whitespace (threshold 0)", + getWarning(C, Code4)); + EXPECT_EQ("function is very large, 2 lines including whitespace (threshold 0)", + getWarning(C, Code5)); + EXPECT_EQ("function is very large, 1 lines including whitespace (threshold 0)", + getWarning(C, Code6)); +} + +TEST(FunctionLengthCheckTest, StatementsTest) { + FunctionLengthCheck C(FunctionLengthCheck::FL_STATEMENTS, 0); + + 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, 2 statements (threshold 0)", + getWarning(C, Code4)); + EXPECT_EQ("function is very large, 3 statements (threshold 0)", + getWarning(C, Code5)); + EXPECT_EQ("function is very large, 1 statements (threshold 0)", + getWarning(C, Code6)); +} + +TEST(FunctionLengthCheckTest, CyclomaticTest) { + FunctionLengthCheck C(FunctionLengthCheck::FL_CYCLOMATIC, 0); + + EXPECT_EQ("function is very large, 1 in cyclomatic complexity (threshold 0)", + getWarning(C, Code1)); + EXPECT_EQ("function is very large, 1 in cyclomatic complexity (threshold 0)", + getWarning(C, Code2)); + EXPECT_EQ("function is very large, 1 in cyclomatic complexity (threshold 0)", + getWarning(C, Code3)); + EXPECT_EQ("function is very large, 2 in cyclomatic complexity (threshold 0)", + getWarning(C, Code4)); + EXPECT_EQ("function is very large, 3 in cyclomatic complexity (threshold 0)", + getWarning(C, Code5)); + EXPECT_EQ("function is very large, 1 in cyclomatic complexity (threshold 0)", + getWarning(C, Code6)); +} + +} // namespace test +} // namespace tidy +} // namespace clang