Skip to content

Commit 85339d7

Browse files
herndlmherndlm
authored
Readonly property with a generated value is considered initialized
1 parent 2879e5f commit 85339d7

8 files changed

+353
-17
lines changed

‎composer.json

+1-1
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
],
88
"require": {
99
"php": "^7.2 || ^8.0",
10-
"phpstan/phpstan": "^1.7.0"
10+
"phpstan/phpstan": "^1.7.3"
1111
},
1212
"conflict": {
1313
"doctrine/collections": "<1.0",

‎src/Rules/Doctrine/ORM/PropertiesExtension.php

+24-16
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
namespace PHPStan\Rules\Doctrine\ORM;
44

5+
use Doctrine\ORM\Mapping\ClassMetadataInfo;
56
use PHPStan\Reflection\PropertyReflection;
67
use PHPStan\Rules\Properties\ReadWritePropertiesExtension;
78
use PHPStan\Type\Doctrine\ObjectMetadataResolver;
@@ -47,26 +48,11 @@ public function isAlwaysWritten(PropertyReflection $property, string $propertyNa
4748
return true;
4849
}
4950

50-
if ($metadata->isIdentifierNatural()) {
51-
return false;
52-
}
53-
5451
if ($metadata->versionField === $propertyName) {
5552
return true;
5653
}
5754

58-
try {
59-
$identifiers = $metadata->getIdentifierFieldNames();
60-
} catch (Throwable $e) {
61-
$mappingException = 'Doctrine\ORM\Mapping\MappingException';
62-
if (!$e instanceof $mappingException) {
63-
throw $e;
64-
}
65-
66-
return false;
67-
}
68-
69-
return in_array($propertyName, $identifiers, true);
55+
return $this->isGeneratedIdentifier($metadata, $propertyName);
7056
}
7157

7258
public function isInitialized(PropertyReflection $property, string $propertyName): bool
@@ -82,7 +68,29 @@ public function isInitialized(PropertyReflection $property, string $propertyName
8268
return false;
8369
}
8470

71+
if ($this->isGeneratedIdentifier($metadata, $propertyName)) {
72+
return true;
73+
}
74+
8575
return $metadata->isReadOnly && !$declaringClass->hasConstructor();
8676
}
8777

78+
private function isGeneratedIdentifier(ClassMetadataInfo $metadata, string $propertyName): bool
79+
{
80+
if ($metadata->isIdentifierNatural()) {
81+
return false;
82+
}
83+
84+
try {
85+
return in_array($propertyName, $metadata->getIdentifierFieldNames(), true);
86+
} catch (Throwable $e) {
87+
$mappingException = 'Doctrine\ORM\Mapping\MappingException';
88+
if (!$e instanceof $mappingException) {
89+
throw $e;
90+
}
91+
92+
return false;
93+
}
94+
}
95+
8896
}

‎tests/Rules/DeadCode/UnusedPrivatePropertyRuleTest.php

+50
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace PHPStan\Rules\DeadCode;
4+
5+
use PHPStan\Rules\Rule;
6+
use PHPStan\Testing\RuleTestCase;
7+
use const PHP_VERSION_ID;
8+
9+
/**
10+
* @extends RuleTestCase<UnusedPrivatePropertyRule>
11+
*/
12+
class UnusedPrivatePropertyRuleTest extends RuleTestCase
13+
{
14+
15+
protected function getRule(): Rule
16+
{
17+
return self::getContainer()->getByType(UnusedPrivatePropertyRule::class);
18+
}
19+
20+
public static function getAdditionalConfigFiles(): array
21+
{
22+
return [__DIR__ . '/../../../extension.neon'];
23+
}
24+
25+
public function testRule(): void
26+
{
27+
if (PHP_VERSION_ID < 70400) {
28+
self::markTestSkipped('Test requires PHP 7.4.');
29+
}
30+
31+
$this->analyse([__DIR__ . '/data/unused-private-property.php'], [
32+
[
33+
'Property UnusedPrivateProperty\EntityWithAGeneratedId::$unused is never written, only read.',
34+
23,
35+
'See: https://phpstan.org/developing-extensions/always-read-written-properties',
36+
],
37+
[
38+
'Property UnusedPrivateProperty\EntityWithAGeneratedId::$unused2 is unused.',
39+
25,
40+
'See: https://phpstan.org/developing-extensions/always-read-written-properties',
41+
],
42+
[
43+
'Property UnusedPrivateProperty\ReadOnlyEntityWithConstructor::$id is never written, only read.',
44+
53,
45+
'See: https://phpstan.org/developing-extensions/always-read-written-properties',
46+
],
47+
]);
48+
}
49+
50+
}

‎tests/Rules/DeadCode/data/unused-private-property.php

+59
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
<?php // lint >= 7.4
2+
3+
namespace UnusedPrivateProperty;
4+
5+
use Doctrine\ORM\Mapping as ORM;
6+
7+
/**
8+
* @ORM\Entity
9+
*/
10+
class EntityWithAGeneratedId
11+
{
12+
13+
/**
14+
* @ORM\Id
15+
* @ORM\GeneratedValue
16+
* @ORM\Column
17+
*/
18+
private int $id; // ok, ID is generated
19+
20+
/**
21+
* @ORM\Column
22+
*/
23+
private int $unused;
24+
25+
private int $unused2;
26+
27+
}
28+
29+
/**
30+
* @ORM\Entity(readOnly=true)
31+
*/
32+
class ReadOnlyEntity
33+
{
34+
35+
/**
36+
* @ORM\Id
37+
* @ORM\Column
38+
*/
39+
private int $id; // ok, entity is read only
40+
41+
}
42+
43+
/**
44+
* @ORM\Entity(readOnly=true)
45+
*/
46+
class ReadOnlyEntityWithConstructor
47+
{
48+
49+
/**
50+
* @ORM\Id
51+
* @ORM\Column
52+
*/
53+
private int $id;
54+
55+
public function __construct()
56+
{
57+
}
58+
59+
}

‎tests/Rules/Properties/MissingReadOnlyByPhpDocPropertyAssignRuleTest.php

+47
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace PHPStan\Rules\Properties;
4+
5+
use PHPStan\Rules\Rule;
6+
use PHPStan\Testing\RuleTestCase;
7+
use const PHP_VERSION_ID;
8+
9+
/**
10+
* @extends RuleTestCase<MissingReadOnlyByPhpDocPropertyAssignRule>
11+
*/
12+
class MissingReadOnlyByPhpDocPropertyAssignRuleTest extends RuleTestCase
13+
{
14+
15+
protected function getRule(): Rule
16+
{
17+
return self::getContainer()->getByType(MissingReadOnlyByPhpDocPropertyAssignRule::class);
18+
}
19+
20+
public static function getAdditionalConfigFiles(): array
21+
{
22+
return [__DIR__ . '/../../../extension.neon'];
23+
}
24+
25+
public function testRule(): void
26+
{
27+
if (PHP_VERSION_ID < 70400) {
28+
self::markTestSkipped('Test requires PHP 7.4.');
29+
}
30+
31+
$this->analyse([__DIR__ . '/data/missing-readonly-property-assign-phpdoc.php'], [
32+
[
33+
'Class MissingReadOnlyPropertyAssignPhpDoc\EntityWithAGeneratedId has an uninitialized @readonly property $unassigned. Assign it in the constructor.',
34+
25,
35+
],
36+
[
37+
'@readonly property MissingReadOnlyPropertyAssignPhpDoc\EntityWithAGeneratedId::$doubleAssigned is already assigned.',
38+
36,
39+
],
40+
[
41+
'Class MissingReadOnlyPropertyAssignPhpDoc\ReadOnlyEntityWithConstructor has an uninitialized @readonly property $id. Assign it in the constructor.',
42+
67,
43+
],
44+
]);
45+
}
46+
47+
}

‎tests/Rules/Properties/MissingReadOnlyPropertyAssignRuleTest.php

+47
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace PHPStan\Rules\Properties;
4+
5+
use PHPStan\Rules\Rule;
6+
use PHPStan\Testing\RuleTestCase;
7+
use const PHP_VERSION_ID;
8+
9+
/**
10+
* @extends RuleTestCase<MissingReadOnlyPropertyAssignRule>
11+
*/
12+
class MissingReadOnlyPropertyAssignRuleTest extends RuleTestCase
13+
{
14+
15+
protected function getRule(): Rule
16+
{
17+
return self::getContainer()->getByType(MissingReadOnlyPropertyAssignRule::class);
18+
}
19+
20+
public static function getAdditionalConfigFiles(): array
21+
{
22+
return [__DIR__ . '/../../../extension.neon'];
23+
}
24+
25+
public function testRule(): void
26+
{
27+
if (PHP_VERSION_ID < 80100) {
28+
self::markTestSkipped('Test requires PHP 8.1.');
29+
}
30+
31+
$this->analyse([__DIR__ . '/data/missing-readonly-property-assign.php'], [
32+
[
33+
'Class MissingReadOnlyPropertyAssign\EntityWithAGeneratedId has an uninitialized readonly property $unassigned. Assign it in the constructor.',
34+
17,
35+
],
36+
[
37+
'Readonly property MissingReadOnlyPropertyAssign\EntityWithAGeneratedId::$doubleAssigned is already assigned.',
38+
25,
39+
],
40+
[
41+
'Class MissingReadOnlyPropertyAssign\ReadOnlyEntityWithConstructor has an uninitialized readonly property $id. Assign it in the constructor.',
42+
46,
43+
],
44+
]);
45+
}
46+
47+
}

‎tests/Rules/Properties/data/missing-readonly-property-assign-phpdoc.php

+73
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,73 @@
1+
<?php // lint >= 7.4
2+
3+
namespace MissingReadOnlyPropertyAssignPhpDoc;
4+
5+
use Doctrine\ORM\Mapping as ORM;
6+
7+
/**
8+
* @ORM\Entity
9+
*/
10+
class EntityWithAGeneratedId
11+
{
12+
13+
/**
14+
* @ORM\Id
15+
* @ORM\GeneratedValue
16+
* @ORM\Column
17+
* @readonly
18+
*/
19+
private int $id; // ok, ID is generated
20+
21+
/**
22+
* @ORM\Column
23+
* @readonly
24+
*/
25+
private int $unassigned;
26+
27+
/**
28+
* @ORM\Column
29+
* @readonly
30+
*/
31+
private int $doubleAssigned;
32+
33+
public function __construct(int $doubleAssigned)
34+
{
35+
$this->doubleAssigned = $doubleAssigned;
36+
$this->doubleAssigned = 17;
37+
}
38+
39+
}
40+
41+
/**
42+
* @ORM\Entity(readOnly=true)
43+
*/
44+
class ReadOnlyEntity
45+
{
46+
47+
/**
48+
* @ORM\Id
49+
* @ORM\Column
50+
* @readonly
51+
*/
52+
private int $id; // ok, entity is read only
53+
54+
}
55+
56+
/**
57+
* @ORM\Entity(readOnly=true)
58+
*/
59+
class ReadOnlyEntityWithConstructor
60+
{
61+
62+
/**
63+
* @ORM\Id
64+
* @ORM\Column
65+
* @readonly
66+
*/
67+
private int $id;
68+
69+
public function __construct()
70+
{
71+
}
72+
73+
}

0 commit comments

Comments
 (0)