Skip to content

Commit

Permalink
Check redeclared promoted readonly properties
Browse files Browse the repository at this point in the history
  • Loading branch information
schlndh committed May 6, 2024
1 parent 1f95482 commit 1004481
Show file tree
Hide file tree
Showing 9 changed files with 568 additions and 0 deletions.
21 changes: 21 additions & 0 deletions src/Analyser/NodeScopeResolver.php
Expand Up @@ -177,6 +177,7 @@
use PHPStan\Type\TypeTraverser;
use PHPStan\Type\TypeUtils;
use PHPStan\Type\UnionType;
use ReflectionProperty;
use Throwable;
use Traversable;
use TypeError;
Expand Down Expand Up @@ -2722,6 +2723,26 @@ static function (): void {
$scope = $scope->invalidateExpression(new Variable('this'), true);
}

if (
$methodReflection !== null
&& !$methodReflection->isStatic()
&& $methodReflection->getName() === '__construct'
&& $scopeFunction instanceof MethodReflection
&& !$scopeFunction->isStatic()
&& $scope->isInClass()
&& $scope->getClassReflection()->isSubclassOf($methodReflection->getDeclaringClass()->getName())
) {
$thisType = $scope->getType(new Variable('this'));
$methodClassReflection = $methodReflection->getDeclaringClass();
foreach ($methodClassReflection->getNativeReflection()->getProperties(ReflectionProperty::IS_PUBLIC | ReflectionProperty::IS_PROTECTED) as $property) {
if (!$property->isPromoted() || $property->getDeclaringClass()->getName() !== $methodClassReflection->getName()) {
continue;
}

$scope = $scope->assignInitializedProperty($thisType, $property->getName());
}
}

$hasYield = $hasYield || $result->hasYield();
$throwPoints = array_merge($throwPoints, $result->getThrowPoints());
$impurePoints = array_merge($impurePoints, $result->getImpurePoints());
Expand Down
28 changes: 28 additions & 0 deletions src/Node/ClassStatementsGatherer.php
Expand Up @@ -18,6 +18,7 @@
use PHPStan\Reflection\ClassReflection;
use PHPStan\ShouldNotHappenException;
use PHPStan\Type\TypeUtils;
use ReflectionProperty;
use function count;
use function in_array;
use function strtolower;
Expand Down Expand Up @@ -157,6 +158,9 @@ private function gatherNodes(Node $node, Scope $scope): void
}
if ($node instanceof MethodCall || $node instanceof StaticCall) {
$this->methodCalls[] = new \PHPStan\Node\Method\MethodCall($node, $scope);
if ($node instanceof StaticCall && $node->name instanceof Identifier && $node->name->toLowerString() === '__construct') {
$this->tryToApplyPropertyWritesFromAncestorConstructor($node, $scope);
}
return;
}
if ($node instanceof MethodCallableNode || $node instanceof StaticMethodCallableNode) {
Expand Down Expand Up @@ -252,4 +256,28 @@ private function tryToApplyPropertyReads(Expr\FuncCall $node, Scope $scope): voi
}
}

private function tryToApplyPropertyWritesFromAncestorConstructor(StaticCall $ancestorConstructorCall, Scope $scope): void
{
if (!$ancestorConstructorCall->class instanceof Node\Name) {
return;
}

$calledOnType = $scope->resolveTypeByName($ancestorConstructorCall->class);
if ($calledOnType->getClassReflection() === null || TypeUtils::findThisType($calledOnType) === null) {
return;
}

$classReflection = $calledOnType->getClassReflection()->getNativeReflection();
foreach ($classReflection->getProperties(ReflectionProperty::IS_PUBLIC | ReflectionProperty::IS_PROTECTED) as $property) {
if (!$property->isPromoted() || $property->getDeclaringClass()->getName() !== $classReflection->getName()) {
continue;
}
$this->propertyUsages[] = new PropertyWrite(
new PropertyFetch(new Expr\Variable('this'), new Identifier($property->getName()), $ancestorConstructorCall->getAttributes()),
$scope,
false,
);
}
}

}
Expand Up @@ -287,4 +287,101 @@ public function testBug10822(): void
$this->analyse([__DIR__ . '/data/bug-10822.php'], []);
}

public function testRedeclaredReadonlyProperties(): void
{
if (PHP_VERSION_ID < 80100) {
$this->markTestSkipped('Test requires PHP 8.1.');
}

$this->analyse([__DIR__ . '/data/redeclare-readonly-property.php'], [
[
'Readonly property RedeclareReadonlyProperty\B1::$myProp is already assigned.',
16,
],
[
'Readonly property RedeclareReadonlyProperty\B5::$myProp is already assigned.',
50,
],
[
'Readonly property RedeclareReadonlyProperty\B7::$myProp is already assigned.',
70,
],
[
'Readonly property class@anonymous/tests/PHPStan/Rules/Properties/data/redeclare-readonly-property.php:117::$myProp is already assigned.',
121,
],
[
'Class RedeclareReadonlyProperty\B16 has an uninitialized readonly property $myProp. Assign it in the constructor.',
195,
],
[
'Class RedeclareReadonlyProperty\C17 has an uninitialized readonly property $aProp. Assign it in the constructor.',
218,
],
[
'Class RedeclareReadonlyProperty\B18 has an uninitialized readonly property $aProp. Assign it in the constructor.',
233,
],
]);
}

public function testRedeclaredPropertiesOfReadonlyClass(): void
{
if (PHP_VERSION_ID < 80200) {
$this->markTestSkipped('Test requires PHP 8.2.');
}

$this->analyse([__DIR__ . '/data/redeclare-property-of-readonly-class.php'], [
[
'Readonly property RedeclarePropertyOfReadonlyClass\B1::$promotedProp is already assigned.',
15,
],
]);
}

public function testBug8101(): void
{
if (PHP_VERSION_ID < 80100) {
$this->markTestSkipped('Test requires PHP 8.1.');
}

$this->analyse([__DIR__ . '/data/bug-8101.php'], [
[
'Readonly property Bug8101\B::$myProp is already assigned.',
12,
],
]);
}

public function testBug9863(): void
{
if (PHP_VERSION_ID < 80100) {
$this->markTestSkipped('Test requires PHP 8.1.');
}

$this->analyse([__DIR__ . '/data/bug-9863.php'], [
[
'Readonly property Bug9863\ReadonlyChildWithoutIsset::$foo is already assigned.',
17,
],
[
'Class Bug9863\ReadonlyParentWithIsset has an uninitialized readonly property $foo. Assign it in the constructor.',
23,
],
[
'Access to an uninitialized readonly property Bug9863\ReadonlyParentWithIsset::$foo.',
28,
],
]);
}

public function testBug9864(): void
{
if (PHP_VERSION_ID < 80100) {
$this->markTestSkipped('Test requires PHP 8.1.');
}

$this->analyse([__DIR__ . '/data/bug-9864.php'], []);
}

}
14 changes: 14 additions & 0 deletions tests/PHPStan/Rules/Properties/UninitializedPropertyRuleTest.php
Expand Up @@ -202,4 +202,18 @@ public function testBug9831(): void
]);
}

public function testRedeclareReadonlyProperties(): void
{
$this->analyse([__DIR__ . '/data/redeclare-readonly-property.php'], [
[
'Class RedeclareReadonlyProperty\B19 has an uninitialized property $prop2. Give it default value or assign it in the constructor.',
249,
],
[
'Access to an uninitialized property RedeclareReadonlyProperty\B19::$prop2.',
260,
],
]);
}

}
16 changes: 16 additions & 0 deletions tests/PHPStan/Rules/Properties/data/bug-8101.php
@@ -0,0 +1,16 @@
<?php declare(strict_types = 1); // lint >= 8.1

namespace Bug8101;

class A {
public function __construct(public readonly int $myProp) {}
}

class B extends A {
// This should be reported as an error, as a readonly prop cannot be redeclared.
public function __construct(public readonly int $myProp) {
parent::__construct($myProp);
}
}

$foo = new B(7);
49 changes: 49 additions & 0 deletions tests/PHPStan/Rules/Properties/data/bug-9863.php
@@ -0,0 +1,49 @@
<?php declare(strict_types = 1); // lint >= 8.1

namespace Bug9863;

class ReadonlyParentWithoutIsset
{
public function __construct(
public readonly int $foo
) {}
}

class ReadonlyChildWithoutIsset extends ReadonlyParentWithoutIsset
{
public function __construct(
public readonly int $foo = 42
) {
parent::__construct($foo);
}
}

class ReadonlyParentWithIsset
{
public readonly int $foo;

public function __construct(
int $foo
) {
if (! isset($this->foo)) {
$this->foo = $foo;
}
}
}

class ReadonlyChildWithIsset extends ReadonlyParentWithIsset
{
public function __construct(
public readonly int $foo = 42
) {
parent::__construct($foo);
}
}

$a = new ReadonlyParentWithoutIsset(0);
$b = new ReadonlyChildWithoutIsset();
$c = new ReadonlyChildWithoutIsset(1);

$x = new ReadonlyParentWithIsset(2);
$y = new ReadonlyChildWithIsset();
$z = new ReadonlyChildWithIsset(3);
31 changes: 31 additions & 0 deletions tests/PHPStan/Rules/Properties/data/bug-9864.php
@@ -0,0 +1,31 @@
<?php declare(strict_types = 1); // lint >= 8.2

namespace Bug9864;

readonly abstract class UuidValueObject
{
public function __construct(public string $value)
{
$this->ensureIsValidUuid($value);
}

private function ensureIsValidUuid(string $value): void
{
}
}


final readonly class ProductId extends UuidValueObject
{
public string $value;

public function __construct(
string $value
) {
parent::__construct($value);
}
}

var_dump(new ProductId('test'));

// property is assigned on parent class, no need to reassing, specially for readonly properties
@@ -0,0 +1,50 @@
<?php declare(strict_types = 1); // lint >= 8.2

namespace RedeclarePropertyOfReadonlyClass;

readonly class A {
public function __construct(public int $promotedProp)
{
}
}

readonly class B1 extends A {
// $promotedProp is written twice
public function __construct(public int $promotedProp)
{
parent::__construct(5);
}
}

readonly class B2 extends A {
// Don't get confused by standard parameter with same name
public function __construct(int $promotedProp)
{
parent::__construct($promotedProp);
}
}

readonly class B3 extends A {
// This is allowed, because we don't write to the property.
public int $promotedProp;

public function __construct()
{
parent::__construct(7);
}
}

readonly class B4 extends A {
// The second write is not from the constructor. It is an error, but it is handled by different rule.
public int $promotedProp;

public function __construct()
{
parent::__construct(7);
}

public function set(): void
{
$this->promotedProp = 7;
}
}

0 comments on commit 1004481

Please sign in to comment.