Skip to content

Commit

Permalink
Properties might be covariant or contravariant, depending on the allo…
Browse files Browse the repository at this point in the history 
…wed operations on the parent
  • Loading branch information
@ondrejmirtes
ondrejmirtes committed Jan 24, 2025
1 parent b3ca610 commit 50f8e49
Show file tree
Hide file tree
Showing 3 changed files with 215 additions and 10 deletions.
89 changes: 80 additions & 9 deletions src/Rules/Properties/OverridingPropertyRule.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
use PhpParser\Node;
use PHPStan\Analyser\Scope;
use PHPStan\Node\ClassPropertyNode;
use PHPStan\Php\PhpVersion;
use PHPStan\Reflection\ClassReflection;
use PHPStan\Reflection\Php\PhpPropertyReflection;
use PHPStan\Rules\Rule;
Expand All @@ -21,6 +22,7 @@ final class OverridingPropertyRule implements Rule
{

public function __construct(
private PhpVersion $phpVersion,
private bool $checkPhpDocMethodSignatures,
private bool $reportMaybes,
)
Expand Down Expand Up @@ -129,15 +131,45 @@ public function processNode(Node $node, Scope $scope): array
))->identifier('property.missingNativeType')->nonIgnorable()->build();
} else {
if (!$prototype->getNativeType()->equals($nativeType)) {
$typeErrors[] = RuleErrorBuilder::message(sprintf(
'Type %s of property %s::$%s is not the same as type %s of overridden property %s::$%s.',
$nativeType->describe(VerbosityLevel::typeOnly()),
$classReflection->getDisplayName(),
$node->getName(),
$prototype->getNativeType()->describe(VerbosityLevel::typeOnly()),
$prototype->getDeclaringClass()->getDisplayName(),
$node->getName(),
))->identifier('property.nativeType')->nonIgnorable()->build();
if (
$this->phpVersion->supportsPropertyHooks()
&& ($prototype->isVirtual()->yes() || $prototype->isAbstract()->yes())
&& (!$prototype->isReadable() || !$prototype->isWritable())
) {
if (!$prototype->isReadable()) {
if (!$nativeType->isSuperTypeOf($prototype->getNativeType())->yes()) {
$typeErrors[] = RuleErrorBuilder::message(sprintf(
'Type %s of property %s::$%s is not contravariant with type %s of overridden property %s::$%s.',
$nativeType->describe(VerbosityLevel::typeOnly()),
$classReflection->getDisplayName(),
$node->getName(),
$prototype->getNativeType()->describe(VerbosityLevel::typeOnly()),
$prototype->getDeclaringClass()->getDisplayName(),
$node->getName(),
))->identifier('property.nativeType')->nonIgnorable()->build();
}
} elseif (!$prototype->getNativeType()->isSuperTypeOf($nativeType)->yes()) {
$typeErrors[] = RuleErrorBuilder::message(sprintf(
'Type %s of property %s::$%s is not covariant with type %s of overridden property %s::$%s.',
$nativeType->describe(VerbosityLevel::typeOnly()),
$classReflection->getDisplayName(),
$node->getName(),
$prototype->getNativeType()->describe(VerbosityLevel::typeOnly()),
$prototype->getDeclaringClass()->getDisplayName(),
$node->getName(),
))->identifier('property.nativeType')->nonIgnorable()->build();
}
} else {
$typeErrors[] = RuleErrorBuilder::message(sprintf(
'Type %s of property %s::$%s is not the same as type %s of overridden property %s::$%s.',
$nativeType->describe(VerbosityLevel::typeOnly()),
$classReflection->getDisplayName(),
$node->getName(),
$prototype->getNativeType()->describe(VerbosityLevel::typeOnly()),
$prototype->getDeclaringClass()->getDisplayName(),
$node->getName(),
))->identifier('property.nativeType')->nonIgnorable()->build();
}
}
}
} elseif ($nativeType !== null) {
Expand Down Expand Up @@ -167,6 +199,45 @@ public function processNode(Node $node, Scope $scope): array
}

$verbosity = VerbosityLevel::getRecommendedLevelByType($prototype->getReadableType(), $propertyReflection->getReadableType());

if (
$this->phpVersion->supportsPropertyHooks()
&& ($prototype->isVirtual()->yes() || $prototype->isAbstract()->yes())
&& (!$prototype->isReadable() || !$prototype->isWritable())
) {
if (!$prototype->isReadable()) {
if (!$propertyReflection->getReadableType()->isSuperTypeOf($prototype->getReadableType())->yes()) {
$errors[] = RuleErrorBuilder::message(sprintf(
'PHPDoc type %s of property %s::$%s is not contravariant with PHPDoc type %s of overridden property %s::$%s.',
$propertyReflection->getReadableType()->describe($verbosity),
$classReflection->getDisplayName(),
$node->getName(),
$prototype->getReadableType()->describe($verbosity),
$prototype->getDeclaringClass()->getDisplayName(),
$node->getName(),
))->identifier('property.phpDocType')->tip(sprintf(
"You can fix 3rd party PHPDoc types with stub files:\n %s",
'<fg=cyan>https://phpstan.org/user-guide/stub-files</>',
))->build();
}
} elseif (!$prototype->getReadableType()->isSuperTypeOf($propertyReflection->getReadableType())->yes()) {
$errors[] = RuleErrorBuilder::message(sprintf(
'PHPDoc type %s of property %s::$%s is not covariant with PHPDoc type %s of overridden property %s::$%s.',
$propertyReflection->getReadableType()->describe($verbosity),
$classReflection->getDisplayName(),
$node->getName(),
$prototype->getReadableType()->describe($verbosity),
$prototype->getDeclaringClass()->getDisplayName(),
$node->getName(),
))->identifier('property.phpDocType')->tip(sprintf(
"You can fix 3rd party PHPDoc types with stub files:\n %s",
'<fg=cyan>https://phpstan.org/user-guide/stub-files</>',
))->build();
}

return $errors;
}

$isSuperType = $prototype->getReadableType()->isSuperTypeOf($propertyReflection->getReadableType());
$canBeTurnedOffError = RuleErrorBuilder::message(sprintf(
'PHPDoc type %s of property %s::$%s is not the same as PHPDoc type %s of overridden property %s::$%s.',
Expand Down
41 changes: 40 additions & 1 deletion tests/PHPStan/Rules/Properties/OverridingPropertyRuleTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

namespace PHPStan\Rules\Properties;

use PHPStan\Php\PhpVersion;
use PHPStan\Rules\Rule;
use PHPStan\Testing\RuleTestCase;
use function sprintf;
Expand All @@ -17,7 +18,11 @@ class OverridingPropertyRuleTest extends RuleTestCase

protected function getRule(): Rule
{
return new OverridingPropertyRule(true, $this->reportMaybes);
return new OverridingPropertyRule(
self::getContainer()->getByType(PhpVersion::class),
true,
$this->reportMaybes,
);
}

public function testRule(): void
Expand Down Expand Up @@ -214,4 +219,38 @@ public function testPropertyPrototypeFromInterface(): void
]);
}

public function testBug12466(): void
{
if (PHP_VERSION_ID < 80400) {
$this->markTestSkipped('Test requires PHP 8.4.');
}

$tip = sprintf(
"You can fix 3rd party PHPDoc types with stub files:\n %s",
'<fg=cyan>https://phpstan.org/user-guide/stub-files</>',
);

$this->reportMaybes = true;
$this->analyse([__DIR__ . '/data/bug-12466.php'], [
[
'Type int|string|null of property Bug12466OverridenProperty\Baz::$onlyGet is not covariant with type int|string of overridden property Bug12466OverridenProperty\Foo::$onlyGet.',
34,
],
[
'Type int of property Bug12466OverridenProperty\Baz::$onlySet is not contravariant with type int|string of overridden property Bug12466OverridenProperty\Foo::$onlySet.',
40,
],
[
'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.',
82,
$tip,
],
[
'PHPDoc type array<int> of property Bug12466OverridenProperty\BazWithPhpDocs::$onlySet is not contravariant with PHPDoc type array<int|string> of overridden property Bug12466OverridenProperty\FooWithPhpDocs::$onlySet.',
89,
$tip,
],
]);
}

}
95 changes: 95 additions & 0 deletions tests/PHPStan/Rules/Properties/data/bug-12466.php
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,95 @@
<?php // lint >= 8.4

namespace Bug12466OverridenProperty;

interface Foo
{

public int|string $onlyGet { get; }

public int|string $onlySet { set; }

}

class Bar implements Foo
{

public int $onlyGet {
get {
return 1;
}
}

public int|string|null $onlySet {
set {
$this->onlySet = $value;
}
}

}

class Baz implements Foo
{

public int|string|null $onlyGet {
get {
return null;
}
}

public int $onlySet {
set {
$this->onlySet = $value;
}
}

}

interface FooWithPhpDocs
{

/** @var array<int|string> */
public array $onlyGet { get; }

/** @var array<int|string> */
public array $onlySet { set; }

}

class BarWithPhpDocs implements FooWithPhpDocs
{

/** @var array<int> */
public array $onlyGet {
get {
return [];
}
}

/** @var array<int|string|null> */
public array $onlySet {
set {
$this->onlySet = $value;
}
}

}

class BazWithPhpDocs implements FooWithPhpDocs
{

/** @var array<int|string|null> */
public array $onlyGet {
get {
return [];
}
}

/** @var array<int> */
public array $onlySet {
set {
$this->onlySet = $value;
}
}

}

0 comments on commit 50f8e49

Please sign in to comment.