Skip to content

Fix PHP CALL extraction: add support for method, static, and unqualified function calls #297#298

Merged
tirth8205 merged 1 commit intotirth8205:mainfrom
Bakul2006:PHP_Call
Apr 18, 2026
Merged

Fix PHP CALL extraction: add support for method, static, and unqualified function calls #297#298
tirth8205 merged 1 commit intotirth8205:mainfrom
Bakul2006:PHP_Call

Conversation

@Bakul2006
Copy link
Copy Markdown
Contributor

@Bakul2006 Bakul2006 commented Apr 15, 2026

Root cause: PHP node child-type mismatch in _get_call_name.
Fix: PHP-specific handling + scoped/nullsafe node coverage + regression tests.
Addressing the issue #297 and #273

Changes
Expanded PHP call node coverage in parser call node configuration.
Added PHP-specific call-name extraction logic

@kojiromike
Copy link
Copy Markdown

kojiromike commented Apr 16, 2026 •
edited
Loading

Tested this PR locally against OpenEMR (4,770 files, a large PHP codebase). Results are dramatic:

Metric main PR #298
PHP CALLS edges 58 145,423
Total graph edges 156,005 304,632

Spot-checking the top targets shows real PHP patterns now resolve correctly — method calls (addChild, text, get, saveXML), static calls (OEGlobalsBag::getInstance), framework/test calls (assertEquals, assertSame), and OpenEMR-specific globals (sqlQuery, sqlStatement, xl, xlt).

Sample blast-radius query on sqlStatement now returns real callers with correct file paths and line numbers — e.g. C_Document.view_action at multiple sites, C_PatientFinder.search_by_FullName, etc. Pre-PR this returned nothing.

The scoped_call_expression handling that constructs Class::method targets is especially valuable — it's what makes OEGlobalsBag::getInstance (and many other singletons/factories across the codebase) participate in caller graphs. Also good catch on nullsafe_member_call_expression ($obj?->method()) — that's increasingly common in modern PHP.

One small observation: the _normalize_php_name helper is currently defined as a closure inside _get_call_name. If this pattern repeats for other languages (e.g. Perl/Ruby also have sigil-prefixed names in some grammars), it might be worth promoting to a module-level helper. Just a thought for a follow-up.

LGTM from an end-user perspective. Thanks for the fast turnaround.

@tirth8205 tirth8205 merged commit d660e02 into tirth8205:main Apr 18, 2026
1 check passed
This was referenced Apr 18, 2026
Closed
Closed
@gzenz gzenz mentioned this pull request Apr 18, 2026
Closed
3 tasks
Minidoracat added a commit to Minidoracat/code-review-graph that referenced this pull request Apr 18, 2026
@Minidoracat
fix(tests): reconcile with upstream tirth8205#298 PHP CALL format
6f0b869 
After rebase onto upstream/main:

- `test_finds_static_calls`: upstream tirth8205#298 keeps `::` as the static-call
  separator instead of normalizing to `.`, so assert on `User::find`.
- Add `test_finds_calls_comprehensive` covering plain/member/nullsafe/
  scoped/global-namespaced extraction (the test upstream tirth8205#298 introduced
  in TestPHPParsing — rebase placed it inside TestBladeParsing by
  accident, where `sqlQuery`/`xl`/`text` aren't present in the Blade
  fixture).
- Remove unused `sources` local in `test_finds_inheritance` (F841).
@Minidoracat Minidoracat mentioned this pull request Apr 18, 2026
Open
7 tasks
Minidoracat added a commit to Minidoracat/code-review-graph that referenced this pull request Apr 18, 2026
@Minidoracat
fix(tests): reconcile with upstream tirth8205#298 PHP CALL format
42bdebe 
After rebase onto upstream/main:

- `test_finds_static_calls`: upstream tirth8205#298 keeps `::` as the static-call
  separator instead of normalizing to `.`, so assert on `User::find`.
- Add `test_finds_calls_comprehensive` covering plain/member/nullsafe/
  scoped/global-namespaced extraction (the test upstream tirth8205#298 introduced
  in TestPHPParsing — rebase placed it inside TestBladeParsing by
  accident, where `sqlQuery`/`xl`/`text` aren't present in the Blade
  fixture).
- Remove unused `sources` local in `test_finds_inheritance` (F841).
Minidoracat added a commit to Minidoracat/code-review-graph that referenced this pull request Apr 18, 2026
@Minidoracat
refactor(parser): remove shadowed PHP branch in _get_call_name
f7b3687 
Codex review flagged two PHP blocks in `_get_call_name()` after the
rebase: upstream tirth8205#298's block (function/member/nullsafe/scoped) runs
first and returns, making the later feat-branch block unreachable for
those node types.  The two blocks also disagreed on scoped-call
formatting (`::` vs `.`), which is exactly the sort of latent rebase
hazard that would bite the next editor.

- Merge the only live arm (`object_creation_expression`) into the
  upstream block with consistent `_normalize_php_name` handling.
- Delete the shadowed/duplicated PHP block entirely.
Minidoracat added a commit to Minidoracat/code-review-graph that referenced this pull request Apr 18, 2026
@Minidoracat
fix(tests): reconcile with upstream tirth8205#298 PHP CALL format
3093028 
After rebase onto upstream/main:

- `test_finds_static_calls`: upstream tirth8205#298 keeps `::` as the static-call
  separator instead of normalizing to `.`, so assert on `User::find`.
- Add `test_finds_calls_comprehensive` covering plain/member/nullsafe/
  scoped/global-namespaced extraction (the test upstream tirth8205#298 introduced
  in TestPHPParsing — rebase placed it inside TestBladeParsing by
  accident, where `sqlQuery`/`xl`/`text` aren't present in the Blade
  fixture).
- Remove unused `sources` local in `test_finds_inheritance` (F841).
Minidoracat added a commit to Minidoracat/code-review-graph that referenced this pull request Apr 18, 2026
@Minidoracat
refactor(parser): remove shadowed PHP branch in _get_call_name
f43c297 
Codex review flagged two PHP blocks in `_get_call_name()` after the
rebase: upstream tirth8205#298's block (function/member/nullsafe/scoped) runs
first and returns, making the later feat-branch block unreachable for
those node types.  The two blocks also disagreed on scoped-call
formatting (`::` vs `.`), which is exactly the sort of latent rebase
hazard that would bite the next editor.

- Merge the only live arm (`object_creation_expression`) into the
  upstream block with consistent `_normalize_php_name` handling.
- Delete the shadowed/duplicated PHP block entirely.
Minidoracat added a commit to Minidoracat/code-review-graph that referenced this pull request Apr 18, 2026
@Minidoracat
fix(tests): reconcile with upstream tirth8205#298 PHP CALL format
13dc2a1 
After rebase onto upstream/main:

- `test_finds_static_calls`: upstream tirth8205#298 keeps `::` as the static-call
  separator instead of normalizing to `.`, so assert on `User::find`.
- Add `test_finds_calls_comprehensive` covering plain/member/nullsafe/
  scoped/global-namespaced extraction (the test upstream tirth8205#298 introduced
  in TestPHPParsing — rebase placed it inside TestBladeParsing by
  accident, where `sqlQuery`/`xl`/`text` aren't present in the Blade
  fixture).
- Remove unused `sources` local in `test_finds_inheritance` (F841).
Minidoracat added a commit to Minidoracat/code-review-graph that referenced this pull request Apr 18, 2026
@Minidoracat
refactor(parser): remove shadowed PHP branch in _get_call_name
0967b7c 
Codex review flagged two PHP blocks in `_get_call_name()` after the
rebase: upstream tirth8205#298's block (function/member/nullsafe/scoped) runs
first and returns, making the later feat-branch block unreachable for
those node types.  The two blocks also disagreed on scoped-call
formatting (`::` vs `.`), which is exactly the sort of latent rebase
hazard that would bite the next editor.

- Merge the only live arm (`object_creation_expression`) into the
  upstream block with consistent `_normalize_php_name` handling.
- Delete the shadowed/duplicated PHP block entirely.
Minidoracat added a commit to Minidoracat/code-review-graph that referenced this pull request Apr 18, 2026
@Minidoracat
fix(tests): reconcile with upstream tirth8205#298 PHP CALL format
c6cdb50 
After rebase onto upstream/main:

- `test_finds_static_calls`: upstream tirth8205#298 keeps `::` as the static-call
  separator instead of normalizing to `.`, so assert on `User::find`.
- Add `test_finds_calls_comprehensive` covering plain/member/nullsafe/
  scoped/global-namespaced extraction (the test upstream tirth8205#298 introduced
  in TestPHPParsing — rebase placed it inside TestBladeParsing by
  accident, where `sqlQuery`/`xl`/`text` aren't present in the Blade
  fixture).
- Remove unused `sources` local in `test_finds_inheritance` (F841).
Minidoracat added a commit to Minidoracat/code-review-graph that referenced this pull request Apr 18, 2026
@Minidoracat
refactor(parser): remove shadowed PHP branch in _get_call_name
83249e5 
Codex review flagged two PHP blocks in `_get_call_name()` after the
rebase: upstream tirth8205#298's block (function/member/nullsafe/scoped) runs
first and returns, making the later feat-branch block unreachable for
those node types.  The two blocks also disagreed on scoped-call
formatting (`::` vs `.`), which is exactly the sort of latent rebase
hazard that would bite the next editor.

- Merge the only live arm (`object_creation_expression`) into the
  upstream block with consistent `_normalize_php_name` handling.
- Delete the shadowed/duplicated PHP block entirely.
This was referenced Apr 21, 2026
Closed
Closed
Open
Open
npkriami18 pushed a commit to npkriami18/code-review-graph that referenced this pull request Apr 21, 2026
@Bakul2006 @npkriami18
fix: PHP CALL extraction — method, static, and unqualified function c…
1e05d73 
…alls (tirth8205#298)

(cherry picked from commit d660e02)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@Bakul2006 @kojiromike @tirth8205