Skip to content

Commit

Permalink
Fixed '$this instanceof X will always be false' in traits
Browse files Browse the repository at this point in the history
  • Loading branch information
staabm committed Jan 31, 2023
1 parent 39e645c commit e1c8380
Show file tree
Hide file tree
Showing 9 changed files with 377 additions and 9 deletions.
31 changes: 22 additions & 9 deletions src/Analyser/MutatingScope.php
Original file line number Diff line number Diff line change
Expand Up @@ -2424,18 +2424,18 @@ public function isInFunctionExists(string $functionName): bool
public function enterClass(ClassReflection $classReflection): self
{
$thisHolder = ExpressionTypeHolder::createYes(new Variable('this'), new ThisType($classReflection));
$constantTypes = $this->getConstantTypes();
$constantTypes['$this'] = $thisHolder;
$nativeConstantTypes = $this->getNativeConstantTypes();
$nativeConstantTypes['$this'] = $thisHolder;

return $this->scopeFactory->create(
$this->context->enterClass($classReflection),
$this->isDeclareStrictTypes(),
null,
$this->getNamespace(),
array_merge($this->getConstantTypes(), [
'$this' => $thisHolder,
]),
array_merge($this->getNativeConstantTypes(), [
'$this' => $thisHolder,
]),
$constantTypes,
$nativeConstantTypes,
[],
null,
null,
Expand All @@ -2456,13 +2456,26 @@ public function enterTrait(ClassReflection $traitReflection): self
if (count($traitNameParts) > 1) {
$namespace = implode('\\', array_slice($traitNameParts, 0, -1));
}

$traitContext = $this->context->enterTrait($traitReflection);
$classReflection = $traitContext->getClassReflection();
if ($classReflection === null) {
throw new ShouldNotHappenException();
}

$thisHolder = ExpressionTypeHolder::createYes(new Variable('this'), new ThisType($classReflection, null, $traitReflection));
$expressionTypes = $this->expressionTypes;
$expressionTypes['$this'] = $thisHolder;
$nativeExpressionTypes = $this->nativeExpressionTypes;
$nativeExpressionTypes['$this'] = $thisHolder;

return $this->scopeFactory->create(
$this->context->enterTrait($traitReflection),
$traitContext,
$this->isDeclareStrictTypes(),
$this->getFunction(),
$namespace,
$this->expressionTypes,
$this->nativeExpressionTypes,
$expressionTypes,
$nativeExpressionTypes,
[],
$this->inClosureBindScopeClass,
$this->anonymousFunctionReflection,
Expand Down
25 changes: 25 additions & 0 deletions src/Type/ObjectType.php
Original file line number Diff line number Diff line change
Expand Up @@ -286,6 +286,31 @@ public function isSuperTypeOf(Type $type): TrinaryLogic
return self::$superTypes[$thisDescription][$description] = TrinaryLogic::createMaybe();
}

if ($type instanceof ThisType && $type->isInTrait()) {
if ($type->getSubtractedType() !== null) {
$isSuperType = $type->getSubtractedType()->isSuperTypeOf($this);
if ($isSuperType->yes()) {
return self::$superTypes[$thisDescription][$description] = TrinaryLogic::createNo();
}
}

if ($this->getClassReflection() === null) {
return self::$superTypes[$thisDescription][$description] = TrinaryLogic::createMaybe();
}

$thisClassReflection = $this->getClassReflection();
if ($thisClassReflection->isTrait()) {
return self::$superTypes[$thisDescription][$description] = TrinaryLogic::createNo();
}

$traitReflection = $type->getTraitReflection();
if ($thisClassReflection->isFinal() && !$thisClassReflection->hasTraitUse($traitReflection->getName())) {
return self::$superTypes[$thisDescription][$description] = TrinaryLogic::createNo();
}

return self::$superTypes[$thisDescription][$description] = TrinaryLogic::createMaybe();
}

$transformResult = static fn (TrinaryLogic $result) => $result;
if ($this->subtractedType !== null) {
$isSuperType = $this->subtractedType->isSuperTypeOf($type);
Expand Down
14 changes: 14 additions & 0 deletions src/Type/ThisType.php
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ class ThisType extends StaticType
public function __construct(
ClassReflection $classReflection,
?Type $subtractedType = null,
private ?ClassReflection $traitReflection = null,
)
{
parent::__construct($classReflection, $subtractedType);
Expand Down Expand Up @@ -57,6 +58,19 @@ public function changeSubtractedType(?Type $subtractedType): Type
return $type;
}

/**
* @phpstan-assert-if-true !null $this->getTraitReflection()
*/
public function isInTrait(): bool
{
return $this->traitReflection !== null;
}

public function getTraitReflection(): ?ClassReflection
{
return $this->traitReflection;
}

public function traverse(callable $cb): Type
{
$subtractedType = $this->getSubtractedType() !== null ? $cb($this->getSubtractedType()) : null;
Expand Down
1 change: 1 addition & 0 deletions tests/PHPStan/Analyser/NodeScopeResolverTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -1179,6 +1179,7 @@ public function dataFileAsserts(): iterable
yield from $this->gatherAssertTypes(__DIR__ . '/data/callsite-cast-narrowing.php');
yield from $this->gatherAssertTypes(__DIR__ . '/data/bug-8775.php');
yield from $this->gatherAssertTypes(__DIR__ . '/data/bug-8752.php');
yield from $this->gatherAssertTypes(__DIR__ . '/data/trait-instance-of.php');
}

/**
Expand Down
70 changes: 70 additions & 0 deletions tests/PHPStan/Analyser/data/trait-instance-of.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
<?php declare(strict_types = 1);

namespace TraitInstanceOf;

use function PHPStan\Testing\assertType;

trait Trait1 {
public function test(): string {
assertType('$this(TraitInstanceOf\ATrait1Class)', $this);
if ($this instanceof WithoutFoo) {
assertType('$this(TraitInstanceOf\ATrait1Class)&TraitInstanceOf\WithoutFoo', $this);
return 'hello world';
}

if ($this instanceof FinalOther) {
assertType('*NEVER*', $this);
return 'hello world';
}

assertType('$this(TraitInstanceOf\ATrait1Class)', $this);
if ($this instanceof Trait2) {
assertType('*NEVER*', $this);
return 'hello world';
}

if ($this instanceof FinalTrait2Class) {
assertType('*NEVER*', $this);
return 'hello world';
}

assertType('$this(TraitInstanceOf\ATrait1Class)', $this);
throw new \Error();
}
}

trait Trait2 {
public function test(): string {
assertType('$this(TraitInstanceOf\FinalTrait2Class)', $this);

if ($this instanceof FinalTrait2Class) {
assertType('$this(TraitInstanceOf\FinalTrait2Class)&TraitInstanceOf\FinalTrait2Class', $this);
return 'hello world';
}

if ($this instanceof ATrait1Class) {
assertType('*NEVER*', $this);
return 'hello world';
}

if ($this instanceof FinalOther) {
assertType('*NEVER*', $this);
return 'hello world';
}

return 'hello world';
}
}

final class FinalOther {
}

final class FinalTrait2Class {
use Trait2;
}

class WithoutFoo {}

class ATrait1Class {
use Trait1;
}
10 changes: 10 additions & 0 deletions tests/PHPStan/Rules/Classes/ExistingClassInInstanceOfRuleTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -70,4 +70,14 @@ public function testBug7720(): void
]);
}

public function testTraitInstanceOf(): void
{
$this->analyse([__DIR__ . '/../../Analyser/data/trait-instance-of.php'], [
[
'Instanceof between $this(TraitInstanceOf\ATrait1Class) and trait TraitInstanceOf\Trait2 will always evaluate to false.',
21,
],
]);
}

}
7 changes: 7 additions & 0 deletions tests/PHPStan/Rules/Classes/ImpossibleInstanceOfRuleTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -368,4 +368,11 @@ public function testBug5333(): void
]);
}

public function testBug3632(): void
{
$this->checkAlwaysTrueInstanceOf = true;
$this->treatPhpDocTypesAsCertain = false;
$this->analyse([__DIR__ . '/data/bug-3632.php'], []);
}

}
33 changes: 33 additions & 0 deletions tests/PHPStan/Rules/Classes/data/bug-3632.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
<?php declare(strict_types = 1);

namespace Bug3632;

trait Foo {
public function test(): string {
if ($this instanceof HelloWorld) {
return 'hello world';
}
if ($this instanceof OtherClass) {
return 'other class';
}

return 'no';
}
}

class HelloWorld
{
use Foo;

function bar(): string {
return $this->test();
}
}

class OtherClass {
use Foo;

function bar(): string {
return $this->test();
}
}
Loading

0 comments on commit e1c8380

Please sign in to comment.