Skip to content

[Php71] Skip AssignArrayToStringRector on a variable re-assigned as string - #8293

Merged
TomasVotruba merged 2 commits into
mainfrom
fix-assign-array-to-string-reassigned-as-string
Aug 5, 2026
Merged

[Php71] Skip AssignArrayToStringRector on a variable re-assigned as string#8293
TomasVotruba merged 2 commits into
mainfrom
fix-assign-array-to-string-reassigned-as-string

Conversation

@TomasVotruba

@TomasVotruba TomasVotruba commented Aug 5, 2026

Copy link
Copy Markdown
Member

Main is red on AssignArrayToStringRectorTest, fixture skip_reassigned_as_string.php.inc. Not caused by any commit here — phpstan/phpstan 2.2.8 was released, and CI resolves it while a local vendor/ still on 2.2.7 stays green. Reproducible with composer update phpstan/phpstan.

2.2.8 infers a possibly undefined variable as ErrorType, where it used to report a union of its possible types. The rule leaned on that union:

if (empty($where)) {
    $where = '';           // native type of $where was array{}|''|null|…, now *ERROR*
} else {
    $where = 'WHERE ' . implode(' AND ', $where);
}

$sql = 'SELECT …' . $where;

Neither the isArray() nor the UnionType guard matches an ErrorType, so the empty string became an array and broke the concatenation below:

 if (empty($where)) {
-    $where = '';
+    $where = [];
 } else {
     $where = 'WHERE ' . implode(' AND ', $where);
 }

Guarding on ErrorType is not the fix — a first assignment like $string = ''; reports *ERROR* for the same reason, and those must keep converting.

What actually makes $where a string here is that it is assigned a string further down, so the rule now checks exactly that. An empty string assign does not count as proof, since it is itself a candidate for the same re-type — that keeps fixture.php.inc converting its second $string = '';.

Full suite green on 2.2.8: 5372 tests, 6884 assertions.

Second commit

SetManagerTest still expected 8 Twig composer-triggered sets. rectorphp/rector-symfony#1010 is merged, so TwigSetProvider now registers only its composer-based trigger — every rule of the per-version Twig sets is already in it, bound to the twig/twig version it needs. Folded in here so this PR is green on its own; it was #8294, now closed.

@TomasVotruba
TomasVotruba enabled auto-merge (squash) August 5, 2026 09:36
…tring

PHPStan 2.2.8 infers a possibly undefined variable as ErrorType, where it used to be a union of its possible types. AssignArrayToStringRector leaned on that union to skip:

    if (empty($where)) {
        $where = '';
    } else {
        $where = 'WHERE ' . implode(' AND ', $where);
    }

With ErrorType, neither the isArray() nor the UnionType guard matches, and the empty string turned into an array, breaking the concat that follows.

The variable being filled as a string later on is what makes it a string here, so that is now checked directly. An empty string assign does not count, as it is a candidate for the very same re-type.
@TomasVotruba
TomasVotruba force-pushed the fix-assign-array-to-string-reassigned-as-string branch from 5cf4efd to b2ebd76 Compare August 5, 2026 10:03
rectorphp/rector-symfony#1010 trims TwigSetProvider to its composer-based trigger, as every rule of the per-version Twig sets is already in it, bound to the twig/twig version it needs.
@TomasVotruba
TomasVotruba merged commit 760f8c0 into main Aug 5, 2026
64 checks passed
@TomasVotruba
TomasVotruba deleted the fix-assign-array-to-string-reassigned-as-string branch August 5, 2026 10:06
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