Skip to content

[CodeQuality] Skip NarrowUnusedSetUpDefinedPropertyRector for property used in nested closure#693

Merged
TomasVotruba merged 2 commits into
mainfrom
fix-narrow-setup-property-used-in-closure
Jun 29, 2026
Merged

[CodeQuality] Skip NarrowUnusedSetUpDefinedPropertyRector for property used in nested closure#693
TomasVotruba merged 2 commits into
mainfrom
fix-narrow-setup-property-used-in-closure

Conversation

@TomasVotruba

Copy link
Copy Markdown
Member

Bug

NarrowUnusedSetUpDefinedPropertyRector turns a property only used in setUp()
into a local variable. While doing so it rewrites every $this->property
reference inside setUp() to a bare $property variable — including references
inside nested closures stored for later execution (resolver callbacks, deferred
factories, etc.).

A closure does not capture that local variable (the rule adds no use ($property)
binding), so when the closure runs after setUp() the variable is undefined and
the closure silently returns null.

Found in webonyx/graphql-php
(DeferredFieldsTest), whose rector.php skips this rule. A resolver closure:

$this->categoryDataSource = [/* ... */];
// ...
'resolve' => function () {
    return $this->categoryDataSource; // -> $categoryDataSource (undefined in closure) -> null
}

The rewrite made the deferred resolvers return null, breaking the tests
(verified: 3 failures with zend.assertions=1, gone after the fix, full suite
otherwise unchanged).

Fix

Skip narrowing a property when it is referenced inside a Closure or
ArrowFunction within setUp(). Added skip_property_used_in_closure.php.inc.

…y used in nested closure

The rule turned a setUp() property into a local variable while rewriting every
$this->property reference inside setUp(), including references inside nested
closures (e.g. resolver callbacks stored for later execution). A closure does
not capture the local variable (no 'use' binding is added), so at call time the
variable was undefined and the closure silently returned null.

Skip narrowing when the property is referenced inside a Closure or
ArrowFunction within setUp().
@TomasVotruba TomasVotruba enabled auto-merge (squash) June 29, 2026 15:57
@TomasVotruba TomasVotruba disabled auto-merge June 29, 2026 15:57
@TomasVotruba TomasVotruba merged commit 2852aed into main Jun 29, 2026
8 checks passed
@TomasVotruba TomasVotruba deleted the fix-narrow-setup-property-used-in-closure branch June 29, 2026 16:01
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.

1 participant