Skip to content

Fix phpstan/phpstan#14352: "Cannot re-assign $this." false-positive #5281

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
staabm merged 11 commits into from
Mar 23, 2026
+112 −0
Merged

Fix phpstan/phpstan#14352: "Cannot re-assign $this." false-positive #5281

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
4f11780
"Cannot re-assign $this." false-positive
staabm Mar 23, 2026
6dfeac6
Update InvalidVariableAssignRuleTest.php
staabm Mar 23, 2026
bbf04d1
test $this=$value when $value is ArrayAccess
staabm Mar 23, 2026
97e4965
fix lint
staabm Mar 23, 2026
8d2b7d4
Use dimFetch flag instead of checking assigned expression type for $t…
phpstan-bot Mar 23, 2026
2e8875d
Revert "Use dimFetch flag instead of checking assigned expression typ…
staabm Mar 23, 2026
5c5e3c7
Use dimFetch flag and check $this type with !->no() for ArrayAccess r…
phpstan-bot Mar 23, 2026
ff7af6b
Revert "Use dimFetch flag and check $this type with !->no() for Array…
staabm Mar 23, 2026
12e4ef5
fix
staabm Mar 23, 2026
272e2a9
Update InvalidVariableAssignRule.php
staabm Mar 23, 2026
5f16ff1
Update InvalidVariableAssignRule.php
staabm Mar 23, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,7 @@ lint:
--exclude tests/PHPStan/Rules/EnumCases/data/bug-14252.php \
--exclude tests/PHPStan/Rules/Functions/data/bug-14241.php \
--exclude tests/PHPStan/Rules/Variables/data/bug-14349.php \
--exclude tests/PHPStan/Rules/Variables/data/bug-14352.php \
src tests

install-paratest:
Expand Down
9 changes: 9 additions & 0 deletions src/Rules/Variables/InvalidVariableAssignRule.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,14 @@

namespace PHPStan\Rules\Variables;

use ArrayAccess;
use PhpParser\Node;
use PHPStan\Analyser\Scope;
use PHPStan\DependencyInjection\RegisteredRule;
use PHPStan\Node\VariableAssignNode;
use PHPStan\Rules\Rule;
use PHPStan\Rules\RuleErrorBuilder;
use PHPStan\Type\ObjectType;
use function is_string;

/**
Expand All @@ -30,6 +32,13 @@ public function processNode(Node $node, Scope $scope): array
}

if ($variable->name === 'this') {
$expr = $node->getAssignedExpr();
$type = $scope->getType($expr);

if ((new ObjectType(ArrayAccess::class))->isSuperTypeOf($type)->yes()) {
return [];
}

return [
RuleErrorBuilder::message('Cannot re-assign $this.')
->identifier('assign.this')
Expand Down
16 changes: 16 additions & 0 deletions tests/PHPStan/Rules/Variables/InvalidVariableAssignRuleTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

use PHPStan\Rules\Rule;
use PHPStan\Testing\RuleTestCase;
use PHPUnit\Framework\Attributes\RequiresPhp;

/**
* @extends RuleTestCase<InvalidVariableAssignRule>
Expand Down Expand Up @@ -50,6 +51,21 @@ public function testBug3585(): void
]);
}

#[RequiresPhp('>= 8.0')]
public function testBug14352(): void
{
$this->analyse([__DIR__ . '/data/bug-14352.php'], [
[
'Cannot re-assign $this.',
13,
],
[
'Cannot re-assign $this.',
37,
],
]);
}

public function testBug14349(): void
{
$this->analyse([__DIR__ . '/data/bug-14349.php'], [
Expand Down
47 changes: 47 additions & 0 deletions tests/PHPStan/Rules/Variables/data/bug-14352.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
<?php declare(strict_types = 1);

namespace Bug14352;

use ArrayAccess;

class TestArrayAccess implements ArrayAccess
{
public function doFoo(string $key, string $value): void
{
$this[$key] = $value; // fine because ArrayAccess

$this = $value; // should still fail
}

public function offsetExists(mixed $offset): bool
{
}

public function offsetGet(mixed $offset): mixed
{
}

public function offsetSet(mixed $offset, mixed $value): void
{
}

public function offsetUnset(mixed $offset): void
{
}
}

final class FinalTestPlain
{
public function doFoo(string $key, string $value): void
{
$this[$key] = $value;
}
}

class TestPlain
{
public function doFoo(string $key, string $value): void
{
$this[$key] = $value;
}
}
Copy link
Contributor

@VincentLanglet VincentLanglet Mar 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know the case

class TestPlain
{
	public function doFoo(string $key, string $value): void
	{
		$this[$key] = $value;
	}
}

might sometimes works if we extends this class and implement ArrayAccess.

But on the opposide, the code

$t = new TestPlain();

$t->doFoo('', '');

will crash.

IMHO ->yes() should be !->no()
and we will have a behavior similar to https://phpstan.org/r/ae445bd2-da35-4b9f-b0cb-a44c0d1b71e8

Copy link
Collaborator Author

@phpstan-bot phpstan-bot Mar 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pushed. Here's a summary of the changes:

Problem: The reviewer pointed out that for non-final classes like TestPlain, $this[$key] = $value should not error because a subclass might implement ArrayAccess. The check should use !->no() instead of ->yes().

Changes:

  1. VariableAssignNode — Added dimFetch flag to distinguish $this[$key] = $value (dim-fetch) from $this = $value (direct assignment)

  2. AssignHandler — Pass dimFetch: true when creating VariableAssignNode from dim-fetch assignment paths

  3. InvalidVariableAssignRule — Changed the check to:

    • Use $node->isDimFetch() to only suppress errors for dim-fetch assignments
    • Check $scope->getType($variable) (the type of $this) instead of the assigned expression type
    • Use !->no() instead of ->yes() — this allows the "maybe" case for non-final classes (line 614 of ObjectType::isSuperTypeOf returns Maybe when checking an interface against a non-final class)

  4. Test data — Added TestArrayAccessReassign case to verify $this = $other still errors even when $other implements ArrayAccess

Behavior:

  • TestArrayAccess (implements ArrayAccess): $this[$key] = $value → no error, $this = $value → error
  • TestArrayAccessReassign: $this = $other → error (even though $other is ArrayAccess)
  • FinalTestPlain (final, no ArrayAccess): $this[$key] = $value → error
  • TestPlain (non-final, no ArrayAccess): $this[$key] = $value → no error (subclass might implement ArrayAccess)

Loading