Skip to content

fix(php): stop language constructs binding to same-named methods (#3830) - #3975

Open
Cintu07 wants to merge 1 commit into
Graphify-Labs:v8from
Cintu07:fix/php-language-construct-calls
Open

Cintu07 wants to merge 1 commit into
Graphify-Labs:v8from
Cintu07:fix/php-language-construct-calls

Conversation

@Cintu07

@Cintu07 Cintu07 commented Oct 1, 2026

Copy link
Copy Markdown
Contributor

what does this pr do?

fixes the builtin half of #3830. empty(), isset(), eval() and die() are language constructs, but tree-sitter-php parses them as a plain function_call_expression and the call pass resolves them by name. a reserved word is a legal method name since php 7, so any class with a method called empty picks up every empty($x) in the repo. same file the edge is EXTRACTED, other files get INFERRED through the case-insensitive fold. the symfony corpus in the issue had about 200 of these on one paginator method.

the php branch now drops the callee for these before the in-file lookup and before raw_calls, same thing _GO_PREDECLARED_FUNCS does for go. php only, bare calls only, and it takes the whole call-shaped keyword list from the php manual (array, die, empty, eval, exit, isset, list, unset) so it doesnt depend on which of them the pinned grammar gives its own node type. $bag->empty() is a member call and still resolves.

the member call half of #3830 is #3391. #2617 refuses methods for php function calls across files, this one is narrower and also covers the same-file EXTRACTED edges.

type of change

  • Bug fix
  • New feature
  • Documentation
  • Tests or CI
  • Refactor
  • Security fix

verification & invariants

a call to a php language construct never produces a calls edge. the method that shares the name, member calls into it, and calls nested inside the construct like empty($this->load()) are unchanged. the tests cover all of those plus EMPTY() in upper case and a genuine cross-file function call.

  • Read the CONTRIBUTING.md guide.
  • Reproduced the issue and identified the invariant.
  • Made the smallest fix necessary.
  • Added a regression test (if bug fix) or isolated boundary test.
  • Kept the PR description synchronized with the final implementation.
  • Documented any limitations / unsupported cases explicitly.

how was this tested?

windows 11 arm64, python 3.14, venv from uv sync --all-extras.

python -m pytest tests/test_php_language_construct_calls.py    7 passed, 3 of them fail on v8 without the fix
python -m pytest tests/<file> per file                          298/313 files clean. the 24 failing tests all fail the same way on unpatched v8 on this box (windows only: read-only replace, cp1252 locale, fifo/unix socket/symlink setup, deleting cwd, install paths). test_cli_export.py and test_hooks.py hang on subprocess startup here with and without the change
python -m ruff check .                                          all checks passed
python -m pyright graphify/extractors/engine.py                 34 errors, the same 34 as v8
python -m tools.skillgen --check                                ok
before
.send() --calls/INFERRED--> .empty()    Mailer.php L8
.add()  --calls/EXTRACTED--> .empty()   Bag.php L20
.add()  --calls/EXTRACTED--> .isset()   Bag.php L20
after
none of the above, .send() --calls--> .deliver() unchanged

graphify-specific checklist

  • I updated generated skill artifacts (uv run python -m tools.skillgen --bless) when changing their source fragments.
  • I confirmed that AST/structural extraction remains deterministic (no ambient state dependencies like ENV variables).
  • I reviewed changes for security implications (no unsafe interpolation into shell/Python).
  • I confirmed no API keys or local-only graph data are included.
  • (If applicable) I disclosed AI authorship in my commit messages.

…phify-Labs#3830)

empty(), isset(), eval() and die() parse as an ordinary
function_call_expression, so the call pass resolved them by bare name.
A reserved word has been a legal method name since PHP 7, so a class
that declares empty() collected every empty($x) in the corpus:
EXTRACTED from its sibling methods, INFERRED from every other file
through the case-insensitive fold.

Drop the callee for these constructs in the PHP branch, before the
in-file lookup and before raw_calls, the same way _GO_PREDECLARED_FUNCS
handles Go builtins. $bag->empty() is a member call and still resolves.
@Cintu07
Cintu07 requested a review from safishamsi as a code owner October 1, 2026 18:25
@github-actions

github-actions Bot commented Oct 1, 2026

Copy link
Copy Markdown

Thanks for the pull request, @Cintu07. A maintainer will review it soon.

Want to talk it through while it is in review? Come join us on our Discord server. For longer-form discussion there is also GitHub Discussions.

A couple of things that speed up review: make sure the test suite passes on Python 3.10 and 3.13, and that the change keeps extraction deterministic.

@graphify-labs graphify-labs Bot left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Graphify reviewed this change.

Looks safe to merge — no coupling regressions and no blocking issues, checked against the code graph (not a self-assessment).

Formal verification. PR-changed functions: 0/1 verified (0 proven, 0 may-equivalent, 0 distinguished) · 1 not verified (1 unsupported).

Not verified on this run: \_extract\_generic (unsupported).


Graphify review — findings

Stops PHP language constructs called like functions (empty($x), isset(), eval(), die(), and the rest of _PHP_LANGUAGE_CONSTRUCTS, matched case-insensitively) from resolving as calls. They no longer produce edges into a user method that happens to share the name, either in the same file or across files. Member calls like $bag->empty() still resolve to that method, and calls nested inside the construct's arguments are still picked up.

No blocking issues surfaced.

Analysis details — impact, health, verification

Impact & health

Graphify review

Impact — 739 functions depend on the 259 functions this change touches.

Health — this change adds coupling hotspots:

  • new: _extract_generic() — 18 callers, 29 callees
  • new: extract_js() — 87 callers, 4 callees
  • new: extract_xaml() — 19 callers, 17 callees
  • new: extract_objc() — 27 callers, 9 callees
  • new: extract_julia() — 19 callers, 7 callees
  • new: extract_cpp() — 31 callers, 3 callees
  • new: extract_vue() — 10 callers, 7 callees
  • new: walk() — 1 callers, 66 callees
  • …and 9 more — each is listed as a finding

Verification — 739 functions in the blast radius were not formally verified this run (proofs are advisory here).

Gate & verification

graphify gate

PASS — objectively clean (no health regressions, tests not run — proofs not run this pass (advisory)). Grounded, not self-assessed.

Advisory (not blocking):

  • verification_scope: 676 function(s) in the blast radius were not formally verified this run

Test selection

Test selection

26 of 313 test file(s) selected (8%) via static blast radius.

  • tests/test_astro_extraction.py — impact
  • tests/test_build.py — impact
  • tests/test_cjs_module_extension.py — impact
  • tests/test_cpp_nested_and_cli.py — impact
  • tests/test_dotnet.py — impact
  • tests/test_extract.py — impact
  • tests/test_extract_php_closures.py — impact
  • tests/test_import_extension_resolution.py — impact
  • tests/test_indirect_call_block_scoped_shadow.py — impact
  • tests/test_indirect_dispatch.py — impact
  • tests/test_indirect_dispatch_assign_return.py — impact
  • tests/test_indirect_dispatch_getattr.py — impact
  • tests/test_js_exported_scalar_bindings.py — impact
  • tests/test_languages.py — impact
  • tests/test_multilang.py — impact
  • tests/test_php_language_construct_calls.py — impact, changed-test
  • tests/test_python_underscore_resolution.py — impact
  • tests/test_rationale.py — impact
  • tests/test_ruby_resolution.py — impact
  • tests/test_scala_self_type.py — impact
  • tests/test_swift_computed_properties.py — impact
  • tests/test_swift_protocol_requirements.py — impact
  • tests/test_trailing_newline_not_a_syntax_error.py — impact
  • tests/test_ts_new_expression_calls.py — impact
  • tests/test_typescript_module_extensions.py — impact
  • tests/test_vue_extraction.py — impact

Selection is safe under the controlled-regression assumption; always-run tests + a periodic full run are the backstops. Advisory — it never changes the check verdict.

Formal verification

Could not verify: Could not verify \_extract\_generic.

The verifier did not have enough to check \_extract\_generic, so it is saying so rather than guessing. No false assurance is the whole point.

Guarantee: No guarantee either way, this is an honest abstention, not a pass.

Note: Reason: no capturable inputs from the test suite; property tier: parameter `config` is annotated `LanguageConfig` — outside the synthesizable primitive/collection set

· 17 more finding(s) on lines outside this diff (see the check run).

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant