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

Make visitor call concreat #653

Open
wants to merge 1 commit into
base: 3.x
Choose a base branch
from
Open

Make visitor call concreat #653

wants to merge 1 commit into from

Conversation

AJenbo
Copy link
Collaborator

@AJenbo AJenbo commented Feb 19, 2023

Instead of relying on a magic function make the analyzers call there specific visitor functions using instanceof in an overwrite

Type: refactoring
Breaking change: yes

This moves the responsibility of calling type specific visitors to the definer as suggested here: #644

The benefit is that the code can now be evaluated by static analysis since it does not make use of magic methods or run time generated callbacks.
To simplify things further I have also adjusted all vist*() methods to follow the same base signature so that they can all be used in the same way.

This did uncover some cases where ASTNode was assumed bug ASTArtifact given, leading to calls to undefined methods that could have resulted in a fatal error.

@AJenbo AJenbo added this to the 3.0.0 milestone Feb 19, 2023
@codecov-commenter
Copy link

codecov-commenter commented Feb 19, 2023

Codecov Report

Attention: Patch coverage is 89.33333% with 48 lines in your changes are missing coverage. Please review.

Project coverage is 88.16%. Comparing base (5dbc13c) to head (dc8e707).

Files Patch % Lines
src/main/php/PDepend/Report/Jdepend/Xml.php 0.00% 19 Missing ⚠️
src/main/php/PDepend/Report/Dependencies/Xml.php 62.50% 9 Missing ⚠️
src/main/php/PDepend/Report/Summary/Xml.php 80.00% 6 Missing ⚠️
...n/php/PDepend/Metrics/Analyzer/NodeLocAnalyzer.php 82.75% 5 Missing ⚠️
...hp/PDepend/Metrics/Analyzer/ClassLevelAnalyzer.php 88.46% 3 Missing ⚠️
...p/PDepend/Source/ASTVisitor/AbstractASTVisitor.php 93.02% 3 Missing ⚠️
...epend/Metrics/Analyzer/ClassDependencyAnalyzer.php 93.75% 1 Missing ⚠️
...hp/PDepend/Metrics/Analyzer/DependencyAnalyzer.php 94.44% 1 Missing ⚠️
...php/PDepend/Metrics/Analyzer/NodeCountAnalyzer.php 95.83% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##                3.x     #653      +/-   ##
============================================
+ Coverage     88.03%   88.16%   +0.12%     
- Complexity     3466     3582     +116     
============================================
  Files           149      149              
  Lines          9151     9452     +301     
============================================
+ Hits           8056     8333     +277     
- Misses         1095     1119      +24     

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

@AJenbo AJenbo changed the base branch from master to 3.x April 25, 2024 21:57
@AJenbo AJenbo force-pushed the visitors branch 4 times, most recently from ca942df to dc2dcee Compare April 27, 2024 09:31
@AJenbo AJenbo marked this pull request as ready for review April 27, 2024 09:32
@AJenbo AJenbo force-pushed the visitors branch 2 times, most recently from 3400441 to e19b240 Compare April 28, 2024 13:05
@AJenbo AJenbo force-pushed the visitors branch 12 times, most recently from 6521174 to d5460dd Compare May 5, 2024 17:49
Instead of relying on a magic function make the analyzers call there
specific visitor functions using instanceof in an overwrite
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

None yet

2 participants