Skip to content

Rework type level metrics calculations #802

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

barnabasdomozi
Copy link
Collaborator

In this PR, the CppTypeDependencyMetrics parsing and various metrics calculations have been completely reworked:

  • The old CppTypeDependencyMetrics table was renamed to CppTypeDependency and is now part of plugins/cpp/model.
  • We now calculate C++ type dependencies during the AST parsing, instead of during efferentTypeLevel.
  • efferentTypeLevel and afferentTypeLevel metrics calculations were reduced to a simple DISTINCT COUNT SQL query on the CppTypeDependency table. This significantly speeds up the metrics calculation process.
  • Test cases for efferentTypeLevel were missing, I implemented those as well
  • The test cases for afferentTypeLevel lacked separation, those test classes were put into namespace CC_CPP_AFFERENT_COUPLING_METRICS_TEST

@barnabasdomozi barnabasdomozi changed the title Rework CppTypeDependencyMetrics, efferentTypeLevel, afferentTypeLevel Rework type level metrics calculations Jul 10, 2025
@mcserep mcserep added Kind: Refactor 🔃 Plugin: C++ Issues related to the parsing and presentation of C++ projects. Plugin: Metrics Issues related to the code metrics plugin. labels Jul 14, 2025
@mcserep mcserep added this to Roadmap Jul 14, 2025
@mcserep mcserep added this to the Upcoming Release milestone Jul 14, 2025
@github-project-automation github-project-automation bot moved this to In progress in Roadmap Jul 14, 2025
@mcserep mcserep requested review from mcserep and Copilot July 14, 2025 16:52
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR reworks how C++ type-level dependency metrics are calculated by renaming the old metrics table, moving dependency collection into AST parsing, and simplifying both efferent and afferent metrics to single SQL DISTINCT COUNT queries, plus adding missing tests.

  • Rename CppTypeDependencyMetrics to CppTypeDependency and update ODB views
  • Persist type dependencies during AST traversal in ClangASTVisitor and adjust parser to use new views
  • Add parameterized tests for efferent coupling and update afferent coupling tests' namespace qualifiers

Reviewed Changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
plugins/cpp_metrics/test/src/cppmetricsparsertest.cpp Add new efferent coupling tests and qualify afferent coupling test entries with the new namespace
plugins/cpp_metrics/test/sources/parser/efferentcoupling.cpp Define test classes under CC_CPP_EFFERENT_COUPLING_METRICS_TEST namespace
plugins/cpp_metrics/test/sources/parser/afferentcoupling.cpp Wrap afferent coupling test sources in CC_CPP_AFFERENT_COUPLING_METRICS_TEST namespace
plugins/cpp_metrics/test/sources/parser/CMakeLists.txt Include efferentcoupling.cpp in the test project
plugins/cpp_metrics/parser/src/cppmetricsparser.cpp Replace manual dependency counting with SQL DISTINCT COUNT queries on CppTypeDependency views
plugins/cpp/parser/src/clangastvisitor.h Collect and persist CppTypeDependency records during AST traversal
plugins/cpp/model/include/model/cpptypedependency.h Introduce CppTypeDependency struct, ODB views for counting, and remove old metrics header
plugins/cpp/model/include/model/cppfunction.h Add recordHash to track the owning record of a function
plugins/cpp_metrics/model/CMakeLists.txt Update ODB sources: remove old metrics header, add new dependency header
plugins/cpp/model/CMakeLists.txt Add cpptypedependency.h to the model build
Comments suppressed due to low confidence (3)

plugins/cpp_metrics/parser/src/cppmetricsparser.cpp:497

  • After switching to CppTypeDependency_Distinct_D_Count for module-level efferent metrics, consider adding or updating unit tests to verify these module-level calculations.
          TypeDependencyResult types = _ctx.db->query_value<model::CppTypeDependency_Distinct_D_Count>(

plugins/cpp/model/include/model/cpptypedependency.h:17

  • [nitpick] The new CppTypeDependency struct lacks a descriptive comment explaining the purpose of entityHash and dependencyHash. Consider adding a brief doc comment summarizing its role in dependency tracking.
struct CppTypeDependency

plugins/cpp_metrics/parser/src/cppmetricsparser.cpp:423

  • [nitpick] The lambda capture list uses [&, this], which can be simplified to [this, &] or just [&] for clarity and consistency.
      util::OdbTransaction{_ctx.db}([&, this]

Comment on lines +42 to +49
#pragma db view \
object(CppTypeDependency) \
object(CppAstNode = EntityAstNode : CppTypeDependency::entityHash == EntityAstNode::entityHash \
&& EntityAstNode::astType == cc::model::CppAstNode::AstType::Definition) \
object(File = EntityFile : EntityAstNode::location.file == EntityFile::id) \
object(CppAstNode = DependencyAstNode : CppTypeDependency::dependencyHash == DependencyAstNode::entityHash \
&& DependencyAstNode::astType == cc::model::CppAstNode::AstType::Definition) \
object(File = DependencyFile : DependencyAstNode::location.file == DependencyFile::id)
Copy link
Preview

Copilot AI Jul 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nitpick] The ODB view definitions for CppTypeDependency are very repetitive across multiple structs. Consider extracting the common view clauses into a macro or helper to reduce duplication and simplify future updates.

Suggested change
#pragma db view \
object(CppTypeDependency) \
object(CppAstNode = EntityAstNode : CppTypeDependency::entityHash == EntityAstNode::entityHash \
&& EntityAstNode::astType == cc::model::CppAstNode::AstType::Definition) \
object(File = EntityFile : EntityAstNode::location.file == EntityFile::id) \
object(CppAstNode = DependencyAstNode : CppTypeDependency::dependencyHash == DependencyAstNode::entityHash \
&& DependencyAstNode::astType == cc::model::CppAstNode::AstType::Definition) \
object(File = DependencyFile : DependencyAstNode::location.file == DependencyFile::id)
#define COMMON_CPP_TYPE_DEPENDENCY_VIEW \
COMMON_CPP_TYPE_DEPENDENCY_VIEW

Copilot uses AI. Check for mistakes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Kind: Refactor 🔃 Plugin: C++ Issues related to the parsing and presentation of C++ projects. Plugin: Metrics Issues related to the code metrics plugin.
Projects
Status: In progress
Development

Successfully merging this pull request may close these issues.

2 participants