Allow nullable property typehints for generated value column

Author: lookyman

phpcs.xml

@@ -7,6 +7,7 @@
 		<exclude name="SlevomatCodingStandard.Namespaces.FullyQualifiedExceptions"/>
 		<exclude name="SlevomatCodingStandard.Namespaces.ReferenceUsedNamesOnly"/>
 		<exclude name="Consistence.Exceptions.ExceptionDeclaration"/>
+		<exclude name="Squiz.Commenting.FunctionComment.MissingParamTag"/>
 	</rule>
 	<rule ref="SlevomatCodingStandard.Namespaces.AlphabeticallySortedUses">
 		<properties>

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,18 +82,32 @@ 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);
 		}
 
 		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;
 	}

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', []];
+	}
+
 }

tests/Rules/Doctrine/ORM/data/GeneratedIdEntity1.php

@@ -0,0 +1,21 @@
+<?php declare(strict_types = 1);
+
+namespace PHPStan\Rules\Doctrine\ORM;
+
+use Doctrine\ORM\Mapping as ORM;
+
+/**
+ * @ORM\Entity()
+ */
+class GeneratedIdEntity1
+{
+
+	/**
+	 * @ORM\Id()
+	 * @ORM\GeneratedValue()
+	 * @ORM\Column(type="bigint")
+	 * @var string
+	 */
+	private $id;
+
+}

tests/Rules/Doctrine/ORM/data/GeneratedIdEntity2.php

@@ -0,0 +1,21 @@
+<?php declare(strict_types = 1);
+
+namespace PHPStan\Rules\Doctrine\ORM;
+
+use Doctrine\ORM\Mapping as ORM;
+
+/**
+ * @ORM\Entity()
+ */
+class GeneratedIdEntity2
+{
+
+	/**
+	 * @ORM\Id()
+	 * @ORM\GeneratedValue()
+	 * @ORM\Column(type="bigint", nullable=true)
+	 * @var string
+	 */
+	private $id;
+
+}

tests/Rules/Doctrine/ORM/data/GeneratedIdEntity3.php

@@ -0,0 +1,21 @@
+<?php declare(strict_types = 1);
+
+namespace PHPStan\Rules\Doctrine\ORM;
+
+use Doctrine\ORM\Mapping as ORM;
+
+/**
+ * @ORM\Entity()
+ */
+class GeneratedIdEntity3
+{
+
+	/**
+	 * @ORM\Id()
+	 * @ORM\GeneratedValue()
+	 * @ORM\Column(type="bigint")
+	 * @var string|null
+	 */
+	private $id;
+
+}

tests/Rules/Doctrine/ORM/data/GeneratedIdEntity4.php

@@ -0,0 +1,21 @@
+<?php declare(strict_types = 1);
+
+namespace PHPStan\Rules\Doctrine\ORM;
+
+use Doctrine\ORM\Mapping as ORM;
+
+/**
+ * @ORM\Entity()
+ */
+class GeneratedIdEntity4
+{
+
+	/**
+	 * @ORM\Id()
+	 * @ORM\GeneratedValue()
+	 * @ORM\Column(type="bigint", nullable=true)
+	 * @var string|null
+	 */
+	private $id;
+
+}

tests/Rules/Doctrine/ORM/data/MyBrokenEntity.php

@@ -9,10 +9,12 @@
  */
 class MyBrokenEntity extends MyBrokenSuperclass
 {
+
 	/**
 	 * @ORM\Id()
+	 * @ORM\GeneratedValue()
 	 * @ORM\Column(type="bigint")
-	 * @var int
+	 * @var int|null
 	 */
 	private $id;