Skip to content

fix(weaving): write all proxies of a source file into one cache file - #764

Open
lisachenko wants to merge 3 commits into
masterfrom
fix/760-combined-proxies-per-file
Open

lisachenko wants to merge 3 commits into
masterfrom
fix/760-combined-proxies-per-file

Conversation

@lisachenko

Copy link
Copy Markdown
Member

What and why

Fixes #760.

When one source file declares several woven class-likes, each proxy used to be written to the same cache path (derived from the source file), so the last one won: the earlier proxies were overwritten and their classes were "not found" at runtime.

Now all proxies of a file go into one proxy file, included once by the woven source:

  • WeavingTransformer::processSingleClass() returns a WovenProxy instead of writing the file; transform() collects them locally (transform() is re-entered mid-transform, so no property state) and writeProxies() writes them through saveProxyToCache() and appends one include_once.
  • Single proxy: output is byte-identical to master; include right after the class, no extra checks.
  • Several proxies: declare(strict_types=1); once, then one braced namespace block per proxy (each keeps its own ProxyImports aliases, short use names preserved). A proxy extending another proxy of the same file is emitted after it.
  • Include point (ProxyIncludePlanner): after the last woven class-like whose proxy is position-bound (its woven trait keeps trait uses, or it extends/implements an unwoven class-like of the file), otherwise after the first woven one.
  • Load-order gap → WeavingException with file, woven classes, include point and each offending line: code running before the include point that needs a woven class-like of the file (new X, X::m(), X::$p, X::C, extends/implements/uses). Not flagged: X::class, instanceof, types, function/closure/arrow-function bodies (LoadedClassNameCollector).
  • New @internal final classes: WovenProxy, ProxyIncludePlanner, LoadedClassNameCollector, Go\Proxy\ProxyFileGenerator. GeneratorInterface::getStmts() added (Class/Enum/TraitGenerator), ClassProxyGenerator::generateStmts(), GeneratedCodePrinter gets a bracedNamespaces option.
  • No FORMAT_VERSION bump (next release is a major anyway).

Repro (from #760)

Bug1\First Bug1\Second
master not found ✅
this PR ✅ [advice] first ✅ [advice] second

Performance (composer test:performance:transformation, base vs head alternating, 7 runs each, medians)

case base ratio head ratio head/base
rewriter, default rules 0.0958 0.0979 1.02 (noise)
rewriter, all rules 1.3456 1.3401 1.00
WeavingTransformer 0.2424 0.2319 0.96

Tests

  • Snapshots: multiple-classes/multiple-ns/php80-*attribute* woven files now have one include; new proxy snapshots for multiple-classes, multiple-ns, multiple-mixed (interface, unwoven class, enum, in-file parent, namespace {}) — compiled and loaded at runtime in vfs.
  • Byte identity for single class / enum / class with static init vs master output.
  • Parent-proxy ordering, static-init call stays in its namespace block, gap positive & negative cases.
  • Functional MultiClassFileTest + ProductionMultiClassFileTest (production instantiates the third class first → runtime class map path).

Known limitations / follow-ups

  • An unwoven same-file interface always makes a class position-bound (PHP would declare a parent-less interface at compile time), so the include may land later than strictly needed.
  • Weaving a class whose parent is declared in the same file still requires the parent to be locatable by composer (parser-reflection locator) — same as master; the fixture registers it in the classmap.

Checklist

  • The title follows Conventional Commits (type(scope): summary)
  • Tests cover the change
  • composer check passes (phpunit 3099 OK, phpstan level 10 clean, php-cs-fixer clean)
  • User-facing or breaking changes are in CHANGELOG.md (and the upgrade guide for breaking changes)

🤖 Generated with Claude Code

https://claude.ai/code/session_01KXjD75egSqDNujmNS5kr5c


Generated by Claude Code

A source file declaring several woven class-likes wrote every proxy to the
same cache path, so the last proxy overwrote the others and all classes but
the last one were never declared ("Class not found").

- processSingleClass() converts the class and returns a WovenProxy instead of
  writing; transform() collects them in a local list (it is re-entered
  mid-transform) and writeProxies() writes one proxy file and appends ONE
  include_once.
- ProxyFileGenerator: a single proxy stays byte-identical to the previous
  output; several proxies get `declare(strict_types=1);` once and one braced
  namespace block per proxy (`namespace {}` for global classes), each with its
  own imports, a proxy extending another proxy of the file after it.
  Class/Enum/TraitGenerator::getStmts(), ClassProxyGenerator::generateStmts(),
  GeneratedCodePrinter option `bracedNamespaces`.
- ProxyIncludePlanner: the include goes after the last woven class-like whose
  woven trait keeps trait uses (declared at runtime) or that extends/implements
  an unwoven class-like of the file, else after the first one (trait-free
  traits are declared at compile time). Code before that point that needs a
  woven class-like (a class-like extending/implementing/using one, a top-level
  statement loading an earlier one) is rejected with a WeavingException naming
  the file, the classes and the lines.
- The runtime class map already mapped every class of the file to its woven
  file, CachePathManager is unchanged; no cache format version bump.

Transformation performance group, base vs head interleaved, 7 runs each,
median ratios: rewriter default rules 0.0958 -> 0.0979 (1.02), all rules
1.3456 -> 1.3401 (1.00), WeavingTransformer 0.2424 -> 0.2319 (0.96).

Closes #760

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KXjD75egSqDNujmNS5kr5c
@codecov

codecov Bot commented Oct 9, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 97.74%. Comparing base (4e95fdd) to head (cd84761).

Additional details and impacted files
@@             Coverage Diff              @@
##             master     #764      +/-   ##
============================================
+ Coverage     97.63%   97.74%   +0.10%     
- Complexity     1829     1918      +89     
============================================
  Files           104      108       +4     
  Lines          4829     5012     +183     
============================================
+ Hits           4715     4899     +184     
+ Misses          114      113       -1     

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

claude added 2 commits October 9, 2026 22:43
… analysis

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KXjD75egSqDNujmNS5kr5c
- Cover a file without namespace declaration (one `namespace {}` block per
  proxy, loaded at runtime), two woven traits sharing a proxy file (child
  trait proxies via TraitProxyGenerator::generateStmts(), loaded at runtime)
  and a class declared by a top-level statement in the gap region.
- ProxyIncludePlanner: drop the unexecuted assert and the unreachable
  top-level statement branch (parser-reflection wraps files without namespace
  into a global namespace block). WeavingTransformer reads the end of a woven
  declaration through getDeclarationTokenRange(), which rejects nodes without
  token positions like the rest of the weaver, instead of a separate branch.
- Platform-neutral tests for Windows: separator-neutral path assertion,
  byte-exact proxy snapshots with a PHP_EOL header, and the attribute test
  reads the include from the raw woven source.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KXjD75egSqDNujmNS5kr5c
@lisachenko
lisachenko marked this pull request as ready for review October 9, 2026 22:52
@lisachenko

Copy link
Copy Markdown
Member Author

Very heavy implementation - too complex. Drop this and rethink with minimum changes - do not introduce a lot of new classes…

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

Development

Successfully merging this pull request may close these issues.

Several woven classes in one file: their proxies overwrite each other, all but the last class are lost (Class not found)

2 participants