fix: resolve PHP use imports to file paths#573
Open
stefanws0 wants to merge 1 commit into
Open
Conversation
PHP `use` statements had no branch in `_extract_import`, so they fell
through to the raw-text fallback and stored the entire statement
("use App\Domain\Entity\Job;") as the IMPORTS_FROM edge target. As a
result `importers_of`, `tests_for`, `inheritors_of`, and the upstream
side of `get_impact_radius` returned nothing for PHP classes, and the
unresolved targets also degraded cross-file CALLS disambiguation in
`resolve_bare_call_targets`.
Add a PHP branch to `_extract_import` that records the fully-qualified
name of each imported symbol, handling `as` aliases, grouped
`use A\{B, C}`, `use function`/`use const`, and a leading `\`. Add a PHP
branch to `_do_resolve_module` that maps the FQN to an absolute `.php`
path by walking up from the importing file, mirroring the existing Java
resolver. Vendor/global classes with no local file stay as the bare FQN.
Adds TestPHPImportResolution covering project-import resolution, the
vendor/unresolved fallback, alias-records-FQN, and grouped-use expansion.
1db8eaf to
e5df619
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Linked issue
Closes #574.
Found while using the graph on a Symfony/PHP codebase:
importers_of(and theupstream side of impact-radius) returned 0 for PHP classes that are imported
by hundreds of files.
What & why
PHP
usestatements had no branch in the parser's_extract_import, so theyfell through to the raw-text fallback (
imports.append(text)) and stored theentire statement as the
IMPORTS_FROMedge target. For example,"use App\Domain\Entity\Job;"was stored instead of the FQNApp\Domain\Entity\Job.Consequences:
importers_of,tests_for, andinheritors_ofreturned nothing for PHPclasses, because the query matches edge targets by resolved file path, which
the raw
use ...;text never matches.get_impact_radiussaw no PHP importers.resolve_bare_call_targetsusesIMPORTS_FROMtargets to disambiguatecross-file
CALLS, so the unparsed targets silently degraded PHP callresolution too.
Every other FQN-style language already has an import branch, and Java
additionally resolves
import a.b.C;to the class's.javafile in_do_resolve_module. This PR brings PHP to parity:_extract_import: newphpbranch recording the fully-qualified name ofeach imported symbol, handling
asaliases (records the FQN, not the alias),grouped
use A\B\{C, D as E}(prefix-joined),use function/use const,and a leading
\._do_resolve_module: newphpbranch mirroring Java. It converts\to/, appends.php, and walks up from the importing file to the source root.Vendor/global classes with no local file (such as
\Exception) stay as thebare FQN, exactly like JDK imports.
No new dependencies, no schema changes, and no post-build resolver. The fix
lives entirely in the existing per-language parser paths, consistent with Java.
Impact
In the codebase where this surfaced,
importers_offor a core entity returned0 despite 452 files doing
use App\Domain\Entity\Job;(confirmed viagrep). With the fix those
usestatements resolve to the entity's file. Thebehavior is covered by a self-contained test (
TestPHPImportResolution) on amini PSR-4 project.
How it was tested
Also verified end to end by building a graph for a mini PSR-4 project with the
patched code and confirming
query_graph("importers_of", <Job.php>)returns thetwo importing files and excludes a non-importing file. It returned nothing
before the change.
Checklist
uv run pytest tests/ --tb=short -quv run ruff check code_review_graph/uv run mypy code_review_graph/ --ignore-missing-imports --no-strict-optional