Skip to content
New issue

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

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

Already on GitHub? # to your account

Cleanup SymbolResolver and make it more extensible #2001

Merged
merged 7 commits into from
Jan 31, 2025

Conversation

oxisto
Copy link
Member

@oxisto oxisto commented Jan 28, 2025

This PR performs a clean up of the ScopeManager especially related to how we handle member resolution, which was still a left over from the "legacy" system.

This makes all the necessary handle functions open so that someone external to this library can extend and replace this pass, if needed. Furthermore, we make all functions we do NOT wish to be overriden private (mostly because we consider them part of a legacy API).

Furthermore, this PR adds more documentation to this pass.

Copy link

codecov bot commented Jan 28, 2025

Codecov Report

Attention: Patch coverage is 95.55556% with 2 lines in your changes missing coverage. Please review.

Project coverage is 78.05%. Comparing base (bc4d52b) to head (971884f).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
...n/de/fraunhofer/aisec/cpg/passes/SymbolResolver.kt 97.29% 0 Missing and 1 partial ⚠️
...raunhofer/aisec/cpg/passes/inference/PassHelper.kt 83.33% 0 Missing and 1 partial ⚠️
Additional details and impacted files
Files with missing lines Coverage Δ
...ofer/aisec/cpg/frontends/java/ExpressionHandler.kt 77.97% <100.00%> (+0.06%) ⬆️
...n/de/fraunhofer/aisec/cpg/passes/SymbolResolver.kt 87.71% <97.29%> (+0.57%) ⬆️
...raunhofer/aisec/cpg/passes/inference/PassHelper.kt 80.14% <83.33%> (-0.31%) ⬇️

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@oxisto oxisto changed the title Make the SymbolResolver more extensible Cleanup SymbolResolver and make it more extensible Jan 28, 2025
@oxisto oxisto marked this pull request as ready for review January 28, 2025 23:03
@oxisto oxisto force-pushed the make-symbol-resolver-open branch from e4c4265 to 2f95012 Compare January 29, 2025 18:09
Copy link
Collaborator

@konradweiss konradweiss left a comment

Choose a reason for hiding this comment

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

LGTM

@KuechA
Copy link
Contributor

KuechA commented Jan 31, 2025

LGTM

Wouldn't it then make sense to approve the PR?

@konradweiss
Copy link
Collaborator

konradweiss commented Jan 31, 2025

LGTM

Wouldn't it then make sense to approve the PR?

No, because it is also based on having other questions answered.

Copy link
Contributor

@maximiliankaul maximiliankaul left a comment

Choose a reason for hiding this comment

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

LGTM

oxisto and others added 7 commits January 31, 2025 15:28
This PR makes all the necessary handle functions `open` so that someone external to this library can extend and replace this pass, if needed. Furthermore, we make all functions we do NOT wish to be overriden `private` (mostly because we consider them part of a legacy API).

Furthermore, this PR adds more documentation to this pass.
@oxisto oxisto force-pushed the make-symbol-resolver-open branch from cf2ff65 to 971884f Compare January 31, 2025 14:28
@oxisto oxisto enabled auto-merge (squash) January 31, 2025 14:28
@oxisto oxisto merged commit 0132710 into main Jan 31, 2025
2 checks passed
@oxisto oxisto deleted the make-symbol-resolver-open branch January 31, 2025 14:36
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
4 participants