Skip to content

fix(weaving): copy the imports of the namespace block declaring the woven class - #758

Merged
lisachenko merged 1 commit into
masterfrom
fix/duplicate-namespace-imports
Oct 8, 2026
Merged

lisachenko merged 1 commit into
masterfrom
fix/duplicate-namespace-imports

Conversation

@lisachenko

Copy link
Copy Markdown
Member

What and why

This bug fix comes from the declined #757 (#749). It has nothing to do with the AST cache limit.

The bug: a file may declare the same namespace in several blocks, each with its own imports:

namespace Test\ns1 {
    use Go\Stubs\Collision\Interceptor as Level;
    class FirstBlockClass {}
}

namespace Test\ns1 {
    use Go\Stubs\Collision\The as Level;
    class SecondBlockClass {
        public function log(string $level = Level::LEVEL): string { return $level; }
    }
}

WeavingTransformer::processSingleClass() looked the imports up again with new ReflectionFileNamespace($file, $namespaceName), which returns the first block of that name. The proxy of SecondBlockClass therefore got use Go\Stubs\Collision\Interceptor as Level;, and the copied parameter default Level::LEVEL referred to the wrong class.

The fix: processSingleClass() now receives the ReflectionFileNamespace being woven from transform() and reads the imports from it. As a side effect, this removes the second namespace lookup per woven class, along with its getFileName() guard, which nothing else in the method used.

Tests

  • New WeavingTransformerTest::testProxyCopiesImportsOfTheNamespaceBlockDeclaringTheClass with fixture _files/duplicate-ns.php. It asserts that the proxy imports The as Level, contains no Interceptor import, keeps the Level::LEVEL default, and compiles.
  • Checked both ways: the test fails with master's src/ and passes with the fix.
  • Gates, run locally:
    • full phpunit (3089 tests), phpstan level 10 and php-cs-fixer are green;
    • the transformation performance group is within budget, and the weaving ratio of 0.25 is unchanged from master.

Checklist

  • The title follows Conventional Commits (type(scope): summary)
  • Tests cover the change
  • composer check passes
  • 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

…oven class

A file may declare the same namespace in several blocks, each with its
own imports. WeavingTransformer::processSingleClass() looked the imports
up again with `new ReflectionFileNamespace($file, $namespaceName)`, which
returns the first block of that name, so the proxy of a class declared
in a later block got the imports of the first block: a parameter default
like `Level::LEVEL` then referred to the wrong class.

The imports are now read from the ReflectionFileNamespace being woven,
passed down from transform(). This also drops the second namespace
lookup per woven class.

Extracted from the declined #757 (#749).

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

codecov Bot commented Oct 8, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 97.63%. Comparing base (79ec2a5) to head (8d26964).

Additional details and impacted files
@@             Coverage Diff              @@
##             master     #758      +/-   ##
============================================
+ Coverage     97.61%   97.63%   +0.01%     
+ Complexity     1830     1829       -1     
============================================
  Files           104      104              
  Lines          4831     4829       -2     
============================================
- Hits           4716     4715       -1     
+ Misses          115      114       -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.

@lisachenko
lisachenko marked this pull request as ready for review October 8, 2026 16:52
@lisachenko
lisachenko merged commit 4e95fdd into master Oct 8, 2026
15 checks passed
@lisachenko
lisachenko deleted the fix/duplicate-namespace-imports branch October 8, 2026 20:53
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.

2 participants