Skip to content

Commit

Permalink
Rule about readonly properties in constructor
Browse files Browse the repository at this point in the history
  • Loading branch information
ondrejmirtes committed Jan 14, 2022
1 parent 28bd563 commit 44fd938
Show file tree
Hide file tree
Showing 5 changed files with 312 additions and 7 deletions.
7 changes: 7 additions & 0 deletions conf/config.level0.neon
Expand Up @@ -164,6 +164,13 @@ services:
tags:
- phpstan.rules.rule

-
class: PHPStan\Rules\Properties\MissingReadOnlyPropertyAssignRule
arguments:
additionalConstructors: %additionalConstructors%
tags:
- phpstan.rules.rule

-
class: PHPStan\Rules\Properties\OverridingPropertyRule
arguments:
Expand Down
22 changes: 15 additions & 7 deletions src/Node/ClassPropertiesNode.php
Expand Up @@ -75,7 +75,7 @@ public function getSubNodeNames(): array
/**
* @param string[] $constructors
* @param ReadWritePropertiesExtension[] $extensions
* @return array{array<string, ClassPropertyNode>, array<array{string, int, ClassPropertyNode}>}
* @return array{array<string, ClassPropertyNode>, array<array{string, int, ClassPropertyNode}>, array<array{string, int, ClassPropertyNode}>}
*/
public function getUninitializedProperties(
Scope $scope,
Expand All @@ -84,7 +84,7 @@ public function getUninitializedProperties(
): array
{
if (!$this->getClass() instanceof Class_) {
return [[], []];
return [[], [], []];
}
if (!$scope->isInClass()) {
throw new ShouldNotHappenException();
Expand Down Expand Up @@ -120,11 +120,13 @@ public function getUninitializedProperties(
}

if ($constructors === []) {
return [$properties, []];
return [$properties, [], []];
}
$classType = new ObjectType($scope->getClassReflection()->getName());
$methodsCalledFromConstructor = $this->getMethodsCalledFromConstructor($classType, $this->methodCalls, $constructors);
$prematureAccess = [];
$additionalAssigns = [];
$originalProperties = $properties;
foreach ($this->getPropertyUsages() as $usage) {
$fetch = $usage->getFetch();
if (!$fetch instanceof PropertyFetch) {
Expand All @@ -149,9 +151,6 @@ public function getUninitializedProperties(
continue;
}
$propertyName = $fetch->name->toString();
if (!array_key_exists($propertyName, $properties)) {
continue;
}
$fetchedOnType = $usageScope->getType($fetch->var);
if ($classType->isSuperTypeOf($fetchedOnType)->no()) {
continue;
Expand All @@ -161,7 +160,15 @@ public function getUninitializedProperties(
}

if ($usage instanceof PropertyWrite) {
unset($properties[$propertyName]);
if (array_key_exists($propertyName, $properties)) {
unset($properties[$propertyName]);
} elseif (array_key_exists($propertyName, $originalProperties)) {
$additionalAssigns[] = [
$propertyName,
$fetch->getLine(),
$originalProperties[$propertyName],
];
}
} elseif (array_key_exists($propertyName, $properties)) {
$prematureAccess[] = [
$propertyName,
Expand All @@ -174,6 +181,7 @@ public function getUninitializedProperties(
return [
$properties,
$prematureAccess,
$additionalAssigns,
];
}

Expand Down
127 changes: 127 additions & 0 deletions src/Rules/Properties/MissingReadOnlyPropertyAssignRule.php
@@ -0,0 +1,127 @@
<?php declare(strict_types = 1);

namespace PHPStan\Rules\Properties;

use PhpParser\Node;
use PHPStan\Analyser\Scope;
use PHPStan\Node\ClassPropertiesNode;
use PHPStan\Reflection\ClassReflection;
use PHPStan\Rules\Rule;
use PHPStan\Rules\RuleErrorBuilder;
use PHPStan\ShouldNotHappenException;
use ReflectionException;
use function array_key_exists;
use function explode;
use function sprintf;

/**
* @implements Rule<ClassPropertiesNode>
*/
class MissingReadOnlyPropertyAssignRule implements Rule
{

/** @var array<string, string[]> */
private array $additionalConstructorsCache = [];

/**
* @param string[] $additionalConstructors
*/
public function __construct(
private array $additionalConstructors,
)
{
}

public function getNodeType(): string
{
return ClassPropertiesNode::class;
}

public function processNode(Node $node, Scope $scope): array
{
if (!$scope->isInClass()) {
throw new ShouldNotHappenException();
}
$classReflection = $scope->getClassReflection();
[$properties, $prematureAccess, $additionalAssigns] = $node->getUninitializedProperties($scope, $this->getConstructors($classReflection), []);

$errors = [];
foreach ($properties as $propertyName => $propertyNode) {
if (!$propertyNode->isReadOnly()) {
continue;
}
$errors[] = RuleErrorBuilder::message(sprintf(
'Class %s has an uninitialized readonly property $%s. Assign it in the constructor.',
$classReflection->getDisplayName(),
$propertyName,
))->line($propertyNode->getLine())->build();
}

foreach ($prematureAccess as [$propertyName, $line, $propertyNode]) {
if (!$propertyNode->isReadOnly()) {
continue;
}
$errors[] = RuleErrorBuilder::message(sprintf(
'Access to an uninitialized readonly property %s::$%s.',
$classReflection->getDisplayName(),
$propertyName,
))->line($line)->build();
}

foreach ($additionalAssigns as [$propertyName, $line, $propertyNode]) {
if (!$propertyNode->isReadOnly()) {
continue;
}
$errors[] = RuleErrorBuilder::message(sprintf(
'Readonly property %s::$%s is already assigned.',
$classReflection->getDisplayName(),
$propertyName,
))->line($line)->build();
}

return $errors;
}

/**
* @return string[]
*/
private function getConstructors(ClassReflection $classReflection): array
{
if (array_key_exists($classReflection->getName(), $this->additionalConstructorsCache)) {
return $this->additionalConstructorsCache[$classReflection->getName()];
}
$constructors = [];
if ($classReflection->hasConstructor()) {
$constructors[] = $classReflection->getConstructor()->getName();
}

$nativeReflection = $classReflection->getNativeReflection();
foreach ($this->additionalConstructors as $additionalConstructor) {
[$className, $methodName] = explode('::', $additionalConstructor);
if (!$nativeReflection->hasMethod($methodName)) {
continue;
}
$nativeMethod = $nativeReflection->getMethod($methodName);
if ($nativeMethod->getDeclaringClass()->getName() !== $nativeReflection->getName()) {
continue;
}

try {
$prototype = $nativeMethod->getPrototype();
} catch (ReflectionException) {
$prototype = $nativeMethod;
}

if ($prototype->getDeclaringClass()->getName() !== $className) {
continue;
}

$constructors[] = $methodName;
}

$this->additionalConstructorsCache[$classReflection->getName()] = $constructors;

return $constructors;
}

}
@@ -0,0 +1,60 @@
<?php declare(strict_types = 1);

namespace PHPStan\Rules\Properties;

use PHPStan\Rules\Rule;
use PHPStan\Testing\RuleTestCase;
use const PHP_VERSION_ID;

/**
* @extends RuleTestCase<MissingReadOnlyPropertyAssignRule>
*/
class MissingReadOnlyPropertyAssignRuleTest extends RuleTestCase
{

protected function getRule(): Rule
{
return new MissingReadOnlyPropertyAssignRule([
'MissingReadOnlyPropertyAssign\\TestCase::setUp',
]);
}

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

$this->analyse([__DIR__ . '/data/missing-readonly-property-assign.php'], [
[
'Class MissingReadOnlyPropertyAssign\Foo has an uninitialized readonly property $unassigned. Assign it in the constructor.',
14,
],
[
'Class MissingReadOnlyPropertyAssign\Foo has an uninitialized readonly property $unassigned2. Assign it in the constructor.',
16,
],
[
'Access to an uninitialized readonly property MissingReadOnlyPropertyAssign\Foo::$readBeforeAssigned.',
33,
],
[
'Readonly property MissingReadOnlyPropertyAssign\Foo::$doubleAssigned is already assigned.',
37,
],
[
'Class MissingReadOnlyPropertyAssign\BarDoubleAssignInSetter has an uninitialized readonly property $foo. Assign it in the constructor.',
53,
],
[
'Access to an uninitialized readonly property MissingReadOnlyPropertyAssign\AssignOp::$foo.',
85,
],
[
'Access to an uninitialized readonly property MissingReadOnlyPropertyAssign\AssignOp::$bar.',
87,
],
]);
}

}
@@ -0,0 +1,103 @@
<?php // lint >= 8.1

namespace MissingReadOnlyPropertyAssign;

class Foo
{

private readonly int $assigned;

private int $unassignedButNotReadOnly;

private int $readBeforeAssignedNotReadOnly;

private readonly int $unassigned;

private readonly int $unassigned2;

private readonly int $readBeforeAssigned;

private readonly int $doubleAssigned;

private int $doubleAssignedNotReadOnly;

public function __construct(
private readonly int $promoted,
)
{
$this->assigned = 1;

echo $this->readBeforeAssignedNotReadOnly;
$this->readBeforeAssignedNotReadOnly = 1;

echo $this->readBeforeAssigned;
$this->readBeforeAssigned = 1;

$this->doubleAssigned = 1;
$this->doubleAssigned = 2;

$this->doubleAssignedNotReadOnly = 1;
$this->doubleAssignedNotReadOnly = 2;
}

public function setUnassigned2(int $i): void
{
$this->unassigned2 = $i;
}

}

class BarDoubleAssignInSetter
{

private readonly int $foo;

public function setFoo(int $i)
{
// reported in ReadOnlyPropertyAssignRule
$this->foo = $i;
$this->foo = $i;
}

}

class TestCase
{

private readonly int $foo;

protected function setUp(): void
{
$this->foo = 1;
}

}

class AssignOp
{

private readonly int $foo;

private readonly ?int $bar;

public function __construct(int $foo)
{
$this->foo .= $foo;

$this->bar ??= 3;
}


}

class AssignRef
{

private readonly int $foo;

public function __construct(int $foo)
{
$this->foo = &$foo;
}

}

0 comments on commit 44fd938

Please sign in to comment.