Skip to content

Commit 50f8e49

Browse files
ondrejmirtesondrejmirtes
committed
Properties might be covariant or contravariant, depending on the allowed operations on the parent
1 parent b3ca610 commit 50f8e49

File tree

3 files changed

+215
-10
lines changed

3 files changed

+215
-10
lines changed

‎src/Rules/Properties/OverridingPropertyRule.php

+80-9
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
use PhpParser\Node;
66
use PHPStan\Analyser\Scope;
77
use PHPStan\Node\ClassPropertyNode;
8+
use PHPStan\Php\PhpVersion;
89
use PHPStan\Reflection\ClassReflection;
910
use PHPStan\Reflection\Php\PhpPropertyReflection;
1011
use PHPStan\Rules\Rule;
@@ -21,6 +22,7 @@ final class OverridingPropertyRule implements Rule
2122
{
2223

2324
public function __construct(
25+
private PhpVersion $phpVersion,
2426
private bool $checkPhpDocMethodSignatures,
2527
private bool $reportMaybes,
2628
)
@@ -129,15 +131,45 @@ public function processNode(Node $node, Scope $scope): array
129131
))->identifier('property.missingNativeType')->nonIgnorable()->build();
130132
} else {
131133
if (!$prototype->getNativeType()->equals($nativeType)) {
132-
$typeErrors[] = RuleErrorBuilder::message(sprintf(
133-
'Type %s of property %s::$%s is not the same as type %s of overridden property %s::$%s.',
134-
$nativeType->describe(VerbosityLevel::typeOnly()),
135-
$classReflection->getDisplayName(),
136-
$node->getName(),
137-
$prototype->getNativeType()->describe(VerbosityLevel::typeOnly()),
138-
$prototype->getDeclaringClass()->getDisplayName(),
139-
$node->getName(),
140-
))->identifier('property.nativeType')->nonIgnorable()->build();
134+
if (
135+
$this->phpVersion->supportsPropertyHooks()
136+
&& ($prototype->isVirtual()->yes() || $prototype->isAbstract()->yes())
137+
&& (!$prototype->isReadable() || !$prototype->isWritable())
138+
) {
139+
if (!$prototype->isReadable()) {
140+
if (!$nativeType->isSuperTypeOf($prototype->getNativeType())->yes()) {
141+
$typeErrors[] = RuleErrorBuilder::message(sprintf(
142+
'Type %s of property %s::$%s is not contravariant with type %s of overridden property %s::$%s.',
143+
$nativeType->describe(VerbosityLevel::typeOnly()),
144+
$classReflection->getDisplayName(),
145+
$node->getName(),
146+
$prototype->getNativeType()->describe(VerbosityLevel::typeOnly()),
147+
$prototype->getDeclaringClass()->getDisplayName(),
148+
$node->getName(),
149+
))->identifier('property.nativeType')->nonIgnorable()->build();
150+
}
151+
} elseif (!$prototype->getNativeType()->isSuperTypeOf($nativeType)->yes()) {
152+
$typeErrors[] = RuleErrorBuilder::message(sprintf(
153+
'Type %s of property %s::$%s is not covariant with type %s of overridden property %s::$%s.',
154+
$nativeType->describe(VerbosityLevel::typeOnly()),
155+
$classReflection->getDisplayName(),
156+
$node->getName(),
157+
$prototype->getNativeType()->describe(VerbosityLevel::typeOnly()),
158+
$prototype->getDeclaringClass()->getDisplayName(),
159+
$node->getName(),
160+
))->identifier('property.nativeType')->nonIgnorable()->build();
161+
}
162+
} else {
163+
$typeErrors[] = RuleErrorBuilder::message(sprintf(
164+
'Type %s of property %s::$%s is not the same as type %s of overridden property %s::$%s.',
165+
$nativeType->describe(VerbosityLevel::typeOnly()),
166+
$classReflection->getDisplayName(),
167+
$node->getName(),
168+
$prototype->getNativeType()->describe(VerbosityLevel::typeOnly()),
169+
$prototype->getDeclaringClass()->getDisplayName(),
170+
$node->getName(),
171+
))->identifier('property.nativeType')->nonIgnorable()->build();
172+
}
141173
}
142174
}
143175
} elseif ($nativeType !== null) {
@@ -167,6 +199,45 @@ public function processNode(Node $node, Scope $scope): array
167199
}
168200

169201
$verbosity = VerbosityLevel::getRecommendedLevelByType($prototype->getReadableType(), $propertyReflection->getReadableType());
202+
203+
if (
204+
$this->phpVersion->supportsPropertyHooks()
205+
&& ($prototype->isVirtual()->yes() || $prototype->isAbstract()->yes())
206+
&& (!$prototype->isReadable() || !$prototype->isWritable())
207+
) {
208+
if (!$prototype->isReadable()) {
209+
if (!$propertyReflection->getReadableType()->isSuperTypeOf($prototype->getReadableType())->yes()) {
210+
$errors[] = RuleErrorBuilder::message(sprintf(
211+
'PHPDoc type %s of property %s::$%s is not contravariant with PHPDoc type %s of overridden property %s::$%s.',
212+
$propertyReflection->getReadableType()->describe($verbosity),
213+
$classReflection->getDisplayName(),
214+
$node->getName(),
215+
$prototype->getReadableType()->describe($verbosity),
216+
$prototype->getDeclaringClass()->getDisplayName(),
217+
$node->getName(),
218+
))->identifier('property.phpDocType')->tip(sprintf(
219+
"You can fix 3rd party PHPDoc types with stub files:\n %s",
220+
'<fg=cyan>https://phpstan.org/user-guide/stub-files</>',
221+
))->build();
222+
}
223+
} elseif (!$prototype->getReadableType()->isSuperTypeOf($propertyReflection->getReadableType())->yes()) {
224+
$errors[] = RuleErrorBuilder::message(sprintf(
225+
'PHPDoc type %s of property %s::$%s is not covariant with PHPDoc type %s of overridden property %s::$%s.',
226+
$propertyReflection->getReadableType()->describe($verbosity),
227+
$classReflection->getDisplayName(),
228+
$node->getName(),
229+
$prototype->getReadableType()->describe($verbosity),
230+
$prototype->getDeclaringClass()->getDisplayName(),
231+
$node->getName(),
232+
))->identifier('property.phpDocType')->tip(sprintf(
233+
"You can fix 3rd party PHPDoc types with stub files:\n %s",
234+
'<fg=cyan>https://phpstan.org/user-guide/stub-files</>',
235+
))->build();
236+
}
237+
238+
return $errors;
239+
}
240+
170241
$isSuperType = $prototype->getReadableType()->isSuperTypeOf($propertyReflection->getReadableType());
171242
$canBeTurnedOffError = RuleErrorBuilder::message(sprintf(
172243
'PHPDoc type %s of property %s::$%s is not the same as PHPDoc type %s of overridden property %s::$%s.',

‎tests/PHPStan/Rules/Properties/OverridingPropertyRuleTest.php

+40-1
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
namespace PHPStan\Rules\Properties;
44

5+
use PHPStan\Php\PhpVersion;
56
use PHPStan\Rules\Rule;
67
use PHPStan\Testing\RuleTestCase;
78
use function sprintf;
@@ -17,7 +18,11 @@ class OverridingPropertyRuleTest extends RuleTestCase
1718

1819
protected function getRule(): Rule
1920
{
20-
return new OverridingPropertyRule(true, $this->reportMaybes);
21+
return new OverridingPropertyRule(
22+
self::getContainer()->getByType(PhpVersion::class),
23+
true,
24+
$this->reportMaybes,
25+
);
2126
}
2227

2328
public function testRule(): void
@@ -214,4 +219,38 @@ public function testPropertyPrototypeFromInterface(): void
214219
]);
215220
}
216221

222+
public function testBug12466(): void
223+
{
224+
if (PHP_VERSION_ID < 80400) {
225+
$this->markTestSkipped('Test requires PHP 8.4.');
226+
}
227+
228+
$tip = sprintf(
229+
"You can fix 3rd party PHPDoc types with stub files:\n %s",
230+
'<fg=cyan>https://phpstan.org/user-guide/stub-files</>',
231+
);
232+
233+
$this->reportMaybes = true;
234+
$this->analyse([__DIR__ . '/data/bug-12466.php'], [
235+
[
236+
'Type int|string|null of property Bug12466OverridenProperty\Baz::$onlyGet is not covariant with type int|string of overridden property Bug12466OverridenProperty\Foo::$onlyGet.',
237+
34,
238+
],
239+
[
240+
'Type int of property Bug12466OverridenProperty\Baz::$onlySet is not contravariant with type int|string of overridden property Bug12466OverridenProperty\Foo::$onlySet.',
241+
40,
242+
],
243+
[
244+
'PHPDoc type array<int|string|null> of property Bug12466OverridenProperty\BazWithPhpDocs::$onlyGet is not covariant with PHPDoc type array<int|string> of overridden property Bug12466OverridenProperty\FooWithPhpDocs::$onlyGet.',
245+
82,
246+
$tip,
247+
],
248+
[
249+
'PHPDoc type array<int> of property Bug12466OverridenProperty\BazWithPhpDocs::$onlySet is not contravariant with PHPDoc type array<int|string> of overridden property Bug12466OverridenProperty\FooWithPhpDocs::$onlySet.',
250+
89,
251+
$tip,
252+
],
253+
]);
254+
}
255+
217256
}

‎tests/PHPStan/Rules/Properties/data/bug-12466.php

+95
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,95 @@
1+
<?php // lint >= 8.4
2+
3+
namespace Bug12466OverridenProperty;
4+
5+
interface Foo
6+
{
7+
8+
public int|string $onlyGet { get; }
9+
10+
public int|string $onlySet { set; }
11+
12+
}
13+
14+
class Bar implements Foo
15+
{
16+
17+
public int $onlyGet {
18+
get {
19+
return 1;
20+
}
21+
}
22+
23+
public int|string|null $onlySet {
24+
set {
25+
$this->onlySet = $value;
26+
}
27+
}
28+
29+
}
30+
31+
class Baz implements Foo
32+
{
33+
34+
public int|string|null $onlyGet {
35+
get {
36+
return null;
37+
}
38+
}
39+
40+
public int $onlySet {
41+
set {
42+
$this->onlySet = $value;
43+
}
44+
}
45+
46+
}
47+
48+
interface FooWithPhpDocs
49+
{
50+
51+
/** @var array<int|string> */
52+
public array $onlyGet { get; }
53+
54+
/** @var array<int|string> */
55+
public array $onlySet { set; }
56+
57+
}
58+
59+
class BarWithPhpDocs implements FooWithPhpDocs
60+
{
61+
62+
/** @var array<int> */
63+
public array $onlyGet {
64+
get {
65+
return [];
66+
}
67+
}
68+
69+
/** @var array<int|string|null> */
70+
public array $onlySet {
71+
set {
72+
$this->onlySet = $value;
73+
}
74+
}
75+
76+
}
77+
78+
class BazWithPhpDocs implements FooWithPhpDocs
79+
{
80+
81+
/** @var array<int|string|null> */
82+
public array $onlyGet {
83+
get {
84+
return [];
85+
}
86+
}
87+
88+
/** @var array<int> */
89+
public array $onlySet {
90+
set {
91+
$this->onlySet = $value;
92+
}
93+
}
94+
95+
}

0 commit comments

Comments
 (0)