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,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 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/FunctionLength.cpp =================================================================== --- /dev/null +++ clang-tidy/misc/FunctionLength.cpp @@ -0,0 +1,92 @@ +//===--- 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) { + Finder->addMatcher( + functionDecl( + unless(isInstantiated()), + 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("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->getSpellingLineNumber(Body->getLocEnd()) - + SM->getSpellingLineNumber(Body->getLocStart()); + } + } + } + + const auto *Statement = Result.Nodes.getNodeAs("stmt"); + ++FI.Statements; + + // TODO: switch cases, gotos + if (isa(Statement) || isa(Statement) || + isa(Statement) || isa(Statement) || + isa(Statement) || isa(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 || FI.Statements > StatementThreshold || + FI.Branches > BranchThreshold) { + diag(P.first->getLocation(), + "function '%0' exceeds recommended size/complexity thresholds") + << P.first->getNameAsString(); + } + + if (FI.Lines > LineThreshold) { + diag(P.first->getLocation(), + "%0 lines including whitespace and comments (threshold %1)", + DiagnosticIDs::Note) + << FI.Lines << LineThreshold; + } + + if (FI.Statements > StatementThreshold) { + diag(P.first->getLocation(), "%0 statements (threshold %1)", + DiagnosticIDs::Note) + << FI.Statements << StatementThreshold; + } + + if (FI.Branches > BranchThreshold) { + diag(P.first->getLocation(), "%0 branches (threshold %1)", + DiagnosticIDs::Note) + << FI.Branches << BranchThreshold; + } + } + + FunctionInfos.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,94 @@ +#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(1, "-std=c++11"); + + 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"; + + // Munge error and notes into one long string. + std::string Out = + StringRef(Context.getErrors().back().Message.Message).rtrim(" []"); + for (const auto &Note : Context.getErrors().back().Notes) + Out += "\n" + StringRef(Note.Message).rtrim(" []").str(); + + return Out; +} +} // namespace + +TEST(FunctionLengthCheckTest, BasicTest) { + FunctionLengthCheck C(0U, 0U, 0U); + + const char *Code1 = "void foo() {\n}"; + EXPECT_EQ("no errors", getWarning(C, Code1)); + + const char *Code2 = "void foo() {;}"; + EXPECT_EQ("function 'foo' exceeds recommended size/complexity thresholds\n" + "1 statements (threshold 0)", + getWarning(C, Code2)); + + const char *Code3 = "void foo() {\n;\n\n}"; + EXPECT_EQ("function 'foo' exceeds recommended size/complexity thresholds\n" + "3 lines including whitespace and comments (threshold 0)\n" + "1 statements (threshold 0)", + getWarning(C, Code3)); + + const char *Code4 = "void foo(int i) { if (i) {} else; {}\n}"; + EXPECT_EQ("function 'foo' exceeds recommended size/complexity thresholds\n" + "1 lines including whitespace and comments (threshold 0)\n" + "3 statements (threshold 0)\n" + "1 branches (threshold 0)", + getWarning(C, Code4)); + + const char *Code5 = "void foo(int i) {for(;i;)while(i)\ndo;while(i);\n}"; + EXPECT_EQ("function 'foo' exceeds recommended size/complexity thresholds\n" + "2 lines including whitespace and comments (threshold 0)\n" + "7 statements (threshold 0)\n" + "3 branches (threshold 0)", + getWarning(C, Code5)); + + const char *Code6 = "template T foo(T i) {return i;\n}\n" + "int x = foo(0);"; + EXPECT_EQ("function 'foo' exceeds recommended size/complexity thresholds\n" + "1 lines including whitespace and comments (threshold 0)\n" + "1 statements (threshold 0)", + getWarning(C, Code6)); + + const char *Code7 = "void bar() { [](){;;;;;;;;;;;if(1){}}(); \n\n\n}"; + EXPECT_EQ("function 'bar' exceeds recommended size/complexity thresholds\n" + "3 lines including whitespace and comments (threshold 0)\n" + "14 statements (threshold 0)\n" + "1 branches (threshold 0)", + getWarning(C, Code7)); + + const char *Code8 = "void bar() { class A { void x() {} }; }"; + EXPECT_EQ("function 'bar' exceeds recommended size/complexity thresholds\n" + "1 statements (threshold 0)", + getWarning(C, Code8)); +} + +} // namespace test +} // namespace tidy +} // namespace clang