Skip to content

Commit 1f21639

Browse files
lookymanondrejmirtes
lookyman
authored and
ondrejmirtes
committed
Allow nullable property typehints for generated value column
1 parent 554f44f commit 1f21639

File tree

8 files changed

+138
-12
lines changed

8 files changed

+138
-12
lines changed

‎phpcs.xml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
<exclude name="SlevomatCodingStandard.Namespaces.FullyQualifiedExceptions"/>
88
<exclude name="SlevomatCodingStandard.Namespaces.ReferenceUsedNamesOnly"/>
99
<exclude name="Consistence.Exceptions.ExceptionDeclaration"/>
10+
<exclude name="Squiz.Commenting.FunctionComment.MissingParamTag"/>
1011
</rule>
1112
<rule ref="SlevomatCodingStandard.Namespaces.AlphabeticallySortedUses">
1213
<properties>

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

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
use PHPStan\Type\Doctrine\ObjectMetadataResolver;
1212
use PHPStan\Type\TypeCombinator;
1313
use PHPStan\Type\VerbosityLevel;
14+
use Throwable;
1415
use function sprintf;
1516

1617
class EntityColumnRule implements Rule
@@ -81,18 +82,32 @@ public function processNode(Node $node, Scope $scope): array
8182
return [];
8283
}
8384

85+
$identifier = null;
86+
if ($metadata->generatorType !== 5) { // ClassMetadataInfo::GENERATOR_TYPE_NONE
87+
try {
88+
$identifier = $metadata->getSingleIdentifierFieldName();
89+
} catch (Throwable $e) {
90+
$mappingException = 'Doctrine\ORM\Mapping\MappingException';
91+
if (!$e instanceof $mappingException) {
92+
throw $e;
93+
}
94+
}
95+
}
96+
8497
$writableToPropertyType = $descriptor->getWritableToPropertyType();
8598
$writableToDatabaseType = $descriptor->getWritableToDatabaseType();
86-
if ($fieldMapping['nullable'] === true) {
99+
$nullable = $fieldMapping['nullable'] === true;
100+
if ($nullable) {
87101
$writableToPropertyType = TypeCombinator::addNull($writableToPropertyType);
88102
$writableToDatabaseType = TypeCombinator::addNull($writableToDatabaseType);
89103
}
90104

91105
if (!$property->getWritableType()->isSuperTypeOf($writableToPropertyType)->yes()) {
92106
$errors[] = sprintf('Database can contain %s but property expects %s.', $writableToPropertyType->describe(VerbosityLevel::typeOnly()), $property->getWritableType()->describe(VerbosityLevel::typeOnly()));
93107
}
94-
if (!$writableToDatabaseType->isSuperTypeOf($property->getReadableType())->yes()) {
95-
$errors[] = sprintf('Property can contain %s but database expects %s.', $property->getReadableType()->describe(VerbosityLevel::typeOnly()), $writableToDatabaseType->describe(VerbosityLevel::typeOnly()));
108+
$propertyReadableType = $property->getReadableType();
109+
if (!$writableToDatabaseType->isSuperTypeOf($identifier === $propertyName && !$nullable ? TypeCombinator::removeNull($propertyReadableType) : $propertyReadableType)->yes()) {
110+
$errors[] = sprintf('Property can contain %s but database expects %s.', $propertyReadableType->describe(VerbosityLevel::typeOnly()), $writableToDatabaseType->describe(VerbosityLevel::typeOnly()));
96111
}
97112
return $errors;
98113
}

‎tests/Rules/Doctrine/ORM/EntityColumnRuleTest.php

Lines changed: 32 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
namespace PHPStan\Rules\Doctrine\ORM;
44

5+
use Iterator;
56
use PHPStan\Rules\Rule;
67
use PHPStan\Testing\RuleTestCase;
78
use PHPStan\Type\Doctrine\DescriptorRegistry;
@@ -31,31 +32,30 @@ protected function getRule(): Rule
3132

3233
public function testRule(): void
3334
{
34-
require_once __DIR__ . '/data/MyBrokenSuperclass.php';
3535
$this->analyse([__DIR__ . '/data/MyBrokenEntity.php'], [
3636
[
37-
'Database can contain string but property expects int.',
38-
17,
37+
'Database can contain string but property expects int|null.',
38+
19,
3939
],
4040
[
4141
'Database can contain string|null but property expects string.',
42-
23,
42+
25,
4343
],
4444
[
4545
'Property can contain string|null but database expects string.',
46-
29,
46+
31,
4747
],
4848
[
4949
'Database can contain DateTime but property expects DateTimeImmutable.',
50-
35,
50+
37,
5151
],
5252
[
5353
'Database can contain DateTimeImmutable but property expects DateTime.',
54-
41,
54+
43,
5555
],
5656
[
5757
'Property can contain DateTime but database expects DateTimeImmutable.',
58-
41,
58+
43,
5959
],
6060
]);
6161
}
@@ -70,4 +70,28 @@ public function testSuperclass(): void
7070
]);
7171
}
7272

73+
/**
74+
* @dataProvider generatedIdsProvider
75+
*/
76+
public function testGeneratedIds(string $file, array $expectedErrors): void
77+
{
78+
$this->analyse([$file], $expectedErrors);
79+
}
80+
81+
public function generatedIdsProvider(): Iterator
82+
{
83+
yield 'not nullable' => [__DIR__ . '/data/GeneratedIdEntity1.php', []];
84+
yield 'nullable column' => [
85+
__DIR__ . '/data/GeneratedIdEntity2.php',
86+
[
87+
[
88+
'Database can contain string|null but property expects string.',
89+
19,
90+
],
91+
],
92+
];
93+
yield 'nullable property' => [__DIR__ . '/data/GeneratedIdEntity3.php', []];
94+
yield 'nullable both' => [__DIR__ . '/data/GeneratedIdEntity4.php', []];
95+
}
96+
7397
}

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

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace PHPStan\Rules\Doctrine\ORM;
4+
5+
use Doctrine\ORM\Mapping as ORM;
6+
7+
/**
8+
* @ORM\Entity()
9+
*/
10+
class GeneratedIdEntity1
11+
{
12+
13+
/**
14+
* @ORM\Id()
15+
* @ORM\GeneratedValue()
16+
* @ORM\Column(type="bigint")
17+
* @var string
18+
*/
19+
private $id;
20+
21+
}

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

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace PHPStan\Rules\Doctrine\ORM;
4+
5+
use Doctrine\ORM\Mapping as ORM;
6+
7+
/**
8+
* @ORM\Entity()
9+
*/
10+
class GeneratedIdEntity2
11+
{
12+
13+
/**
14+
* @ORM\Id()
15+
* @ORM\GeneratedValue()
16+
* @ORM\Column(type="bigint", nullable=true)
17+
* @var string
18+
*/
19+
private $id;
20+
21+
}

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

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace PHPStan\Rules\Doctrine\ORM;
4+
5+
use Doctrine\ORM\Mapping as ORM;
6+
7+
/**
8+
* @ORM\Entity()
9+
*/
10+
class GeneratedIdEntity3
11+
{
12+
13+
/**
14+
* @ORM\Id()
15+
* @ORM\GeneratedValue()
16+
* @ORM\Column(type="bigint")
17+
* @var string|null
18+
*/
19+
private $id;
20+
21+
}

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

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace PHPStan\Rules\Doctrine\ORM;
4+
5+
use Doctrine\ORM\Mapping as ORM;
6+
7+
/**
8+
* @ORM\Entity()
9+
*/
10+
class GeneratedIdEntity4
11+
{
12+
13+
/**
14+
* @ORM\Id()
15+
* @ORM\GeneratedValue()
16+
* @ORM\Column(type="bigint", nullable=true)
17+
* @var string|null
18+
*/
19+
private $id;
20+
21+
}

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

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,10 +9,12 @@
99
*/
1010
class MyBrokenEntity extends MyBrokenSuperclass
1111
{
12+
1213
/**
1314
* @ORM\Id()
15+
* @ORM\GeneratedValue()
1416
* @ORM\Column(type="bigint")
15-
* @var int
17+
* @var int|null
1618
*/
1719
private $id;
1820

0 commit comments

Comments
 (0)