diff --git a/phpcs.xml b/phpcs.xml index b2f5f0b0..4b572918 100644 --- a/phpcs.xml +++ b/phpcs.xml @@ -7,6 +7,7 @@ + diff --git a/src/Rules/Doctrine/ORM/EntityColumnRule.php b/src/Rules/Doctrine/ORM/EntityColumnRule.php index d909b3c1..68eec9b9 100644 --- a/src/Rules/Doctrine/ORM/EntityColumnRule.php +++ b/src/Rules/Doctrine/ORM/EntityColumnRule.php @@ -11,6 +11,7 @@ use PHPStan\Type\Doctrine\ObjectMetadataResolver; use PHPStan\Type\TypeCombinator; use PHPStan\Type\VerbosityLevel; +use Throwable; use function sprintf; class EntityColumnRule implements Rule @@ -81,9 +82,22 @@ public function processNode(Node $node, Scope $scope): array return []; } + $identifier = null; + if ($metadata->generatorType !== 5) { // ClassMetadataInfo::GENERATOR_TYPE_NONE + try { + $identifier = $metadata->getSingleIdentifierFieldName(); + } catch (Throwable $e) { + $mappingException = 'Doctrine\ORM\Mapping\MappingException'; + if (!$e instanceof $mappingException) { + throw $e; + } + } + } + $writableToPropertyType = $descriptor->getWritableToPropertyType(); $writableToDatabaseType = $descriptor->getWritableToDatabaseType(); - if ($fieldMapping['nullable'] === true) { + $nullable = $fieldMapping['nullable'] === true; + if ($nullable) { $writableToPropertyType = TypeCombinator::addNull($writableToPropertyType); $writableToDatabaseType = TypeCombinator::addNull($writableToDatabaseType); } @@ -91,8 +105,9 @@ public function processNode(Node $node, Scope $scope): array if (!$property->getWritableType()->isSuperTypeOf($writableToPropertyType)->yes()) { $errors[] = sprintf('Database can contain %s but property expects %s.', $writableToPropertyType->describe(VerbosityLevel::typeOnly()), $property->getWritableType()->describe(VerbosityLevel::typeOnly())); } - if (!$writableToDatabaseType->isSuperTypeOf($property->getReadableType())->yes()) { - $errors[] = sprintf('Property can contain %s but database expects %s.', $property->getReadableType()->describe(VerbosityLevel::typeOnly()), $writableToDatabaseType->describe(VerbosityLevel::typeOnly())); + $propertyReadableType = $property->getReadableType(); + if (!$writableToDatabaseType->isSuperTypeOf($identifier === $propertyName && !$nullable ? TypeCombinator::removeNull($propertyReadableType) : $propertyReadableType)->yes()) { + $errors[] = sprintf('Property can contain %s but database expects %s.', $propertyReadableType->describe(VerbosityLevel::typeOnly()), $writableToDatabaseType->describe(VerbosityLevel::typeOnly())); } return $errors; } diff --git a/tests/Rules/Doctrine/ORM/EntityColumnRuleTest.php b/tests/Rules/Doctrine/ORM/EntityColumnRuleTest.php index fa366352..d6683fe7 100644 --- a/tests/Rules/Doctrine/ORM/EntityColumnRuleTest.php +++ b/tests/Rules/Doctrine/ORM/EntityColumnRuleTest.php @@ -2,6 +2,7 @@ namespace PHPStan\Rules\Doctrine\ORM; +use Iterator; use PHPStan\Rules\Rule; use PHPStan\Testing\RuleTestCase; use PHPStan\Type\Doctrine\DescriptorRegistry; @@ -31,31 +32,30 @@ protected function getRule(): Rule public function testRule(): void { - require_once __DIR__ . '/data/MyBrokenSuperclass.php'; $this->analyse([__DIR__ . '/data/MyBrokenEntity.php'], [ [ - 'Database can contain string but property expects int.', - 17, + 'Database can contain string but property expects int|null.', + 19, ], [ 'Database can contain string|null but property expects string.', - 23, + 25, ], [ 'Property can contain string|null but database expects string.', - 29, + 31, ], [ 'Database can contain DateTime but property expects DateTimeImmutable.', - 35, + 37, ], [ 'Database can contain DateTimeImmutable but property expects DateTime.', - 41, + 43, ], [ 'Property can contain DateTime but database expects DateTimeImmutable.', - 41, + 43, ], ]); } @@ -70,4 +70,28 @@ public function testSuperclass(): void ]); } + /** + * @dataProvider generatedIdsProvider + */ + public function testGeneratedIds(string $file, array $expectedErrors): void + { + $this->analyse([$file], $expectedErrors); + } + + public function generatedIdsProvider(): Iterator + { + yield 'not nullable' => [__DIR__ . '/data/GeneratedIdEntity1.php', []]; + yield 'nullable column' => [ + __DIR__ . '/data/GeneratedIdEntity2.php', + [ + [ + 'Database can contain string|null but property expects string.', + 19, + ], + ], + ]; + yield 'nullable property' => [__DIR__ . '/data/GeneratedIdEntity3.php', []]; + yield 'nullable both' => [__DIR__ . '/data/GeneratedIdEntity4.php', []]; + } + } diff --git a/tests/Rules/Doctrine/ORM/data/GeneratedIdEntity1.php b/tests/Rules/Doctrine/ORM/data/GeneratedIdEntity1.php new file mode 100644 index 00000000..28006ef5 --- /dev/null +++ b/tests/Rules/Doctrine/ORM/data/GeneratedIdEntity1.php @@ -0,0 +1,21 @@ +