diff --git a/composer.json b/composer.json index a31a0059..ef81194f 100644 --- a/composer.json +++ b/composer.json @@ -7,7 +7,7 @@ ], "require": { "php": "^7.2 || ^8.0", - "phpstan/phpstan": "^1.7.0" + "phpstan/phpstan": "^1.7.3" }, "conflict": { "doctrine/collections": "<1.0", diff --git a/src/Rules/Doctrine/ORM/PropertiesExtension.php b/src/Rules/Doctrine/ORM/PropertiesExtension.php index 7e622040..7328d6f4 100644 --- a/src/Rules/Doctrine/ORM/PropertiesExtension.php +++ b/src/Rules/Doctrine/ORM/PropertiesExtension.php @@ -2,6 +2,7 @@ namespace PHPStan\Rules\Doctrine\ORM; +use Doctrine\ORM\Mapping\ClassMetadataInfo; use PHPStan\Reflection\PropertyReflection; use PHPStan\Rules\Properties\ReadWritePropertiesExtension; use PHPStan\Type\Doctrine\ObjectMetadataResolver; @@ -47,26 +48,11 @@ public function isAlwaysWritten(PropertyReflection $property, string $propertyNa return true; } - if ($metadata->isIdentifierNatural()) { - return false; - } - if ($metadata->versionField === $propertyName) { return true; } - try { - $identifiers = $metadata->getIdentifierFieldNames(); - } catch (Throwable $e) { - $mappingException = 'Doctrine\ORM\Mapping\MappingException'; - if (!$e instanceof $mappingException) { - throw $e; - } - - return false; - } - - return in_array($propertyName, $identifiers, true); + return $this->isGeneratedIdentifier($metadata, $propertyName); } public function isInitialized(PropertyReflection $property, string $propertyName): bool @@ -82,7 +68,29 @@ public function isInitialized(PropertyReflection $property, string $propertyName return false; } + if ($this->isGeneratedIdentifier($metadata, $propertyName)) { + return true; + } + return $metadata->isReadOnly && !$declaringClass->hasConstructor(); } + private function isGeneratedIdentifier(ClassMetadataInfo $metadata, string $propertyName): bool + { + if ($metadata->isIdentifierNatural()) { + return false; + } + + try { + return in_array($propertyName, $metadata->getIdentifierFieldNames(), true); + } catch (Throwable $e) { + $mappingException = 'Doctrine\ORM\Mapping\MappingException'; + if (!$e instanceof $mappingException) { + throw $e; + } + + return false; + } + } + } diff --git a/tests/Rules/DeadCode/UnusedPrivatePropertyRuleTest.php b/tests/Rules/DeadCode/UnusedPrivatePropertyRuleTest.php new file mode 100644 index 00000000..ccef2e9c --- /dev/null +++ b/tests/Rules/DeadCode/UnusedPrivatePropertyRuleTest.php @@ -0,0 +1,50 @@ + + */ +class UnusedPrivatePropertyRuleTest extends RuleTestCase +{ + + protected function getRule(): Rule + { + return self::getContainer()->getByType(UnusedPrivatePropertyRule::class); + } + + public static function getAdditionalConfigFiles(): array + { + return [__DIR__ . '/../../../extension.neon']; + } + + public function testRule(): void + { + if (PHP_VERSION_ID < 70400) { + self::markTestSkipped('Test requires PHP 7.4.'); + } + + $this->analyse([__DIR__ . '/data/unused-private-property.php'], [ + [ + 'Property UnusedPrivateProperty\EntityWithAGeneratedId::$unused is never written, only read.', + 23, + 'See: https://phpstan.org/developing-extensions/always-read-written-properties', + ], + [ + 'Property UnusedPrivateProperty\EntityWithAGeneratedId::$unused2 is unused.', + 25, + 'See: https://phpstan.org/developing-extensions/always-read-written-properties', + ], + [ + 'Property UnusedPrivateProperty\ReadOnlyEntityWithConstructor::$id is never written, only read.', + 53, + 'See: https://phpstan.org/developing-extensions/always-read-written-properties', + ], + ]); + } + +} diff --git a/tests/Rules/DeadCode/data/unused-private-property.php b/tests/Rules/DeadCode/data/unused-private-property.php new file mode 100644 index 00000000..88aca7cf --- /dev/null +++ b/tests/Rules/DeadCode/data/unused-private-property.php @@ -0,0 +1,59 @@ += 7.4 + +namespace UnusedPrivateProperty; + +use Doctrine\ORM\Mapping as ORM; + +/** + * @ORM\Entity + */ +class EntityWithAGeneratedId +{ + + /** + * @ORM\Id + * @ORM\GeneratedValue + * @ORM\Column + */ + private int $id; // ok, ID is generated + + /** + * @ORM\Column + */ + private int $unused; + + private int $unused2; + +} + +/** + * @ORM\Entity(readOnly=true) + */ +class ReadOnlyEntity +{ + + /** + * @ORM\Id + * @ORM\Column + */ + private int $id; // ok, entity is read only + +} + +/** + * @ORM\Entity(readOnly=true) + */ +class ReadOnlyEntityWithConstructor +{ + + /** + * @ORM\Id + * @ORM\Column + */ + private int $id; + + public function __construct() + { + } + +} diff --git a/tests/Rules/Properties/MissingReadOnlyByPhpDocPropertyAssignRuleTest.php b/tests/Rules/Properties/MissingReadOnlyByPhpDocPropertyAssignRuleTest.php new file mode 100644 index 00000000..2f42a9a4 --- /dev/null +++ b/tests/Rules/Properties/MissingReadOnlyByPhpDocPropertyAssignRuleTest.php @@ -0,0 +1,47 @@ + + */ +class MissingReadOnlyByPhpDocPropertyAssignRuleTest extends RuleTestCase +{ + + protected function getRule(): Rule + { + return self::getContainer()->getByType(MissingReadOnlyByPhpDocPropertyAssignRule::class); + } + + public static function getAdditionalConfigFiles(): array + { + return [__DIR__ . '/../../../extension.neon']; + } + + public function testRule(): void + { + if (PHP_VERSION_ID < 70400) { + self::markTestSkipped('Test requires PHP 7.4.'); + } + + $this->analyse([__DIR__ . '/data/missing-readonly-property-assign-phpdoc.php'], [ + [ + 'Class MissingReadOnlyPropertyAssignPhpDoc\EntityWithAGeneratedId has an uninitialized @readonly property $unassigned. Assign it in the constructor.', + 25, + ], + [ + '@readonly property MissingReadOnlyPropertyAssignPhpDoc\EntityWithAGeneratedId::$doubleAssigned is already assigned.', + 36, + ], + [ + 'Class MissingReadOnlyPropertyAssignPhpDoc\ReadOnlyEntityWithConstructor has an uninitialized @readonly property $id. Assign it in the constructor.', + 67, + ], + ]); + } + +} diff --git a/tests/Rules/Properties/MissingReadOnlyPropertyAssignRuleTest.php b/tests/Rules/Properties/MissingReadOnlyPropertyAssignRuleTest.php new file mode 100644 index 00000000..6f5c545f --- /dev/null +++ b/tests/Rules/Properties/MissingReadOnlyPropertyAssignRuleTest.php @@ -0,0 +1,47 @@ + + */ +class MissingReadOnlyPropertyAssignRuleTest extends RuleTestCase +{ + + protected function getRule(): Rule + { + return self::getContainer()->getByType(MissingReadOnlyPropertyAssignRule::class); + } + + public static function getAdditionalConfigFiles(): array + { + return [__DIR__ . '/../../../extension.neon']; + } + + public function testRule(): void + { + if (PHP_VERSION_ID < 80100) { + self::markTestSkipped('Test requires PHP 8.1.'); + } + + $this->analyse([__DIR__ . '/data/missing-readonly-property-assign.php'], [ + [ + 'Class MissingReadOnlyPropertyAssign\EntityWithAGeneratedId has an uninitialized readonly property $unassigned. Assign it in the constructor.', + 17, + ], + [ + 'Readonly property MissingReadOnlyPropertyAssign\EntityWithAGeneratedId::$doubleAssigned is already assigned.', + 25, + ], + [ + 'Class MissingReadOnlyPropertyAssign\ReadOnlyEntityWithConstructor has an uninitialized readonly property $id. Assign it in the constructor.', + 46, + ], + ]); + } + +} diff --git a/tests/Rules/Properties/data/missing-readonly-property-assign-phpdoc.php b/tests/Rules/Properties/data/missing-readonly-property-assign-phpdoc.php new file mode 100644 index 00000000..16f9d556 --- /dev/null +++ b/tests/Rules/Properties/data/missing-readonly-property-assign-phpdoc.php @@ -0,0 +1,73 @@ += 7.4 + +namespace MissingReadOnlyPropertyAssignPhpDoc; + +use Doctrine\ORM\Mapping as ORM; + +/** + * @ORM\Entity + */ +class EntityWithAGeneratedId +{ + + /** + * @ORM\Id + * @ORM\GeneratedValue + * @ORM\Column + * @readonly + */ + private int $id; // ok, ID is generated + + /** + * @ORM\Column + * @readonly + */ + private int $unassigned; + + /** + * @ORM\Column + * @readonly + */ + private int $doubleAssigned; + + public function __construct(int $doubleAssigned) + { + $this->doubleAssigned = $doubleAssigned; + $this->doubleAssigned = 17; + } + +} + +/** + * @ORM\Entity(readOnly=true) + */ +class ReadOnlyEntity +{ + + /** + * @ORM\Id + * @ORM\Column + * @readonly + */ + private int $id; // ok, entity is read only + +} + +/** + * @ORM\Entity(readOnly=true) + */ +class ReadOnlyEntityWithConstructor +{ + + /** + * @ORM\Id + * @ORM\Column + * @readonly + */ + private int $id; + + public function __construct() + { + } + +} diff --git a/tests/Rules/Properties/data/missing-readonly-property-assign.php b/tests/Rules/Properties/data/missing-readonly-property-assign.php new file mode 100644 index 00000000..edbac4d7 --- /dev/null +++ b/tests/Rules/Properties/data/missing-readonly-property-assign.php @@ -0,0 +1,52 @@ += 8.1 + +namespace MissingReadOnlyPropertyAssign; + +use Doctrine\ORM\Mapping as ORM; + +#[ORM\Entity] +class EntityWithAGeneratedId +{ + + #[ORM\Id] + #[ORM\GeneratedValue] + #[ORM\Column] + private readonly int $id; // ok, ID is generated + + #[ORM\Column] + private readonly int $unassigned; + + #[ORM\Column] + private readonly int $doubleAssigned; + + public function __construct(int $doubleAssigned) + { + $this->doubleAssigned = $doubleAssigned; + $this->doubleAssigned = 17; + } + +} + +#[ORM\Entity(null, true)] +class ReadOnlyEntity +{ + + #[ORM\Id] + #[ORM\Column] + private readonly int $id; // ok, entity is readonly + +} + +#[ORM\Entity(null, true)] +class ReadOnlyEntityWithConstructor +{ + + #[ORM\Id] + #[ORM\Column] + private readonly int $id; + + public function __construct() + { + } + +}