Lack of cohesion metrics (#688)

Author: dbukki

plugins/cpp/parser/src/clangastvisitor.h

@@ -1633,34 +1633,27 @@ class ClangASTVisitor : public clang::RecursiveASTVisitor<ClangASTVisitor>
     model::FileLoc fileLoc;
 
     if (start_.isInvalid() || end_.isInvalid())
-    {
       fileLoc.file = getFile(start_);
-      const std::string& type = fileLoc.file.load()->type;
-      if (type != model::File::DIRECTORY_TYPE && type != _cppSourceType)
-      {
-        fileLoc.file->type = _cppSourceType;
-        _ctx.srcMgr.updateFile(*fileLoc.file);
-      }
-      return fileLoc;
-    }
-
-    clang::SourceLocation realStart = start_;
-    clang::SourceLocation realEnd = end_;
+    else
+    {
+      clang::SourceLocation realStart = start_;
+      clang::SourceLocation realEnd = end_;
 
-    if (_clangSrcMgr.isMacroBodyExpansion(start_))
-      realStart = _clangSrcMgr.getExpansionLoc(start_);
-    if (_clangSrcMgr.isMacroArgExpansion(start_))
-      realStart = _clangSrcMgr.getSpellingLoc(start_);
+      if (_clangSrcMgr.isMacroBodyExpansion(start_))
+        realStart = _clangSrcMgr.getExpansionLoc(start_);
+      if (_clangSrcMgr.isMacroArgExpansion(start_))
+        realStart = _clangSrcMgr.getSpellingLoc(start_);
 
-    if (_clangSrcMgr.isMacroBodyExpansion(end_))
-      realEnd = _clangSrcMgr.getExpansionLoc(end_);
-    if (_clangSrcMgr.isMacroArgExpansion(end_))
-      realEnd = _clangSrcMgr.getSpellingLoc(end_);
+      if (_clangSrcMgr.isMacroBodyExpansion(end_))
+        realEnd = _clangSrcMgr.getExpansionLoc(end_);
+      if (_clangSrcMgr.isMacroArgExpansion(end_))
+        realEnd = _clangSrcMgr.getSpellingLoc(end_);
 
-    if (!_isImplicit)
-      _fileLocUtil.setRange(realStart, realEnd, fileLoc.range);
+      if (!_isImplicit)
+        _fileLocUtil.setRange(realStart, realEnd, fileLoc.range);
 
-    fileLoc.file = getFile(realStart);
+      fileLoc.file = getFile(realStart);
+    }
 
     const std::string& type = fileLoc.file.load()->type;
     if (type != model::File::DIRECTORY_TYPE && type != _cppSourceType)

plugins/cpp/test/src/cpparsertest.cpp

@@ -4,6 +4,7 @@ include_directories(
 
 set(ODB_SOURCES
   include/model/cppastnodemetrics.h
+  include/model/cppcohesionmetrics.h
   include/model/cppfilemetrics.h)
 
 generate_odb_files("${ODB_SOURCES}" "cpp")

plugins/cpp_metrics/model/CMakeLists.txt

@@ -2,6 +2,8 @@
 #define CC_MODEL_CPPASTNODEMETRICS_H
 
 #include <model/cppastnode.h>
+#include <model/cppentity.h>
+#include <model/cpprecord.h>
 
 namespace cc
 {
@@ -14,7 +16,9 @@ struct CppAstNodeMetrics
   enum Type
   {
     PARAMETER_COUNT,
-    MCCABE
+    MCCABE,
+    LACK_OF_COHESION,
+    LACK_OF_COHESION_HS,
   };
 
   #pragma db id auto
@@ -26,8 +30,18 @@ struct CppAstNodeMetrics
   #pragma db not_null
   Type type;
 
-  #pragma db not_null
-  unsigned value;
+  #pragma db null
+  double value;
+};
+
+#pragma db view \
+  object(CppRecord) \
+  object(CppAstNodeMetrics : \
+    CppRecord::astNodeId == CppAstNodeMetrics::astNodeId)
+struct CppRecordMetricsView
+{
+  #pragma db column(CppAstNodeMetrics::value)
+  double value;
 };
 
 } //model

plugins/cpp_metrics/model/include/model/cppastnodemetrics.h

@@ -0,0 +1,77 @@
+#ifndef CC_MODEL_CPPCOHESIONMETRICS_H
+#define CC_MODEL_CPPCOHESIONMETRICS_H
+
+#include <model/cppentity.h>
+#include <model/cpprecord.h>
+#include <model/cppastnode.h>
+
+namespace cc
+{
+namespace model
+{
+#pragma db view \
+  object(CppRecord) \
+  object(CppAstNode : CppRecord::astNodeId == CppAstNode::id) \
+  object(File : CppAstNode::location.file)
+struct CohesionCppRecordView
+{
+  #pragma db column(CppEntity::entityHash)
+  std::size_t entityHash;
+
+  #pragma db column(CppEntity::qualifiedName)
+  std::string qualifiedName;
+
+  #pragma db column(CppEntity::astNodeId)
+  CppAstNodeId astNodeId;
+  
+  #pragma db column(File::path)
+  std::string filePath;
+};
+
+#pragma db view \
+  object(CppMemberType) \
+  object(CppAstNode : CppMemberType::memberAstNode) \
+  query(CppMemberType::kind == cc::model::CppMemberType::Kind::Field && (?))
+struct CohesionCppFieldView
+{
+  #pragma db column(CppAstNode::entityHash)
+  std::size_t entityHash;
+};
+
+#pragma db view \
+  object(CppMemberType) \
+  object(CppAstNode : CppMemberType::memberAstNode) \
+  object(File : CppAstNode::location.file) \
+  query(CppMemberType::kind == cc::model::CppMemberType::Kind::Method && (?))
+struct CohesionCppMethodView
+{
+  typedef cc::model::Position::PosType PosType;
+
+  #pragma db column(CppAstNode::location.range.start.line)
+  PosType startLine;
+  #pragma db column(CppAstNode::location.range.start.column)
+  PosType startColumn;
+  #pragma db column(CppAstNode::location.range.end.line)
+  PosType endLine;
+  #pragma db column(CppAstNode::location.range.end.column)
+  PosType endColumn;
+
+  #pragma db column(File::path)
+  std::string filePath;
+};
+
+#pragma db view \
+  object(CppAstNode) \
+  object(File : CppAstNode::location.file) \
+  query((CppAstNode::astType == cc::model::CppAstNode::AstType::Read \
+      || CppAstNode::astType == cc::model::CppAstNode::AstType::Write) && (?))
+struct CohesionCppAstNodeView
+{
+  #pragma db column(CppAstNode::entityHash)
+  std::uint64_t entityHash;
+};
+
+} //model
+} //cc
+
+#endif //CC_MODEL_CPPCOHESIONMETRICS_H

plugins/cpp_metrics/model/include/model/cppcohesionmetrics.h

@@ -10,6 +10,9 @@
 #include <model/cppfunction.h>
 #include <model/cppfunction-odb.hxx>
 
+#include <model/cpprecord.h>
+#include <model/cpprecord-odb.hxx>
+
 #include <util/parserutil.h>
 #include <util/threadpool.h>
 
@@ -23,12 +26,18 @@ class CppMetricsParser : public AbstractParser
 public:
   CppMetricsParser(ParserContext& ctx_);
   virtual ~CppMetricsParser();
+
   virtual bool cleanupDatabase() override;
   virtual bool parse() override;
 
 private:
+  // Calculate the count of parameters for every function.
   void functionParameters();
+  // Calculate the McCabe complexity of functions.
   void functionMcCabe();
+  // Calculate the lack of cohesion between member variables
+  // and member functions for every type.
+  void lackOfCohesion();
 
   std::vector<std::string> _inputPaths;
   std::unordered_set<model::FileId> _fileIdCache;

plugins/cpp_metrics/parser/include/cppmetricsparser/cppmetricsparser.h

@@ -2,6 +2,8 @@
 
 #include <model/cppastnodemetrics.h>
 #include <model/cppastnodemetrics-odb.hxx>
+#include <model/cppcohesionmetrics.h>
+#include <model/cppcohesionmetrics-odb.hxx>
 #include <model/cppfilemetrics.h>
 #include <model/cppfilemetrics-odb.hxx>
 
@@ -12,6 +14,7 @@
 
 #include <util/filesystem.h>
 #include <util/logutil.h>
+#include <util/filesystem.h>
 #include <util/odbtransaction.h>
 
 #include <memory>
@@ -138,13 +141,116 @@ void CppMetricsParser::functionMcCabe()
   });
 }
 
+void CppMetricsParser::lackOfCohesion()
+{
+  util::OdbTransaction {_ctx.db} ([&, this]
+  {
+    // Simplify some type names for readability.
+    typedef std::uint64_t HashType;
+
+    typedef odb::query<model::CohesionCppFieldView>::query_columns QField;
+    const auto& QFieldTypeHash = QField::CppMemberType::typeHash;
+
+    typedef odb::query<model::CohesionCppMethodView>::query_columns QMethod;
+    const auto& QMethodTypeHash = QMethod::CppMemberType::typeHash;
+    
+    typedef odb::query<model::CohesionCppAstNodeView>::query_columns QNode;
+    const auto& QNodeFilePath = QNode::File::path;
+    const auto& QNodeRange = QNode::CppAstNode::location.range;
+
+    // Calculate the cohesion metric for all types.
+    for (const model::CohesionCppRecordView& type
+      : _ctx.db->query<model::CohesionCppRecordView>())
+    {
+      // Skip types that were included from external libraries.
+      if (!cc::util::isRootedUnderAnyOf(_inputPaths, type.filePath))
+        continue;
+
+      std::unordered_set<HashType> fieldHashes;
+      // Query all fields of the current type.
+      for (const model::CohesionCppFieldView& field
+        : _ctx.db->query<model::CohesionCppFieldView>(
+          QFieldTypeHash == type.entityHash
+        ))
+      {
+        // Record these fields for later use.
+        fieldHashes.insert(field.entityHash);
+      }
+      std::size_t fieldCount = fieldHashes.size();
+
+      std::size_t methodCount = 0;
+      std::size_t totalCohesion = 0;
+      // Query all methods of the current type.
+      for (const model::CohesionCppMethodView& method
+        : _ctx.db->query<model::CohesionCppMethodView>(
+          QMethodTypeHash == type.entityHash
+        ))
+      {
+        // Do not consider methods with no explicit bodies.
+        const model::Position start(method.startLine, method.startColumn);
+        const model::Position end(method.endLine, method.endColumn);
+        if (start < end)
+        {
+          std::unordered_set<HashType> usedFields;
+          
+          // Query all AST nodes that use a variable for reading or writing...
+          for (const model::CohesionCppAstNodeView& node
+            : _ctx.db->query<model::CohesionCppAstNodeView>(
+              // ... in the same file as the current method
+              (QNodeFilePath == method.filePath &&
+              // ... within the textual scope of the current method's body.
+              (QNodeRange.start.line >= start.line
+                || (QNodeRange.start.line == start.line
+                && QNodeRange.start.column >= start.column)) &&
+              (QNodeRange.end.line <= end.line
+                || (QNodeRange.end.line == end.line
+                && QNodeRange.end.column <= end.column)))
+            ))
+          {
+            // If this AST node is a reference to a field of the type...
+            if (fieldHashes.find(node.entityHash) != fieldHashes.end())
+            {
+              // ... then mark it as used by this method.
+              usedFields.insert(node.entityHash);
+            }
+          }
+          
+          ++methodCount;
+          totalCohesion += usedFields.size();
+        }
+      }
+
+      // Calculate and record metrics.
+      const double dF = fieldCount;
+      const double dM = methodCount;
+      const double dC = totalCohesion;
+      const bool trivial = fieldCount == 0 || methodCount == 0;
+      const bool singular = methodCount == 1;
+      
+      // Standard lack of cohesion (range: [0,1])
+      model::CppAstNodeMetrics lcm;
+      lcm.astNodeId = type.astNodeId;
+      lcm.type = model::CppAstNodeMetrics::Type::LACK_OF_COHESION;
+      lcm.value = trivial ? 0.0 :
+        (1.0 - dC / (dM * dF));
+      _ctx.db->persist(lcm);
+
+      // Henderson-Sellers variant (range: [0,2])
+      model::CppAstNodeMetrics lcm_hs;
+      lcm_hs.astNodeId = type.astNodeId;
+      lcm_hs.type = model::CppAstNodeMetrics::Type::LACK_OF_COHESION_HS;
+      lcm_hs.value = trivial ? 0.0 : singular ? NAN :
+        ((dM - dC / dF) / (dM - 1.0));
+      _ctx.db->persist(lcm_hs);
+    }
+  });
+}
+
 bool CppMetricsParser::parse()
 {
-  // Function parameter number metric.
   functionParameters();
-  // Function McCabe metric
   functionMcCabe();
-
+  lackOfCohesion();
   return true;
 }
 

plugins/cpp_metrics/parser/src/cppmetricsparser.cpp

@@ -2,7 +2,9 @@ include_directories(
         ${PLUGIN_DIR}/model/include
         ${PLUGIN_DIR}/service/include
         ${CMAKE_CURRENT_BINARY_DIR}/../service/gen-cpp
-        ${PROJECT_SOURCE_DIR}/model/include)
+        ${PROJECT_SOURCE_DIR}/model/include
+        ${PROJECT_SOURCE_DIR}/plugins/cpp_metrics/model/include
+        ${PROJECT_BINARY_DIR}/plugins/cpp_metrics/model/include)
 
 include_directories(SYSTEM
         ${THRIFT_LIBTHRIFT_INCLUDE_DIRS})
@@ -27,8 +29,9 @@ target_link_libraries(cppmetricsservicetest
         pthread)
 
 target_link_libraries(cppmetricsparsertest
-        util
+        cppmetricsmodel
         model
+        util
         cppmodel
         ${Boost_LIBRARIES}
         ${GTEST_BOTH_LIBRARIES}
@@ -53,6 +56,7 @@ else()
        --database \"${TEST_DB}\" \
        --name cppmetricsservicetest \
        --input ${CMAKE_CURRENT_BINARY_DIR}/build/compile_commands.json \
+       --input ${CMAKE_CURRENT_SOURCE_DIR}/sources/service \
        --workspace ${CMAKE_CURRENT_BINARY_DIR}/workdir/ \
        --force"
             "${TEST_DB}")
@@ -68,6 +72,7 @@ else()
        --database \"${TEST_DB}\" \
        --name cppparsertest \
        --input ${CMAKE_CURRENT_BINARY_DIR}/build/compile_commands.json \
+       --input ${CMAKE_CURRENT_SOURCE_DIR}/sources/parser \
        --workspace ${CMAKE_CURRENT_BINARY_DIR}/workdir/ \
        --force"
             "${TEST_DB}")

plugins/cpp_metrics/test/CMakeLists.txt

@@ -1,5 +1,6 @@
 cmake_minimum_required(VERSION 2.6)
-project(CppTestProject)
+project(CppMetricsTestProject)
 
-add_library(CppTestProject STATIC
-  mccabe.cpp)
+add_library(CppMetricsTestProject STATIC
+  mccabe.cpp
+  lackofcohesion.cpp)