Skip to content

Commit

Permalink
Fixed handling of static in parameter type in implemented interfaces
Browse files Browse the repository at this point in the history
  • Loading branch information
ondrejmirtes committed Mar 17, 2021
1 parent 00a8ba9 commit d225a68
Show file tree
Hide file tree
Showing 6 changed files with 243 additions and 6 deletions.
34 changes: 28 additions & 6 deletions src/Rules/Methods/MethodSignatureRule.php
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,10 @@
use PHPStan\TrinaryLogic;
use PHPStan\Type\Generic\TemplateTypeHelper;
use PHPStan\Type\MixedType;
use PHPStan\Type\StaticType;
use PHPStan\Type\Type;
use PHPStan\Type\TypehintHelper;
use PHPStan\Type\TypeTraverser;
use PHPStan\Type\VerbosityLevel;
use PHPStan\Type\VoidType;

Expand Down Expand Up @@ -62,6 +64,7 @@ public function processNode(Node $node, Scope $scope): array
$parameters = ParametersAcceptorSelector::selectSingle($method->getVariants());

$errors = [];
$declaringClass = $method->getDeclaringClass();
foreach ($this->collectParentMethods($methodName, $method->getDeclaringClass()) as $parentMethod) {
$parentVariants = $parentMethod->getVariants();
if (count($parentVariants) !== 1) {
Expand All @@ -72,7 +75,7 @@ public function processNode(Node $node, Scope $scope): array
continue;
}

[$returnTypeCompatibility, $returnType, $parentReturnType] = $this->checkReturnTypeCompatibility($parameters, $parentParameters);
[$returnTypeCompatibility, $returnType, $parentReturnType] = $this->checkReturnTypeCompatibility($declaringClass, $parameters, $parentParameters);
if ($returnTypeCompatibility->no() || (!$returnTypeCompatibility->yes() && $this->reportMaybes)) {
$errors[] = RuleErrorBuilder::message(sprintf(
'Return type (%s) of method %s::%s() should be %s with return type (%s) of method %s::%s()',
Expand All @@ -86,7 +89,7 @@ public function processNode(Node $node, Scope $scope): array
))->build();
}

$parameterResults = $this->checkParameterTypeCompatibility($parameters->getParameters(), $parentParameters->getParameters());
$parameterResults = $this->checkParameterTypeCompatibility($declaringClass, $parameters->getParameters(), $parentParameters->getParameters());
foreach ($parameterResults as $parameterIndex => [$parameterResult, $parameterType, $parentParameterType]) {
if ($parameterResult->yes()) {
continue;
Expand Down Expand Up @@ -149,6 +152,7 @@ private function collectParentMethods(string $methodName, ClassReflection $class
* @return array{TrinaryLogic, Type, Type}
*/
private function checkReturnTypeCompatibility(
ClassReflection $declaringClass,
ParametersAcceptorWithPhpDocs $currentVariant,
ParametersAcceptorWithPhpDocs $parentVariant
): array
Expand All @@ -157,10 +161,11 @@ private function checkReturnTypeCompatibility(
$currentVariant->getNativeReturnType(),
TemplateTypeHelper::resolveToBounds($currentVariant->getPhpDocReturnType())
);
$parentReturnType = TypehintHelper::decideType(
$originalParentReturnType = TypehintHelper::decideType(
$parentVariant->getNativeReturnType(),
TemplateTypeHelper::resolveToBounds($parentVariant->getPhpDocReturnType())
);
$parentReturnType = $this->transformStaticType($declaringClass, $originalParentReturnType);
// Allow adding `void` return type hints when the parent defines no return type
if ($returnType instanceof VoidType && $parentReturnType instanceof MixedType) {
return [TrinaryLogic::createYes(), $returnType, $parentReturnType];
Expand All @@ -174,7 +179,7 @@ private function checkReturnTypeCompatibility(
return [$parentReturnType->isSuperTypeOf($returnType), TypehintHelper::decideType(
$currentVariant->getNativeReturnType(),
$currentVariant->getPhpDocReturnType()
), $parentReturnType];
), $originalParentReturnType];
}

/**
Expand All @@ -183,6 +188,7 @@ private function checkReturnTypeCompatibility(
* @return array<int, array{TrinaryLogic, Type, Type}>
*/
private function checkParameterTypeCompatibility(
ClassReflection $declaringClass,
array $parameters,
array $parentParameters
): array
Expand All @@ -198,18 +204,34 @@ private function checkParameterTypeCompatibility(
$parameter->getNativeType(),
TemplateTypeHelper::resolveToBounds($parameter->getPhpDocType())
);
$parentParameterType = TypehintHelper::decideType(
$originalParameterType = TypehintHelper::decideType(
$parentParameter->getNativeType(),
TemplateTypeHelper::resolveToBounds($parentParameter->getPhpDocType())
);
$parentParameterType = $this->transformStaticType($declaringClass, $originalParameterType);

$parameterResults[] = [$parameterType->isSuperTypeOf($parentParameterType), TypehintHelper::decideType(
$parameter->getNativeType(),
$parameter->getPhpDocType()
), $parentParameterType];
), $originalParameterType];
}

return $parameterResults;
}

private function transformStaticType(ClassReflection $declaringClass, Type $type): Type
{
return TypeTraverser::map($type, static function (Type $type, callable $traverse) use ($declaringClass): Type {
if ($type instanceof StaticType) {
$changedType = $type->changeBaseClass($declaringClass);
if ($declaringClass->isFinal()) {
$changedType = $changedType->getStaticObjectType();
}
return $traverse($changedType);
}

return $traverse($type);
});
}

}
31 changes: 31 additions & 0 deletions tests/PHPStan/Rules/Methods/MethodSignatureRuleTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -302,4 +302,35 @@ public function testBug3523(): void
]);
}

public function testBug4707(): void
{
$this->reportMaybes = true;
$this->reportStatic = true;
$this->analyse([__DIR__ . '/data/bug-4707.php'], [
[
'Return type (array<int, Bug4707\Row2>) of method Bug4707\Block2::getChildren() should be compatible with return type (array<int, Bug4707\ChildNodeInterface<static(Bug4707\ParentNodeInterface)>>) of method Bug4707\ParentNodeInterface::getChildren()',
38,
],
]);
}

public function testBug4707Covariant(): void
{
$this->reportMaybes = true;
$this->reportStatic = true;
$this->analyse([__DIR__ . '/data/bug-4707-covariant.php'], [
[
'Return type (array<int, Bug4707Covariant\Row2>) of method Bug4707Covariant\Block2::getChildren() should be covariant with return type (array<int, Bug4707Covariant\ChildNodeInterface<static(Bug4707Covariant\ParentNodeInterface)>>) of method Bug4707Covariant\ParentNodeInterface::getChildren()',
38,
],
]);
}

public function testBug4707Two(): void
{
$this->reportMaybes = true;
$this->reportStatic = true;
$this->analyse([__DIR__ . '/data/bug-4707-two.php'], []);
}

}
66 changes: 66 additions & 0 deletions tests/PHPStan/Rules/Methods/data/bug-4707-covariant.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
<?php

namespace Bug4707Covariant;

/**
* @template-covariant TParent of ParentNodeInterface
*/
interface ChildNodeInterface
{
/** @return TParent */
public function getParent(): ParentNodeInterface;
}

interface ParentNodeInterface
{
/** @return list<ChildNodeInterface<static>> */
public function getChildren(): array;
}

final class Block implements ParentNodeInterface
{
/** @var list<Row> */
private $rows = [];

/** @return list<Row> */
public function getChildren(): array
{
return $this->rows;
}
}

class Block2 implements ParentNodeInterface
{
/** @var list<Row2> */
private $rows = [];

/** @return list<Row2> */
public function getChildren(): array
{
return $this->rows;
}
}

/** @implements ChildNodeInterface<Block> */
final class Row implements ChildNodeInterface
{
/** @var Block $parent */
private $parent;

public function getParent(): Block
{
return $this->parent;
}
}

/** @implements ChildNodeInterface<Block2> */
final class Row2 implements ChildNodeInterface
{
/** @var Block2 $parent */
private $parent;

public function getParent(): Block2
{
return $this->parent;
}
}
26 changes: 26 additions & 0 deletions tests/PHPStan/Rules/Methods/data/bug-4707-three.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
<?php

namespace Bug4707Three;

/**
* @template TParent of ParentNodeInterface
*/
interface ChildNodeInterface
{}

interface ParentNodeInterface
{
/** @return ChildNodeInterface<Block> */
public function getChildren();
}

/** @implements ChildNodeInterface<Block> */
final class Row implements ChildNodeInterface {}

class Block implements ParentNodeInterface
{
/** @return Row */
public function getChildren() {
return new Row();
}
}
26 changes: 26 additions & 0 deletions tests/PHPStan/Rules/Methods/data/bug-4707-two.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
<?php

namespace Bug4707Two;

/**
* @template TParent of ParentNodeInterface
*/
interface ChildNodeInterface
{}

interface ParentNodeInterface
{
/** @return ChildNodeInterface<Block> */
public function getChildren();
}

/** @implements ChildNodeInterface<Block> */
final class Row implements ChildNodeInterface {}

final class Block implements ParentNodeInterface
{
/** @return Row */
public function getChildren() {
return new Row();
}
}
66 changes: 66 additions & 0 deletions tests/PHPStan/Rules/Methods/data/bug-4707.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
<?php

namespace Bug4707;

/**
* @template TParent of ParentNodeInterface
*/
interface ChildNodeInterface
{
/** @return TParent */
public function getParent(): ParentNodeInterface;
}

interface ParentNodeInterface
{
/** @return list<ChildNodeInterface<static>> */
public function getChildren(): array;
}

final class Block implements ParentNodeInterface
{
/** @var list<Row> */
private $rows = [];

/** @return list<Row> */
public function getChildren(): array
{
return $this->rows;
}
}

class Block2 implements ParentNodeInterface
{
/** @var list<Row2> */
private $rows = [];

/** @return list<Row2> */
public function getChildren(): array
{
return $this->rows;
}
}

/** @implements ChildNodeInterface<Block> */
final class Row implements ChildNodeInterface
{
/** @var Block $parent */
private $parent;

public function getParent(): Block
{
return $this->parent;
}
}

/** @implements ChildNodeInterface<Block2> */
final class Row2 implements ChildNodeInterface
{
/** @var Block2 $parent */
private $parent;

public function getParent(): Block2
{
return $this->parent;
}
}

0 comments on commit d225a68

Please sign in to comment.