Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Lack of cohesion metrics #688

Merged
merged 30 commits into from
Feb 5, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
1bc9060
Initial implementation of the lack of cohesion metric.
dbukki Nov 25, 2023
7700e16
Added manual ODB lazy pointer loading to avoid segfaults.
dbukki Nov 26, 2023
3eb47f6
Replaced the lazy pointer loading with db views.
dbukki Nov 26, 2023
d015b22
Further refactor of queries via db views.
dbukki Nov 26, 2023
e08e522
Removed unneeded fields from the db views.
dbukki Nov 26, 2023
7121d53
Added checks to ensure that the cohesion metrics are only calculated …
dbukki Dec 3, 2023
25ea687
Added debug utilities to the cohesion metrics.
dbukki Dec 3, 2023
66390e3
Cohesion database views have been moved to their own new header (in t…
dbukki Dec 3, 2023
4433df3
Minor interface refactors + code comments.
dbukki Dec 3, 2023
b0dcc41
Replaced std::cout with LOG(debug).
dbukki Dec 9, 2023
adf2c44
isInInputPath is generalized and moved to cc::util::isRootedUnderAnyOf.
dbukki Dec 11, 2023
24ead2d
Initial implementation of the lack of cohesion metric.
dbukki Nov 25, 2023
bb2b46c
Added manual ODB lazy pointer loading to avoid segfaults.
dbukki Nov 26, 2023
87d8f58
Replaced the lazy pointer loading with db views.
dbukki Nov 26, 2023
9ee55af
Further refactor of queries via db views.
dbukki Nov 26, 2023
bab58a9
Removed unneeded fields from the db views.
dbukki Nov 26, 2023
a6747a0
Added checks to ensure that the cohesion metrics are only calculated …
dbukki Dec 3, 2023
31aaa8e
Added debug utilities to the cohesion metrics.
dbukki Dec 3, 2023
9ee52a9
Cohesion database views have been moved to their own new header (in t…
dbukki Dec 3, 2023
e902fb2
Minor interface refactors + code comments.
dbukki Dec 3, 2023
14971b7
Replaced std::cout with LOG(debug).
dbukki Dec 9, 2023
2fc2782
isInInputPath is generalized and moved to cc::util::isRootedUnderAnyOf.
dbukki Dec 11, 2023
656dca0
Rebased branch onto master.
dbukki Jan 27, 2024
b0f7011
Conflict during pull during rebase.
dbukki Jan 27, 2024
8d757ae
Changed the metrics value type to double in the database.
dbukki Jan 27, 2024
9125ad0
Unit tests for LoC metrics.
dbukki Jan 27, 2024
7caf4d7
Fixed missing math.h include. Query success is now also checked in te…
dbukki Jan 28, 2024
4f43dd7
Add project path as input path during parsing for C++ metrics test.
mcserep Feb 4, 2024
33b319b
Merge branch 'master' into loc
mcserep Feb 4, 2024
aa2d22b
Deleted trace macro.
intjftw Feb 4, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 16 additions & 23 deletions plugins/cpp/parser/src/clangastvisitor.h
Original file line number Diff line number Diff line change
Expand Up @@ -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_;
mcserep marked this conversation as resolved.
Show resolved Hide resolved
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)
Expand Down
Empty file.
1 change: 1 addition & 0 deletions plugins/cpp_metrics/model/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
20 changes: 17 additions & 3 deletions plugins/cpp_metrics/model/include/model/cppastnodemetrics.h
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@
#define CC_MODEL_CPPASTNODEMETRICS_H

#include <model/cppastnode.h>
#include <model/cppentity.h>
#include <model/cpprecord.h>

namespace cc
{
Expand All @@ -14,7 +16,9 @@ struct CppAstNodeMetrics
enum Type
{
PARAMETER_COUNT,
MCCABE
MCCABE,
LACK_OF_COHESION,
LACK_OF_COHESION_HS,
};

#pragma db id auto
Expand All @@ -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
Expand Down
77 changes: 77 additions & 0 deletions plugins/cpp_metrics/model/include/model/cppcohesionmetrics.h
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
Expand Up @@ -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>

Expand All @@ -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;
Expand Down
112 changes: 109 additions & 3 deletions plugins/cpp_metrics/parser/src/cppmetricsparser.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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>

Expand All @@ -12,6 +14,7 @@

#include <util/filesystem.h>
#include <util/logutil.h>
#include <util/filesystem.h>
#include <util/odbtransaction.h>

#include <memory>
Expand Down Expand Up @@ -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;
}

Expand Down
9 changes: 7 additions & 2 deletions plugins/cpp_metrics/test/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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})
Expand All @@ -27,8 +29,9 @@ target_link_libraries(cppmetricsservicetest
pthread)

target_link_libraries(cppmetricsparsertest
util
cppmetricsmodel
model
util
cppmodel
${Boost_LIBRARIES}
${GTEST_BOTH_LIBRARIES}
Expand All @@ -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}")
Expand All @@ -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}")
Expand Down
7 changes: 4 additions & 3 deletions plugins/cpp_metrics/test/sources/parser/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -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)
Loading
Loading