diff --git a/plugins/cpp/model/include/model/cppfunction.h b/plugins/cpp/model/include/model/cppfunction.h index 2492d9f07..938f04aa4 100644 --- a/plugins/cpp/model/include/model/cppfunction.h +++ b/plugins/cpp/model/include/model/cppfunction.h @@ -18,7 +18,10 @@ struct CppFunction : CppTypedEntity std::vector> parameters; #pragma db on_delete(cascade) std::vector> locals; + unsigned int mccabe; + unsigned int bumpiness; + unsigned int statementCount; std::string toString() const { @@ -82,6 +85,25 @@ struct CppFunctionMcCabe std::string filePath; }; +#pragma db view \ + object(CppFunction) \ + object(CppAstNode : CppFunction::astNodeId == CppAstNode::id) \ + object(File : CppAstNode::location.file) +struct CppFunctionBumpyRoad +{ + #pragma db column(CppEntity::astNodeId) + CppAstNodeId astNodeId; + + #pragma db column(CppFunction::bumpiness) + unsigned int bumpiness; + + #pragma db column(CppFunction::statementCount) + unsigned int statementCount; + + #pragma db column(File::path) + std::string filePath; +}; + } } diff --git a/plugins/cpp/parser/CMakeLists.txt b/plugins/cpp/parser/CMakeLists.txt index 136d25f02..43c2cffb6 100644 --- a/plugins/cpp/parser/CMakeLists.txt +++ b/plugins/cpp/parser/CMakeLists.txt @@ -44,7 +44,8 @@ add_library(cppparser SHARED src/ppmacrocallback.cpp src/relationcollector.cpp src/doccommentformatter.cpp - src/diagnosticmessagehandler.cpp) + src/diagnosticmessagehandler.cpp + src/nestedscope.cpp) target_link_libraries(cppparser cppmodel diff --git a/plugins/cpp/parser/src/clangastvisitor.h b/plugins/cpp/parser/src/clangastvisitor.h index e07e8928e..2a56c0b31 100644 --- a/plugins/cpp/parser/src/clangastvisitor.h +++ b/plugins/cpp/parser/src/clangastvisitor.h @@ -39,11 +39,13 @@ #include #include #include +#include #include #include "entitycache.h" #include "symbolhelper.h" +#include "nestedscope.h" namespace cc { @@ -70,6 +72,8 @@ namespace parser class ClangASTVisitor : public clang::RecursiveASTVisitor { public: + using Base = clang::RecursiveASTVisitor; + ClangASTVisitor( ParserContext& ctx_, clang::ASTContext& astContext_, @@ -128,445 +132,387 @@ class ClangASTVisitor : public clang::RecursiveASTVisitor }); } + + // AST Visitor configuration + bool shouldVisitImplicitCode() const { return true; } bool shouldVisitTemplateInstantiations() const { return true; } bool shouldVisitLambdaBody() const { return true; } - bool TraverseDecl(clang::Decl* decl_) - { - // We use implicitness to determine if actual symbol location information - // should be stored for AST nodes in our database. This differs somewhat - // from Clang's concept of implicitness. - // To bridge the gap between the two interpretations, we mostly assume - // implicitness to be hereditary from parent to child nodes, except - // in some known special cases (see lambdas in TraverseCXXRecordDecl). - bool wasImplicit = _isImplicit; - if (decl_ != nullptr) - _isImplicit |= decl_->isImplicit(); - - bool b = Base::TraverseDecl(decl_); - - _isImplicit = wasImplicit; - return b; - } + // Traverse Decl helpers - bool TraverseFunctionDecl(clang::FunctionDecl* fd_) + class TypeScope final { - if (fd_->doesThisDeclarationHaveABody()) - return TraverseFunctionDeclWithBody(fd_); + DECLARE_SCOPED_TYPE(TypeScope) + + private: + ClangASTVisitor* _visitor; + model::CppRecordPtr _type; - _functionStack.push(std::make_shared()); - - bool b = Base::TraverseFunctionDecl(fd_); - - if (_functionStack.top()->astNodeId) - _functions.push_back(_functionStack.top()); - _functionStack.pop(); - - return b; - } - - bool TraverseFunctionDeclWithBody(clang::FunctionDecl* fd_) - { - _functionStack.push(std::make_shared()); - _mcCabeStack.push(1); - - bool b = Base::TraverseFunctionDecl(fd_); - - model::CppFunctionPtr top = _functionStack.top(); - if (top->astNodeId) + public: + TypeScope( + ClangASTVisitor* visitor_ + ) : + _visitor(visitor_), + _type(std::make_shared()) { - top->mccabe = _mcCabeStack.top(); - _functions.push_back(top); + _visitor->_typeStack.push(_type); } - _mcCabeStack.pop(); - _functionStack.pop(); - return b; - } + ~TypeScope() + { + assert(_type == _visitor->_typeStack.top() && + "Type scope destruction order has been violated."); + if (_type->astNodeId) + _visitor->_types.push_back(_type); + _visitor->_typeStack.pop(); + } + }; - bool TraverseCXXDeductionGuideDecl(clang::CXXDeductionGuideDecl* fd_) + class EnumScope final { - if (fd_->doesThisDeclarationHaveABody()) - return TraverseCXXDeductionGuideDeclWithBody(fd_); + DECLARE_SCOPED_TYPE(EnumScope) - _functionStack.push(std::make_shared()); + private: + ClangASTVisitor* _visitor; + model::CppEnumPtr _enum; - bool b = Base::TraverseCXXDeductionGuideDecl(fd_); - - if (_functionStack.top()->astNodeId) - _functions.push_back(_functionStack.top()); - _functionStack.pop(); + public: + EnumScope( + ClangASTVisitor* visitor_ + ) : + _visitor(visitor_), + _enum(std::make_shared()) + { + _visitor->_enumStack.push(_enum); + } - return b; - } + ~EnumScope() + { + assert(_enum == _visitor->_enumStack.top() && + "Enum scope destruction order has been violated."); + if (_enum->astNodeId) + _visitor->_enums.push_back(_enum); + _visitor->_enumStack.pop(); + } + }; - bool TraverseCXXDeductionGuideDeclWithBody(clang::CXXDeductionGuideDecl* fd_) + class FunctionScope final { - _functionStack.push(std::make_shared()); - _mcCabeStack.push(1); + DECLARE_SCOPED_TYPE(FunctionScope) - bool b = Base::TraverseCXXDeductionGuideDecl(fd_); + private: + ClangASTVisitor* _visitor; + model::CppFunctionPtr _curFun; - model::CppFunctionPtr top = _functionStack.top(); - if (top->astNodeId) + public: + FunctionScope(ClangASTVisitor* visitor_) : + _visitor(visitor_), + _curFun(std::make_shared()) { - top->mccabe = _mcCabeStack.top(); - _functions.push_back(top); + _visitor->_functionStack.push(_curFun); } - _mcCabeStack.pop(); - _functionStack.pop(); - return b; - } - - bool TraverseCXXMethodDecl(clang::CXXMethodDecl* fd_) - { - if (fd_->doesThisDeclarationHaveABody()) - return TraverseCXXMethodDeclWithBody(fd_); - - _functionStack.push(std::make_shared()); + ~FunctionScope() + { + assert(_curFun == _visitor->_functionStack.top() && + "Function scope destruction order has been violated."); + if (_curFun->astNodeId) + _visitor->_functions.push_back(_curFun); + _visitor->_functionStack.pop(); - bool b = Base::TraverseCXXMethodDecl(fd_); + if (_curFun->astNodeId && !_visitor->_functionStack.empty()) + { + StatementScope* scope = _visitor->_stmtStack.TopValid(); + assert(scope != nullptr && + "Previous function entry has no corresponding scope stack entry."); + scope->EnsureConfigured(); + + // If the currently parsed function had a total bumpiness of B + // from S statements, and is nested inside an enclosing function + // at depth D, then the total bumpiness of that enclosing function + // must be increased by the total bumpiness of the nested function: + // B (core nested bumpiness) + S * D (accounting for the indentation). + model::CppFunctionPtr& prevFun = _visitor->_functionStack.top(); + prevFun->bumpiness += _curFun->bumpiness + + _curFun->statementCount * scope->Depth(); + prevFun->statementCount += _curFun->statementCount; + } + } + }; - if (_functionStack.top()->astNodeId) - _functions.push_back(_functionStack.top()); - _functionStack.pop(); - return b; - } + // Traverse Decl - bool TraverseCXXMethodDeclWithBody(clang::CXXMethodDecl* fd_) + bool TraverseCXXRecordDecl(clang::CXXRecordDecl* rd_) { - _functionStack.push(std::make_shared()); - _mcCabeStack.push(1); - - bool b = Base::TraverseCXXMethodDecl(fd_); - - model::CppFunctionPtr top = _functionStack.top(); - if (top->astNodeId) - { - top->mccabe = _mcCabeStack.top(); - _functions.push_back(top); - } - _mcCabeStack.pop(); - _functionStack.pop(); - - return b; + // Although lambda closure types are implicit by nature as far as + // Clang is concerned, their operator() is not. In order to be able to + // properly assign symbol location information to AST nodes within + // lambda bodies, we must force lambdas to be considered explicit. + util::ScopedValue sv(_isImplicit, _isImplicit && !rd_->isLambda()); + return Base::TraverseCXXRecordDecl(rd_); } - bool TraverseCXXConstructorDecl(clang::CXXConstructorDecl* fd_) + bool TraverseEnumDecl(clang::EnumDecl* ed_) { - if (fd_->doesThisDeclarationHaveABody()) - return TraverseCXXConstructorDeclWithBody(fd_); - - _functionStack.push(std::make_shared()); - - bool b = Base::TraverseCXXConstructorDecl(fd_); - - if (_functionStack.top()->astNodeId) - _functions.push_back(_functionStack.top()); - _functionStack.pop(); - - return b; + // Any type of enum must be placed on the enum stack + // for the duration of its traversal. + // The scope creates a database object for the enum + // at the beginning, and persists it at the end. + EnumScope es(this); + return Base::TraverseEnumDecl(ed_); } - bool TraverseCXXConstructorDeclWithBody(clang::CXXConstructorDecl* fd_) - { - _functionStack.push(std::make_shared()); - _mcCabeStack.push(1); - bool b = Base::TraverseCXXConstructorDecl(fd_); + bool TraverseDecl(clang::Decl* d_) + { + if (d_ == nullptr) + return Base::TraverseDecl(d_); - model::CppFunctionPtr top = _functionStack.top(); - if (top->astNodeId) + // We use implicitness to determine if actual symbol location information + // should be stored for AST nodes in our database. This differs somewhat + // from Clang's concept of implicitness. + // To bridge the gap between the two interpretations, we mostly assume + // implicitness to be hereditary from parent to child nodes, except + // in some known special cases (see lambdas in TraverseCXXRecordDecl). + util::ScopedValue sv(_isImplicit, _isImplicit || d_->isImplicit()); + + if (clang::FunctionDecl* fd = llvm::dyn_cast(d_)) { - top->mccabe = _mcCabeStack.top(); - _functions.push_back(top); + // Any type of function must be placed on the function stack + // for the duration of its traversal. + // The scope creates a database object for the function + // at the beginning, and persists it at the end. + FunctionScope fs(this); + // We must also make an initial scope entry for the function's body. + StatementScope ss(&_stmtStack, nullptr); + ss.MakeFunction(fd->getBody()); + return Base::TraverseDecl(fd); } - _mcCabeStack.pop(); - _functionStack.pop(); - - return b; + else if (clang::RecordDecl* rd = llvm::dyn_cast(d_)) + { + // Any type of record/type must be placed on the type stack + // for the duration of its traversal. + // The scope creates a database object for the record/type + // at the beginning, and persists it at the end. + TypeScope ts(this); + return Base::TraverseDecl(rd); + } + else return Base::TraverseDecl(d_); } - bool TraverseCXXDestructorDecl(clang::CXXDestructorDecl* fd_) - { - if (fd_->doesThisDeclarationHaveABody()) - return TraverseCXXDestructorDeclWithBody(fd_); - - _functionStack.push(std::make_shared()); - bool b = Base::TraverseCXXDestructorDecl(fd_); + // Metrics helpers - if (_functionStack.top()->astNodeId) - _functions.push_back(_functionStack.top()); - _functionStack.pop(); - - return b; + void CountMcCabe() + { + if (!_functionStack.empty()) + ++_functionStack.top()->mccabe; } - bool TraverseCXXDestructorDeclWithBody(clang::CXXDestructorDecl* fd_) + void CountBumpiness(const StatementScope& scope_) { - _functionStack.push(std::make_shared()); - _mcCabeStack.push(1); - - bool b = Base::TraverseCXXDestructorDecl(fd_); - - model::CppFunctionPtr top = _functionStack.top(); - if (top->astNodeId) + if (!_functionStack.empty() && scope_.IsReal()) { - top->mccabe = _mcCabeStack.top(); - _functions.push_back(top); + model::CppFunctionPtr& fun = _functionStack.top(); + fun->bumpiness += scope_.Depth(); + ++fun->statementCount; } - _mcCabeStack.pop(); - _functionStack.pop(); - - return b; } - bool TraverseCXXConversionDecl(clang::CXXConversionDecl* fd_) - { - if (fd_->doesThisDeclarationHaveABody()) - return TraverseCXXConversionDeclWithBody(fd_); - - _functionStack.push(std::make_shared()); - bool b = Base::TraverseCXXConversionDecl(fd_); + // Traverse Stmt helpers - if (_functionStack.top()->astNodeId) - _functions.push_back(_functionStack.top()); - _functionStack.pop(); - - return b; - } - - bool TraverseCXXConversionDeclWithBody(clang::CXXConversionDecl* fd_) + class CtxStmtScope final { - _functionStack.push(std::make_shared()); - _mcCabeStack.push(1); + DECLARE_SCOPED_TYPE(CtxStmtScope) - bool b = Base::TraverseCXXConversionDecl(fd_); + private: + ClangASTVisitor* _visitor; - model::CppFunctionPtr top = _functionStack.top(); - if (top->astNodeId) + public: + CtxStmtScope( + ClangASTVisitor* visitor_, + clang::Stmt* stmt_ + ) : + _visitor(visitor_) { - top->mccabe = _mcCabeStack.top(); - _functions.push_back(top); + _visitor->_contextStatementStack.push(stmt_); } - _mcCabeStack.pop(); - _functionStack.pop(); - - return b; - } - bool TraverseRecordDecl(clang::RecordDecl* rd_) - { - _typeStack.push(std::make_shared()); - - bool b = Base::TraverseRecordDecl(rd_); + ~CtxStmtScope() + { + _visitor->_contextStatementStack.pop(); + } + }; - if (_typeStack.top()->astNodeId) - _types.push_back(_typeStack.top()); - _typeStack.pop(); - return b; - } + // Traverse Expr (Expr : Stmt) - bool TraverseCXXRecordDecl(clang::CXXRecordDecl* rd_) + bool TraverseCallExpr(clang::CallExpr* ce_) { - // Although lambda closure types are implicit by nature as far as - // Clang is concerned, their operator() is not. In order to be able to - // properly assign symbol location information to AST nodes within - // lambda bodies, we must force lambdas to be considered explicit. - bool wasImplicit = _isImplicit; - if (rd_ != nullptr) - _isImplicit &= !rd_->isLambda(); - _typeStack.push(std::make_shared()); - - bool b = Base::TraverseCXXRecordDecl(rd_); - - if (_typeStack.top()->astNodeId) - _types.push_back(_typeStack.top()); - _typeStack.pop(); - _isImplicit = wasImplicit; - - return b; + CtxStmtScope css(this, ce_); + return Base::TraverseCallExpr(ce_); } - bool TraverseClassTemplateSpecializationDecl( - clang::ClassTemplateSpecializationDecl* rd_) + bool TraverseMemberExpr(clang::MemberExpr* me_) { - _typeStack.push(std::make_shared()); - - bool b = Base::TraverseClassTemplateSpecializationDecl(rd_); - - if (_typeStack.top()->astNodeId) - _types.push_back(_typeStack.top()); - _typeStack.pop(); - - return b; + CtxStmtScope css(this, me_); + return Base::TraverseMemberExpr(me_); } - bool TraverseClassTemplatePartialSpecializationDecl( - clang::ClassTemplatePartialSpecializationDecl* rd_) + bool TraverseCXXDeleteExpr(clang::CXXDeleteExpr* de_) { - _typeStack.push(std::make_shared()); - - bool b = Base::TraverseClassTemplatePartialSpecializationDecl(rd_); - - if (_typeStack.top()->astNodeId) - _types.push_back(_typeStack.top()); - _typeStack.pop(); - - return b; + CtxStmtScope css(this, de_); + return Base::TraverseCXXDeleteExpr(de_); } - bool TraverseEnumDecl(clang::EnumDecl* ed_) + bool TraverseBinaryOperator(clang::BinaryOperator* bo_) { - _enumStack.push(std::make_shared()); - - bool b = Base::TraverseEnumDecl(ed_); - - if (_enumStack.top()->astNodeId) - _enums.push_back(_enumStack.top()); - _enumStack.pop(); - - return b; + if (bo_->isLogicalOp()) CountMcCabe(); + CtxStmtScope css(this, bo_); + return Base::TraverseBinaryOperator(bo_); } - bool TraverseCallExpr(clang::CallExpr* ce_) + bool TraverseConditionalOperator(clang::ConditionalOperator* co_) { - _contextStatementStack.push(ce_); - bool b = Base::TraverseCallExpr(ce_); - _contextStatementStack.pop(); - return b; + CountMcCabe(); + return Base::TraverseConditionalOperator(co_); } - bool TraverseDeclStmt(clang::DeclStmt* ds_) + bool TraverseBinaryConditionalOperator( + clang::BinaryConditionalOperator* bco_) { - _contextStatementStack.push(ds_); - bool b = Base::TraverseDeclStmt(ds_); - _contextStatementStack.pop(); - return b; + CountMcCabe(); + return Base::TraverseBinaryConditionalOperator(bco_); } - bool TraverseMemberExpr(clang::MemberExpr* me_) + + // Traverse Stmt + + bool TraverseCompoundStmt(clang::CompoundStmt* cs_) { - _contextStatementStack.push(me_); - bool b = Base::TraverseMemberExpr(me_); - _contextStatementStack.pop(); - return b; + _stmtStack.Top()->MakeCompound(); + return Base::TraverseCompoundStmt(cs_); } - bool TraverseBinaryOperator(clang::BinaryOperator* bo_) + bool TraverseDeclStmt(clang::DeclStmt* ds_) { - if (bo_->isLogicalOp() && !_mcCabeStack.empty()) - ++_mcCabeStack.top(); - _contextStatementStack.push(bo_); - bool b = Base::TraverseBinaryOperator(bo_); - _contextStatementStack.pop(); - return b; + CtxStmtScope css(this, ds_); + return Base::TraverseDeclStmt(ds_); } bool TraverseReturnStmt(clang::ReturnStmt* rs_) { - _contextStatementStack.push(rs_); - bool b = Base::TraverseReturnStmt(rs_); - _contextStatementStack.pop(); - return b; + CtxStmtScope css(this, rs_); + return Base::TraverseReturnStmt(rs_); } - bool TraverseCXXDeleteExpr(clang::CXXDeleteExpr* de_) + bool TraverseIfStmt(clang::IfStmt* is_) { - _contextStatementStack.push(de_); - bool b = Base::TraverseCXXDeleteExpr(de_); - _contextStatementStack.pop(); - return b; + _stmtStack.Top()->MakeTwoWay(is_->getThen(), is_->getElse()); + CountMcCabe(); + return Base::TraverseIfStmt(is_); } - template - bool StatementStackDecorator(T* stmt_, const std::function& func_) + bool TraverseWhileStmt(clang::WhileStmt* ws_) { - _statements.push(stmt_); - bool b = func_(stmt_); - _statements.pop(); - return b; + _stmtStack.Top()->MakeOneWay(ws_->getBody()); + CountMcCabe(); + return Base::TraverseWhileStmt(ws_); } - template - bool McCabeDecorator(T* stmt_, const std::function& func_) + bool TraverseDoStmt(clang::DoStmt* ds_) { - if(!_mcCabeStack.empty()) - ++_mcCabeStack.top(); - return StatementStackDecorator(stmt_, func_); + _stmtStack.Top()->MakeOneWay(ds_->getBody()); + CountMcCabe(); + return Base::TraverseDoStmt(ds_); } - bool TraverseIfStmt(clang::IfStmt* stmt_) + bool TraverseForStmt(clang::ForStmt* fs_) { - return McCabeDecorator(stmt_, [this] (clang::IfStmt* s) { return Base::TraverseIfStmt(s); }); + _stmtStack.Top()->MakeOneWay(fs_->getBody()); + CountMcCabe(); + return Base::TraverseForStmt(fs_); } - bool TraverseWhileStmt(clang::WhileStmt* stmt_) + bool TraverseCXXForRangeStmt(clang::CXXForRangeStmt* frs_) { - return McCabeDecorator(stmt_, [this] (clang::WhileStmt* s) { return Base::TraverseWhileStmt(s); }); + _stmtStack.Top()->MakeOneWay(frs_->getBody()); + CountMcCabe(); + return Base::TraverseCXXForRangeStmt(frs_); } - bool TraverseDoStmt(clang::DoStmt* stmt_) + bool TraverseSwitchStmt(clang::SwitchStmt* ss_) { - return McCabeDecorator(stmt_, [this] (clang::DoStmt* s) { return Base::TraverseDoStmt(s); }); + _stmtStack.Top()->MakeOneWay(ss_->getBody()); + return Base::TraverseSwitchStmt(ss_); } - bool TraverseForStmt(clang::ForStmt* stmt_) + bool TraverseCaseStmt(clang::CaseStmt* cs_) { - return McCabeDecorator(stmt_, [this] (clang::ForStmt* s) { return Base::TraverseForStmt(s); }); + _stmtStack.Top()->MakeTransparent(); + CountMcCabe(); + return Base::TraverseCaseStmt(cs_); } - bool TraverseCXXForRangeStmt(clang::CXXForRangeStmt* stmt_) + bool TraverseDefaultStmt(clang::DefaultStmt* ds_) { - return McCabeDecorator(stmt_, [this] (clang::CXXForRangeStmt* s) { return Base::TraverseCXXForRangeStmt(s); }); + _stmtStack.Top()->MakeTransparent(); + CountMcCabe(); + return Base::TraverseDefaultStmt(ds_); } - bool TraverseCaseStmt(clang::CaseStmt* stmt_) + bool TraverseContinueStmt(clang::ContinueStmt* cs_) { - return McCabeDecorator(stmt_, [this] (clang::CaseStmt* s) { return Base::TraverseCaseStmt(s); }); + CountMcCabe(); + return Base::TraverseContinueStmt(cs_); } - bool TraverseDefaultStmt(clang::DefaultStmt* stmt_) + bool TraverseGotoStmt(clang::GotoStmt* gs_) { - return McCabeDecorator(stmt_, [this] (clang::DefaultStmt* s) { return Base::TraverseDefaultStmt(s); }); + CountMcCabe(); + return Base::TraverseGotoStmt(gs_); } - bool TraverseContinueStmt(clang::ContinueStmt* stmt_) + bool TraverseCXXTryStmt(clang::CXXTryStmt* ts_) { - return McCabeDecorator(stmt_, [this] (clang::ContinueStmt* s) { return Base::TraverseContinueStmt(s); }); + _stmtStack.Top()->MakeOneWay(ts_->getTryBlock()); + return Base::TraverseCXXTryStmt(ts_); } - bool TraverseGotoStmt(clang::GotoStmt* stmt_) + bool TraverseCXXCatchStmt(clang::CXXCatchStmt* cs_) { - return McCabeDecorator(stmt_, [this] (clang::GotoStmt* s) { return Base::TraverseGotoStmt(s); }); + _stmtStack.Top()->MakeOneWay(cs_->getHandlerBlock()); + CountMcCabe(); + return Base::TraverseCXXCatchStmt(cs_); } - bool TraverseCXXCatchStmt(clang::CXXCatchStmt* stmt_) - { - return McCabeDecorator(stmt_, [this] (clang::CXXCatchStmt* s) { return Base::TraverseCXXCatchStmt(s); }); - } - bool TraverseConditionalOperator(clang::ConditionalOperator* op_) + bool TraverseStmt(clang::Stmt* s_) { - return McCabeDecorator(op_, [this] (clang::ConditionalOperator* s) { return Base::TraverseConditionalOperator(s); }); - } + if (s_ == nullptr) + return Base::TraverseStmt(s_); - bool TraverseBinaryConditionalOperator(clang::BinaryConditionalOperator* op_) - { - return McCabeDecorator(op_, [this] (clang::BinaryConditionalOperator* s) { return Base::TraverseBinaryConditionalOperator(s); }); + // Create a statement scope for the current statement for the duration + // of its traversal. For every statement, there must be exactly one scope. + StatementScope scope(&_stmtStack, s_); + bool b = Base::TraverseStmt(s_); + // Base::TraverseStmt will select the best handler function for the + // statement's dynamic type. If it is not any of the special statement + // types we are explicitly handling, it must be a "normal" statement. + // In that case, none of the scope's specialized Make* member functions + // will be called. Even so, before counting its nestedness, + // we must ensure it is configured at least as a normal statement. + scope.EnsureConfigured(); + CountBumpiness(scope); + return b; } - bool TraverseStmt(clang::Stmt* stmt_) - { - return StatementStackDecorator(stmt_, [this] (clang::Stmt* s) { return Base::TraverseStmt(s); }); - } + + // Visit functions bool VisitTypedefTypeLoc(clang::TypedefTypeLoc tl_) { @@ -932,7 +878,9 @@ class ClangASTVisitor : public clang::RecursiveASTVisitor cppFunction->qualifiedName = fn_->getQualifiedNameAsString(); cppFunction->typeHash = util::fnvHash(getUSR(qualType, _astContext)); cppFunction->qualifiedType = qualType.getAsString(); - cppFunction->mccabe = 1; + cppFunction->mccabe = fn_->isThisDeclarationADefinition() ? 1 : 0; + cppFunction->bumpiness = 0; + cppFunction->statementCount = 0; //--- Tags ---// @@ -1176,18 +1124,23 @@ class ClangASTVisitor : public clang::RecursiveASTVisitor bool VisitDeclStmt(clang::DeclStmt* ds_) { - assert(_statements.top() == ds_ && - "ds_ was expected to be the top-level statement."); - _statements.pop(); - clang::Stmt* scopeSt = _statements.top(); - _statements.push(ds_); + clang::Stmt* s; + StatementScope* scope = _stmtStack.TopValid(); + if (scope->Statement() == ds_) + { + assert(scope->Previous() != nullptr && + "Declaration statement is not nested in a scope."); + s = scope->Previous()->Statement(); + } + else + s = scope->Statement(); for (auto it = ds_->decl_begin(); it != ds_->decl_end(); ++it) { if (clang::VarDecl* vd = llvm::dyn_cast(*it)) { model::FileLoc loc = - getFileLoc(scopeSt->getEndLoc(), scopeSt->getEndLoc()); + getFileLoc(s->getEndLoc(), s->getEndLoc()); addDestructorUsage(vd->getType(), loc, vd); } } @@ -1567,8 +1520,6 @@ class ClangASTVisitor : public clang::RecursiveASTVisitor } private: - using Base = clang::RecursiveASTVisitor; - void addDestructorUsage( clang::QualType type_, const model::FileLoc& location_, @@ -1870,7 +1821,6 @@ class ClangASTVisitor : public clang::RecursiveASTVisitor std::stack _functionStack; std::stack _typeStack; std::stack _enumStack; - std::stack _mcCabeStack; bool _isImplicit; ParserContext& _ctx; @@ -1896,8 +1846,7 @@ class ClangASTVisitor : public clang::RecursiveASTVisitor std::unordered_map _locToAstType; std::unordered_map _locToAstValue; - // This stack contains the parent chain of the current Stmt. - std::stack _statements; + StatementStack _stmtStack; // This stack has the same role as _locTo* maps. In case of // clang::DeclRefExpr objects we need to determine the contect of the given // expression. In this stack we store the deepest statement node in AST which diff --git a/plugins/cpp/parser/src/nestedscope.cpp b/plugins/cpp/parser/src/nestedscope.cpp new file mode 100644 index 000000000..824eeca24 --- /dev/null +++ b/plugins/cpp/parser/src/nestedscope.cpp @@ -0,0 +1,166 @@ +#include "nestedscope.h" + +namespace cc +{ +namespace parser +{ + + StatementScope* StatementStack::TopValid() const + { + StatementScope* top = _top; + while (top != nullptr && top->_state == StatementScope::State::Invalid) + top = top->Previous(); + return top; + } + + + StatementScope::State StatementScope::CheckNext(clang::Stmt* stmt_) const + { + if (_state == State::Invalid) + return State::Invalid; + switch (_kind) + { + case Kind::Open: + return State::Standalone; + case Kind::Closed: + return State::Invalid; + case Kind::OneWay: + return (stmt_ == _exp0) + ? State::Expected : State::Invalid; + case Kind::TwoWay: + return (stmt_ == _exp0 || stmt_ == _exp1) + ? State::Expected : State::Invalid; + default: + assert(false && "This scope has not been configured yet."); + return State::Invalid; + } + } + + + StatementScope::StatementScope( + StatementStack* stack_, + clang::Stmt* stmt_ + ) : + _stack(stack_), + _previous(_stack->_top), + _stmt(stmt_), + _depth(0), + _state(State::Initial), + _kind(Kind::Unknown), + _exp0(nullptr), + _exp1(nullptr) + { + if (_previous != nullptr) + { + _previous->EnsureConfigured(); + _depth = _previous->_depth; + if (_stmt != nullptr) + _state = _previous->CheckNext(_stmt); + } + _stack->_top = this; + } + + StatementScope::~StatementScope() + { + assert(_stack->_top == this && + "Scope destruction order has been violated."); + _stack->_top = _previous; + } + + + void StatementScope::EnsureConfigured() + { + if (_kind == Kind::Unknown) + MakeStatement(); + } + + void StatementScope::MakeTransparent() + { + assert(_kind == Kind::Unknown && + "Scope has already been configured; it cannot be reconfigured."); + + // Anything inside a transparent statement block is standalone, + // we do not expect any particular type of statement to be nested in it. + _kind = Kind::Open; + } + + void StatementScope::MakeCompound() + { + assert(_kind == Kind::Unknown && + "Scope has already been configured; it cannot be reconfigured."); + + // Anything inside a compound statement block is standalone, + // we do not expect any particular type of statement to be nested in it. + _kind = Kind::Open; + + // A compound statement block only counts as a non-transparent scope + // when it is not the expected body of any other statement. + if (_state == State::Standalone) + ++_depth; + } + + void StatementScope::MakeStatement() + { + assert(_kind == Kind::Unknown && + "Scope has already been configured; it cannot be reconfigured."); + + // A non-specialized statement scope is a single statement. + // Anything inside a single statement (e.g sub-expressions) is not + // something to be counted towards the total bumpiness of a function. + _kind = Kind::Closed; + + // As long as the statement itself is valid on the stack, + // it is a real scoped statement. + if (_state != State::Invalid) + ++_depth; + } + + void StatementScope::MakeOneWay(clang::Stmt* next_) + { + assert(_kind == Kind::Unknown && + "Scope has already been configured; it cannot be reconfigured."); + + // A one-way scope expects a particular statement to be nested inside it. + // Anything else above it on the stack is invalid. + _kind = Kind::OneWay; + _exp0 = next_; + + // As long as the statement itself is valid on the stack, + // it is a real scoped statement. + if (_state != State::Invalid) + ++_depth; + } + + void StatementScope::MakeFunction(clang::Stmt* body_) + { + assert(_kind == Kind::Unknown && + "Scope has already been configured; it cannot be reconfigured."); + + // A function scope expects its body to be nested inside it. + // Anything else above it on the stack is invalid. + _kind = Kind::OneWay; + _exp0 = body_; + + // The level of nestedness always starts from zero at the function level. + _depth = 0; + } + + void StatementScope::MakeTwoWay(clang::Stmt* next0_, clang::Stmt* next1_) + { + assert(_kind == Kind::Unknown && + "Scope has already been configured; it cannot be reconfigured."); + + // A two-way scope expects either of two particular statements to be + // nested inside it. Anything else above it on the stack is invalid. + _kind = Kind::TwoWay; + _exp0 = next0_; + _exp1 = next1_; + + // As long as the statement itself is valid on the stack, + // it is a real scoped statement. + if (_state != State::Invalid) + ++_depth; + } + +} +} diff --git a/plugins/cpp/parser/src/nestedscope.h b/plugins/cpp/parser/src/nestedscope.h new file mode 100644 index 000000000..0fdad6bc2 --- /dev/null +++ b/plugins/cpp/parser/src/nestedscope.h @@ -0,0 +1,91 @@ +#ifndef CC_PARSER_NESTEDSCOPE_H +#define CC_PARSER_NESTEDSCOPE_H + +#include + +#include + +namespace cc +{ +namespace parser +{ + + class StatementScope; + + class StatementStack final + { + DECLARE_SCOPED_TYPE(StatementStack) + + friend class StatementScope; + + private: + StatementScope* _top; + + public: + StatementScope* Top() const { return _top; } + StatementScope* TopValid() const; + + StatementStack() : _top(nullptr) {} + }; + + class StatementScope final + { + DECLARE_SCOPED_TYPE(StatementScope) + + friend class StatementStack; + + private: + enum class State : unsigned char + { + Initial,// This statement is the root in its function. + Expected,// This statement was expected to be nested inside its parent. + Standalone,// This statement was not expected, but not forbidden either. + Invalid,// This statement was not expected to be on the stack. + }; + + enum class Kind : unsigned char + { + Unknown,// This scope has not been configured yet. + Open,// Any statement can be nested inside this statement. + Closed,// No other statement is allowed to be nested inside. + OneWay,// Only one specific statement is allowed to be nested inside. + TwoWay,// Only two specific statements are allowed to be nested inside. + }; + + StatementStack* _stack; + StatementScope* _previous; + clang::Stmt* _stmt; + unsigned int _depth; + State _state; + Kind _kind; + clang::Stmt* _exp0; + clang::Stmt* _exp1; + + State CheckNext(clang::Stmt* stmt_) const; + + public: + StatementStack* Stack() const { return _stack; } + StatementScope* Previous() const { return _previous; } + clang::Stmt* Statement() const { return _stmt; } + + unsigned int PrevDepth() const + { return _previous != nullptr ? _previous->_depth : 0; } + unsigned int Depth() const { return _depth; } + bool IsReal() const { return Depth() > PrevDepth(); } + + StatementScope(StatementStack* stack_, clang::Stmt* stmt_); + ~StatementScope(); + + void EnsureConfigured(); + void MakeTransparent(); + void MakeCompound(); + void MakeStatement(); + void MakeOneWay(clang::Stmt* next_); + void MakeFunction(clang::Stmt* body_); + void MakeTwoWay(clang::Stmt* next0_, clang::Stmt* next1_); + }; + +} +} + +#endif diff --git a/plugins/cpp_metrics/model/include/model/cppastnodemetrics.h b/plugins/cpp_metrics/model/include/model/cppastnodemetrics.h index eb17cdf3d..f95b4a6e1 100644 --- a/plugins/cpp_metrics/model/include/model/cppastnodemetrics.h +++ b/plugins/cpp_metrics/model/include/model/cppastnodemetrics.h @@ -17,8 +17,9 @@ struct CppAstNodeMetrics { PARAMETER_COUNT = 1, MCCABE = 2, - LACK_OF_COHESION = 3, - LACK_OF_COHESION_HS = 4, + BUMPY_ROAD = 3, + LACK_OF_COHESION = 4, + LACK_OF_COHESION_HS = 5, }; #pragma db id auto diff --git a/plugins/cpp_metrics/parser/include/cppmetricsparser/cppmetricsparser.h b/plugins/cpp_metrics/parser/include/cppmetricsparser/cppmetricsparser.h index 6dcfb0bc2..5abe1af83 100644 --- a/plugins/cpp_metrics/parser/include/cppmetricsparser/cppmetricsparser.h +++ b/plugins/cpp_metrics/parser/include/cppmetricsparser/cppmetricsparser.h @@ -35,6 +35,8 @@ class CppMetricsParser : public AbstractParser void functionParameters(); // Calculate the McCabe complexity of functions. void functionMcCabe(); + // Calculate the bumpy road metric for every function. + void functionBumpyRoad(); // Calculate the lack of cohesion between member variables // and member functions for every type. void lackOfCohesion(); diff --git a/plugins/cpp_metrics/parser/src/cppmetricsparser.cpp b/plugins/cpp_metrics/parser/src/cppmetricsparser.cpp index 7e8269a4d..12ebd1cb7 100644 --- a/plugins/cpp_metrics/parser/src/cppmetricsparser.cpp +++ b/plugins/cpp_metrics/parser/src/cppmetricsparser.cpp @@ -141,6 +141,30 @@ void CppMetricsParser::functionMcCabe() }); } +void CppMetricsParser::functionBumpyRoad() +{ + util::OdbTransaction {_ctx.db} ([&, this] + { + for (const model::CppFunctionBumpyRoad& function + : _ctx.db->query()) + { + // Skip functions that were included from external libraries. + if (!cc::util::isRootedUnderAnyOf(_inputPaths, function.filePath)) + continue; + + const double dB = function.bumpiness; + const double dC = function.statementCount; + const bool empty = function.statementCount == 0; + + model::CppAstNodeMetrics metrics; + metrics.astNodeId = function.astNodeId; + metrics.type = model::CppAstNodeMetrics::Type::BUMPY_ROAD; + metrics.value = empty ? 1.0 : (dB / dC); + _ctx.db->persist(metrics); + } + }); +} + void CppMetricsParser::lackOfCohesion() { util::OdbTransaction {_ctx.db} ([&, this] @@ -250,6 +274,7 @@ bool CppMetricsParser::parse() { functionParameters(); functionMcCabe(); + functionBumpyRoad(); lackOfCohesion(); return true; } diff --git a/plugins/cpp_metrics/test/sources/parser/CMakeLists.txt b/plugins/cpp_metrics/test/sources/parser/CMakeLists.txt index 7a4633ff0..83409bc33 100644 --- a/plugins/cpp_metrics/test/sources/parser/CMakeLists.txt +++ b/plugins/cpp_metrics/test/sources/parser/CMakeLists.txt @@ -3,4 +3,5 @@ project(CppMetricsTestProject) add_library(CppMetricsTestProject STATIC mccabe.cpp - lackofcohesion.cpp) + lackofcohesion.cpp + bumpyroad.cpp) diff --git a/plugins/cpp_metrics/test/sources/parser/bumpyroad.cpp b/plugins/cpp_metrics/test/sources/parser/bumpyroad.cpp new file mode 100644 index 000000000..71bfa6f04 --- /dev/null +++ b/plugins/cpp_metrics/test/sources/parser/bumpyroad.cpp @@ -0,0 +1,276 @@ +#include + +using namespace std; + +// Flat + +void flat_empty_inline() {} + +void flat_empty() +{ + +} + +int flat_regular(int a, int b) +{ + int aa = a * a; + int ab2 = 2 * a * b; + int bb = b * b; + return aa + ab2 + bb; +} + +// Single bump + +void single_compound() +{ + { + int z = 0; + } +} + +bool single_if_simple(bool a) +{ + if (a) + a = false; + return a; +} + +int single_if_complex(int a) +{ + if (2ULL * a + static_cast(1.0) < 12) + a += 5 * (a << 3); + return a; +} + +int single_for_each(const vector& v) +{ + int s = 0; + for (int e : v) + s += e; + return s; +} + +int single_for_loop(const vector& v) +{ + int s = 0; + for (size_t i = 0; i < v.size(); ++i) + s += v[i]; + return s; +} + +// Nested chain + +void nested_chain_compound() +{ + { + { + { + int z = 0; + } + } + } +} + +void nested_chain_if() +{ + if (0 == 0) + if (true) + if (false) + flat_empty(); +} + +void nested_chain_compound_if() +{ + if (0 == 0) + { + if (true) + { + if (false) + { + flat_empty(); + } + } + } +} + +void nested_chain_for(const vector>>& array, int& sum) +{ + for (const auto& a0 : array) + for (const auto& a1 : a0) + for (const auto& a2 : a1) + sum += a2; +} + +void nested_chain_compound_for(const vector>>& array, int& sum) +{ + for (const auto& a0 : array) + { + for (const auto& a1 : a0) + { + for (const auto& a2 : a1) + { + sum += a2; + } + } + } +} + +int nested_chain_mixed(int n) +{ + switch (n) + { + default: + if (n < 0) + { + for (int i = 0; i < n; ++i) + { + try + { + while (false) + { + do + { + return 0; + } + while (true); + } + } + catch (...) {} + } + } + } + return 0; +} + +// Compare + +void compare_level1() +{ + if (true) + { + long a = 7; + long b = 3; + long c = a * b; + } +} + +void compare_level2() +{ + if (true) + { + if (true) + { + long a = 7; + long b = 3; + long c = a * b; + } + } +} + +void compare_level3() +{ + if (true) + { + if (true) + { + if (true) + { + long a = 7; + long b = 3; + long c = a * b; + } + } + } +} + +// Complex + +int complex_two_levels(int i, int j) +{ + int m = i + 2 * j, n = -j; + if (m + n < 42) + { + if (n < 0) + m *= 8; + } + else + { + if (m > 0) + n -= m * 2 - 1; + } + + int s = 0; + for (int k = m; k < n; ++k) + s += (k % 6 == 0) ? k : n; + + return s; +} + +int complex_three_levels_min(int a, int b) +{ + if (a > 0) + if (b > 0) + if (a + b < 100) + return a + b; + + a ^= b; + b ^= a; + a ^= b; + return 2 * a + b; +} + +int complex_three_levels_max(int a, int b) +{ + if (a > 0) + if (b > 0) + if (a + b < 100) + { + a ^= b; + b ^= a; + a ^= b; + return 2 * a + b; + } + + return a + b; +} + +// Nested lambda + +void nested_lambda() +{ + auto a = []() + { + auto b = []() + { + auto c = []() + { + int z = 0; + }; + }; + }; +} + +// Nested type + +void nested_type() +{ + struct nested1 + { + void method1() + { + struct nested2 + { + void method2() + { + struct nested3 + { + void method3() + { + int z = 0; + } + }; + } + }; + } + }; +} diff --git a/plugins/cpp_metrics/test/sources/parser/mccabe.cpp b/plugins/cpp_metrics/test/sources/parser/mccabe.cpp index 058c22039..5d78ff0f1 100644 --- a/plugins/cpp_metrics/test/sources/parser/mccabe.cpp +++ b/plugins/cpp_metrics/test/sources/parser/mccabe.cpp @@ -112,37 +112,75 @@ class MyClass MyClass(unsigned int arg1) // +1 { if (arg1 < LIMIT) // +1 - badPractice = new char[LIMIT]; + size = LIMIT; else - badPractice = new char[arg1]; + size = arg1; + badPractice = new char[size]; } // 2 ~MyClass() // +1 { - for (unsigned int i = 0; i < LIMIT; ++i) {} // +1 + for (unsigned int i = 0; i < size; ++i) {} // +1 delete[] badPractice; } // 2 - void method1(int arg1) // +1 + void method1(int arg1); // - + + operator unsigned int() const; // - + + explicit operator bool() const // +1 { - for (unsigned int i = arg1; i < LIMIT; ++i) // +1 - { - switch(arg1) - { - case -1: // +1 - goto endfor; // +1 - case 0: // +1 - break; - case 1: // +1 - break; - default: // +1 - continue; // +1 - } - arg1 *= 2; - } - endfor:; - } // 8 + return badPractice != nullptr; + } // 1 + private: - const unsigned int LIMIT = 42; + static constexpr unsigned int LIMIT = 42; + char* badPractice; + unsigned int size; +}; + +MyClass::operator unsigned int() const // +1 +{ + return size; +} // 1 + +void MyClass::method1(int arg1) // +1 +{ +for (unsigned int i = arg1; i < LIMIT; ++i) // +1 +{ + switch(arg1) + { + case -1: // +1 + goto endfor; // +1 + case 0: // +1 + break; + case 1: // +1 + break; + default: // +1 + continue; // +1 + } + arg1 *= 2; +} +endfor:; +} // 8 + +class NoBody1 +{ +public: + NoBody1() = default; // 1 }; + +class NoBody2 +{ +public: + NoBody2(const NoBody2& other) = delete; // 1 +}; + +class NoBody3 +{ +public: + virtual ~NoBody3(); // - +}; + +NoBody3::~NoBody3() = default; // 1 diff --git a/plugins/cpp_metrics/test/sources/service/CMakeLists.txt b/plugins/cpp_metrics/test/sources/service/CMakeLists.txt index 7a4633ff0..83409bc33 100644 --- a/plugins/cpp_metrics/test/sources/service/CMakeLists.txt +++ b/plugins/cpp_metrics/test/sources/service/CMakeLists.txt @@ -3,4 +3,5 @@ project(CppMetricsTestProject) add_library(CppMetricsTestProject STATIC mccabe.cpp - lackofcohesion.cpp) + lackofcohesion.cpp + bumpyroad.cpp) diff --git a/plugins/cpp_metrics/test/sources/service/bumpyroad.cpp b/plugins/cpp_metrics/test/sources/service/bumpyroad.cpp new file mode 100644 index 000000000..71bfa6f04 --- /dev/null +++ b/plugins/cpp_metrics/test/sources/service/bumpyroad.cpp @@ -0,0 +1,276 @@ +#include + +using namespace std; + +// Flat + +void flat_empty_inline() {} + +void flat_empty() +{ + +} + +int flat_regular(int a, int b) +{ + int aa = a * a; + int ab2 = 2 * a * b; + int bb = b * b; + return aa + ab2 + bb; +} + +// Single bump + +void single_compound() +{ + { + int z = 0; + } +} + +bool single_if_simple(bool a) +{ + if (a) + a = false; + return a; +} + +int single_if_complex(int a) +{ + if (2ULL * a + static_cast(1.0) < 12) + a += 5 * (a << 3); + return a; +} + +int single_for_each(const vector& v) +{ + int s = 0; + for (int e : v) + s += e; + return s; +} + +int single_for_loop(const vector& v) +{ + int s = 0; + for (size_t i = 0; i < v.size(); ++i) + s += v[i]; + return s; +} + +// Nested chain + +void nested_chain_compound() +{ + { + { + { + int z = 0; + } + } + } +} + +void nested_chain_if() +{ + if (0 == 0) + if (true) + if (false) + flat_empty(); +} + +void nested_chain_compound_if() +{ + if (0 == 0) + { + if (true) + { + if (false) + { + flat_empty(); + } + } + } +} + +void nested_chain_for(const vector>>& array, int& sum) +{ + for (const auto& a0 : array) + for (const auto& a1 : a0) + for (const auto& a2 : a1) + sum += a2; +} + +void nested_chain_compound_for(const vector>>& array, int& sum) +{ + for (const auto& a0 : array) + { + for (const auto& a1 : a0) + { + for (const auto& a2 : a1) + { + sum += a2; + } + } + } +} + +int nested_chain_mixed(int n) +{ + switch (n) + { + default: + if (n < 0) + { + for (int i = 0; i < n; ++i) + { + try + { + while (false) + { + do + { + return 0; + } + while (true); + } + } + catch (...) {} + } + } + } + return 0; +} + +// Compare + +void compare_level1() +{ + if (true) + { + long a = 7; + long b = 3; + long c = a * b; + } +} + +void compare_level2() +{ + if (true) + { + if (true) + { + long a = 7; + long b = 3; + long c = a * b; + } + } +} + +void compare_level3() +{ + if (true) + { + if (true) + { + if (true) + { + long a = 7; + long b = 3; + long c = a * b; + } + } + } +} + +// Complex + +int complex_two_levels(int i, int j) +{ + int m = i + 2 * j, n = -j; + if (m + n < 42) + { + if (n < 0) + m *= 8; + } + else + { + if (m > 0) + n -= m * 2 - 1; + } + + int s = 0; + for (int k = m; k < n; ++k) + s += (k % 6 == 0) ? k : n; + + return s; +} + +int complex_three_levels_min(int a, int b) +{ + if (a > 0) + if (b > 0) + if (a + b < 100) + return a + b; + + a ^= b; + b ^= a; + a ^= b; + return 2 * a + b; +} + +int complex_three_levels_max(int a, int b) +{ + if (a > 0) + if (b > 0) + if (a + b < 100) + { + a ^= b; + b ^= a; + a ^= b; + return 2 * a + b; + } + + return a + b; +} + +// Nested lambda + +void nested_lambda() +{ + auto a = []() + { + auto b = []() + { + auto c = []() + { + int z = 0; + }; + }; + }; +} + +// Nested type + +void nested_type() +{ + struct nested1 + { + void method1() + { + struct nested2 + { + void method2() + { + struct nested3 + { + void method3() + { + int z = 0; + } + }; + } + }; + } + }; +} diff --git a/plugins/cpp_metrics/test/sources/service/mccabe.cpp b/plugins/cpp_metrics/test/sources/service/mccabe.cpp index 058c22039..5d78ff0f1 100644 --- a/plugins/cpp_metrics/test/sources/service/mccabe.cpp +++ b/plugins/cpp_metrics/test/sources/service/mccabe.cpp @@ -112,37 +112,75 @@ class MyClass MyClass(unsigned int arg1) // +1 { if (arg1 < LIMIT) // +1 - badPractice = new char[LIMIT]; + size = LIMIT; else - badPractice = new char[arg1]; + size = arg1; + badPractice = new char[size]; } // 2 ~MyClass() // +1 { - for (unsigned int i = 0; i < LIMIT; ++i) {} // +1 + for (unsigned int i = 0; i < size; ++i) {} // +1 delete[] badPractice; } // 2 - void method1(int arg1) // +1 + void method1(int arg1); // - + + operator unsigned int() const; // - + + explicit operator bool() const // +1 { - for (unsigned int i = arg1; i < LIMIT; ++i) // +1 - { - switch(arg1) - { - case -1: // +1 - goto endfor; // +1 - case 0: // +1 - break; - case 1: // +1 - break; - default: // +1 - continue; // +1 - } - arg1 *= 2; - } - endfor:; - } // 8 + return badPractice != nullptr; + } // 1 + private: - const unsigned int LIMIT = 42; + static constexpr unsigned int LIMIT = 42; + char* badPractice; + unsigned int size; +}; + +MyClass::operator unsigned int() const // +1 +{ + return size; +} // 1 + +void MyClass::method1(int arg1) // +1 +{ +for (unsigned int i = arg1; i < LIMIT; ++i) // +1 +{ + switch(arg1) + { + case -1: // +1 + goto endfor; // +1 + case 0: // +1 + break; + case 1: // +1 + break; + default: // +1 + continue; // +1 + } + arg1 *= 2; +} +endfor:; +} // 8 + +class NoBody1 +{ +public: + NoBody1() = default; // 1 }; + +class NoBody2 +{ +public: + NoBody2(const NoBody2& other) = delete; // 1 +}; + +class NoBody3 +{ +public: + virtual ~NoBody3(); // - +}; + +NoBody3::~NoBody3() = default; // 1 diff --git a/plugins/cpp_metrics/test/src/cppmetricsparsertest.cpp b/plugins/cpp_metrics/test/src/cppmetricsparsertest.cpp index b299bea0f..e22f26d68 100644 --- a/plugins/cpp_metrics/test/src/cppmetricsparsertest.cpp +++ b/plugins/cpp_metrics/test/src/cppmetricsparsertest.cpp @@ -6,7 +6,7 @@ const auto v1 = val1; const auto v2 = val2; \ const bool n1 = isnan(v1); const bool n2 = isnan(v2); \ EXPECT_EQ(n1, n2); \ - if (!n1 && !n2) EXPECT_NEAR(v1, v2, abs_error); \ + if (!n1 && !n2) { EXPECT_NEAR(v1, v2, abs_error); } \ } #include @@ -79,13 +79,19 @@ std::vector paramMcCabe = { {"trycatch", 3}, {"MyClass::MyClass", 2}, {"MyClass::~MyClass", 2}, - {"MyClass::method1", 8} + {"MyClass::operator bool", 1}, + {"MyClass::operator unsigned int", 1}, + {"MyClass::method1", 8}, + {"NoBody1::NoBody1", 1}, + {"NoBody2::NoBody2", 1}, + {"NoBody3::~NoBody3", 1}, }; TEST_P(ParameterizedMcCabeTest, McCabeTest) { _transaction([&, this]() { + typedef odb::query QFun; model::CppFunction func = _db->query_value( - odb::query::qualifiedName == GetParam().first); + QFun::qualifiedName == GetParam().first && QFun::mccabe != 0); EXPECT_EQ(GetParam().second, func.mccabe); }); @@ -97,6 +103,72 @@ INSTANTIATE_TEST_SUITE_P( ::testing::ValuesIn(paramMcCabe) ); +// Bumpy Road + +struct BumpyRoadParam +{ + std::string funName; + unsigned int expBumpiness; + unsigned int expStmtCount; +}; + +class ParameterizedBumpyRoadTest + : public CppMetricsParserTest, + public ::testing::WithParamInterface +{}; + +#define BR_LAM "::(anonymous class)::operator()" +#define BR_NM1 "::nested1::method1" +#define BR_NM2 "::nested2::method2" +#define BR_NM3 "::nested3::method3" +std::vector paramBumpyRoad = { + {"flat_empty_inline", 0, 0}, + {"flat_empty", 0, 0}, + {"flat_regular", 4, 4}, + {"single_compound", 3, 2}, + {"single_if_simple", 4, 3}, + {"single_if_complex", 4, 3}, + {"single_for_each", 5, 4}, + {"single_for_loop", 5, 4}, + {"nested_chain_compound", 10, 4}, + {"nested_chain_if", 10, 4}, + {"nested_chain_compound_if", 10, 4}, + {"nested_chain_for", 10, 4}, + {"nested_chain_compound_for", 10, 4}, + {"nested_chain_mixed", 29, 8}, + {"compare_level1", 7, 4}, + {"compare_level2", 12, 5}, + {"compare_level3", 18, 6}, + {"complex_two_levels", 17, 10}, + {"complex_three_levels_min", 14, 8}, + {"complex_three_levels_max", 23, 8}, + {"nested_lambda()" BR_LAM "()" BR_LAM "()" BR_LAM, 1, 1}, + {"nested_lambda()" BR_LAM "()" BR_LAM, 3, 2}, + {"nested_lambda()" BR_LAM, 6, 3}, + {"nested_lambda", 10, 4}, + {"nested_type()" BR_NM1 "()" BR_NM2 "()" BR_NM3, 1, 1}, + {"nested_type()" BR_NM1 "()" BR_NM2, 3, 2}, + {"nested_type()" BR_NM1, 6, 3}, + {"nested_type", 10, 4}, +}; + +TEST_P(ParameterizedBumpyRoadTest, BumpyRoadTest) { + _transaction([&, this]() { + typedef odb::query QFun; + model::CppFunction func = _db->query_value( + QFun::qualifiedName == GetParam().funName); + + EXPECT_EQ(GetParam().expBumpiness, func.bumpiness); + EXPECT_EQ(GetParam().expStmtCount, func.statementCount); + }); +} + +INSTANTIATE_TEST_SUITE_P( + ParameterizedBumpyRoadTestSuite, + ParameterizedBumpyRoadTest, + ::testing::ValuesIn(paramBumpyRoad) +); + // Lack of Cohesion struct LackOfCohesionParam diff --git a/util/include/util/scopedvalue.h b/util/include/util/scopedvalue.h new file mode 100644 index 000000000..4f4aa8860 --- /dev/null +++ b/util/include/util/scopedvalue.h @@ -0,0 +1,53 @@ +#ifndef CC_UTIL_SCOPEDVALUE_H +#define CC_UTIL_SCOPEDVALUE_H + +#include + +#define DECLARE_SCOPED_TYPE(type) \ +private: \ + type(const type&) = delete; \ + type(type&&) = delete; \ + type& operator=(const type&) = delete; \ + type& operator=(type&&) = delete; \ + + +namespace cc +{ +namespace util +{ + + /** + * @brief Scoped value manager. + * + * Temporarily stores the given value in a variable upon construction, + * and restores its original value upon destruction. + * + * @tparam TValue The type of the variable and value to store. + */ + template + class ScopedValue final + { + DECLARE_SCOPED_TYPE(ScopedValue) + + private: + TValue& _storage; + TValue _oldValue; + + public: + ScopedValue(TValue& storage_, TValue&& newValue_) : + _storage(storage_), + _oldValue(std::move(_storage)) + { + _storage = std::forward(newValue_); + } + + ~ScopedValue() + { + _storage = std::move(_oldValue); + } + }; + +} // util +} // cc + +#endif // CC_UTIL_SCOPEDVALUE_H