Skip to content

[TypeDeclaration] Skip constructor-assigned property in PropertyTypeFromStrictSetterGetterRector#8137

Merged
TomasVotruba merged 2 commits into
mainfrom
tv-setter-inferer-constructor-guard
Jul 2, 2026
Merged

[TypeDeclaration] Skip constructor-assigned property in PropertyTypeFromStrictSetterGetterRector#8137
TomasVotruba merged 2 commits into
mainfrom
tv-setter-inferer-constructor-guard

Conversation

@TomasVotruba

Copy link
Copy Markdown
Member

Problem

PropertyTypeFromStrictSetterGetterRector inferred a wrong property type when a constructor assigns the property.

Given:

class MaxMindDoNotSellList
{
    private $listPath;

    public function __construct(CoreParametersHelper $coreParametersHelper)
    {
        $this->listPath = $coreParametersHelper->get('maxmind_do_not_sell_list_path', '');
    }

    public function getListPath(): string
    {
        return $this->listPath;
    }

    public function setListPath(string $path): void
    {
        $this->listPath = $path;
    }
}

it produced:

-    private $listPath;
+    private \Mautic\CoreBundle\Helper\CoreParametersHelper|string $listPath;

Cause

SetterTypeDeclarationPropertyTypeInferer used hasOnlyPropertyAssign(), which matches any single-statement $this->prop = <expr> method — including the constructor. The constructor got treated as a setter, so its first param type (CoreParametersHelper) leaked into the inference, unioning with the getter's string.

Fix

Add ClassMethodAndPropertyAnalyzer::hasOnlyFirstParamPropertyAssign(), which additionally requires the assigned value to be exactly the method's first parameter variable ($this->prop = $param), and use it in the setter inferer. The shared hasOnlyPropertyAssign() is left unchanged (still used by Php84\SetterAndGetterFinder).

Now the property is correctly typed private string $listPath;.

Tests

  • constructor_assigned_no_default.php.inc — direct constructor param assign
  • constructor_non_param_assign.php.inc (+ Source/SomeConfigHelper.php) — constructor assigns a method-call result; constructor is ignored, setter provides string

…terRector setter inference

The setter type inferer matched any single-statement `$this->prop = <expr>`
method as a setter, including constructors. A constructor such as
`$this->listPath = $coreParametersHelper->get(...)` was treated as a setter,
so its first param type (CoreParametersHelper) leaked into the inferred
property type, producing a bogus `CoreParametersHelper|string` union.

Add `hasOnlyFirstParamPropertyAssign()` which additionally requires the
assigned value to be exactly the method's first parameter variable
(`$this->prop = $param`), and use it in the setter inferer.
Simpler, more conservative: if a property is assigned anywhere in the
constructor, its runtime type may differ from the setter/getter, so skip
it entirely rather than inferring a type.

Reverts the setter-inferer/analyzer change in favor of an isAssignedInConstructor()
guard in the rule.
@TomasVotruba TomasVotruba changed the title [TypeDeclaration] Skip constructor in PropertyTypeFromStrictSetterGetterRector setter inference [TypeDeclaration] Skip constructor-assigned property in PropertyTypeFromStrictSetterGetterRector Jul 2, 2026
@TomasVotruba TomasVotruba merged commit 40100d1 into main Jul 2, 2026
65 checks passed
@TomasVotruba TomasVotruba deleted the tv-setter-inferer-constructor-guard branch July 2, 2026 19:52
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