mirror_php/rector
1
0
Fork 0
mirror of https://github.com/rectorphp/rector.git synced 2025-02-20 08:05:29 +01:00
Code Issues Projects Releases Wiki Activity

Fix return override in case of parent vendor lock (#2501)

Browse Source 
Fix return override in case of parent vendor lock
This commit is contained in:
Tomas Votruba 2019-12-27 00:26:00 +01:00 committed by GitHub
commit 14d207a137
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
12 changed files with 376 additions and 172 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
ecs.yaml
View File

@ -111,6 +111,8 @@ parameters:
- 'packages/NetteTesterToPHPUnit/src/AssertManipulator.php'
- 'packages/Legacy/src/NodeAnalyzer/SingletonClassMethodAnalyzer.php'
- 'src/Rector/AbstractRector/ComplexRemovalTrait.php'
- 'packages/TypeDeclaration/src/VendorLock/VendorLockResolver.php'
- 'packages/TypeDeclaration/src/PhpParserTypeAnalyzer.php'
# aliases
- 'packages/CodingStyle/src/Rector/Namespace_/ImportFullyQualifiedNamesRector.php'

65
packages/TypeDeclaration/src/PhpParserTypeAnalyzer.php Normal file
View File

@ -0,0 +1,65 @@
<?php
declare(strict_types=1);
namespace Rector\TypeDeclaration;
use PhpParser\Node;
use PhpParser\Node\Identifier;
use PhpParser\Node\Name;
use PhpParser\Node\NullableType;
use PhpParser\Node\UnionType;
final class PhpParserTypeAnalyzer
{
/**
* @param Name|NullableType|UnionType|Identifier $possibleSubtype
* @param Name|NullableType|UnionType|Identifier $possibleParentType
*/
public function isSubtypeOf(Node $possibleSubtype, Node $possibleParentType): bool
{
// skip until PHP 8 is out
if ($possibleSubtype instanceof UnionType || $possibleParentType instanceof UnionType) {
return false;
}
// possible - https://3v4l.org/ZuJCh
if ($possibleSubtype instanceof NullableType && ! $possibleParentType instanceof NullableType) {
return $this->isSubtypeOf($possibleSubtype->type, $possibleParentType);
}
// not possible - https://3v4l.org/iNDTc
if (! $possibleSubtype instanceof NullableType && $possibleParentType instanceof NullableType) {
return false;
}
// unwrap nullable types
if ($possibleParentType instanceof NullableType) {
$possibleParentType = $possibleParentType->type;
}
if ($possibleSubtype instanceof NullableType) {
$possibleSubtype = $possibleSubtype->type;
}
$possibleSubtype = $possibleSubtype->toString();
$possibleParentType = $possibleParentType->toString();
if (is_a($possibleSubtype, $possibleParentType, true)) {
return true;
}
if (in_array($possibleSubtype, ['array', 'Traversable'], true) && $possibleParentType === 'iterable') {
return true;
}
if (in_array($possibleSubtype, ['array', 'ArrayIterator'], true) && $possibleParentType === 'countable') {
return true;
}
if ($possibleParentType === $possibleSubtype) {
return true;
}
return ctype_upper($possibleSubtype[0]) && $possibleParentType === 'object';
}
}

181
packages/TypeDeclaration/src/Rector/FunctionLike/AbstractTypeDeclarationRector.php
View File

@ -8,21 +8,19 @@ use PhpParser\Node;
use PhpParser\Node\Identifier;
use PhpParser\Node\Name;
use PhpParser\Node\NullableType;
use PhpParser\Node\Stmt\Class_;
use PhpParser\Node\Stmt\ClassLike;
use PhpParser\Node\Stmt\ClassMethod;
use PhpParser\Node\Stmt\Function_;
use PhpParser\Node\Stmt\Interface_;
use PhpParser\Node\UnionType;
use PHPStan\Type\MixedType;
use PHPStan\Type\ObjectType;
use PHPStan\Type\StaticType;
use PHPStan\Type\Type;
use Rector\NodeContainer\ParsedNodesByType;
use Rector\NodeTypeResolver\Node\AttributeKey;
use Rector\NodeTypeResolver\PhpDoc\NodeAnalyzer\DocBlockManipulator;
use Rector\PHPStan\Type\SelfObjectType;
use Rector\Rector\AbstractRector;
use Rector\TypeDeclaration\PhpParserTypeAnalyzer;
use Rector\TypeDeclaration\VendorLock\VendorLockResolver;
/**
* @see https://wiki.php.net/rfc/scalar_type_hints_v5
@ -35,7 +33,7 @@ abstract class AbstractTypeDeclarationRector extends AbstractRector
/**
* @var string
*/
public const HAS_NEW_INHERITED_TYPE = 'has_new_inherited_return_type';
protected const HAS_NEW_INHERITED_TYPE = 'has_new_inherited_return_type';
/**
* @var DocBlockManipulator
@ -47,15 +45,29 @@ abstract class AbstractTypeDeclarationRector extends AbstractRector
*/
protected $parsedNodesByType;
/**
* @var PhpParserTypeAnalyzer
*/
protected $phpParserTypeAnalyzer;
/**
* @var VendorLockResolver
*/
protected $vendorLockResolver;
/**
* @required
*/
public function autowireAbstractTypeDeclarationRector(
DocBlockManipulator $docBlockManipulator,
ParsedNodesByType $parsedNodesByType
ParsedNodesByType $parsedNodesByType,
PhpParserTypeAnalyzer $phpParserTypeAnalyzer,
VendorLockResolver $vendorLockResolver
): void {
$this->docBlockManipulator = $docBlockManipulator;
$this->parsedNodesByType = $parsedNodesByType;
$this->phpParserTypeAnalyzer = $phpParserTypeAnalyzer;
$this->vendorLockResolver = $vendorLockResolver;
}
/**
@ -66,106 +78,6 @@ abstract class AbstractTypeDeclarationRector extends AbstractRector
return [Function_::class, ClassMethod::class];
}
protected function isChangeVendorLockedIn(ClassMethod $classMethod, int $paramPosition): bool
{
if (! $this->hasParentClassOrImplementsInterface($classMethod)) {
return false;
}
$methodName = $this->getName($classMethod);
// @todo extract to some "inherited parent method" service
/** @var string|null $parentClassName */
$parentClassName = $classMethod->getAttribute(AttributeKey::PARENT_CLASS_NAME);
if ($parentClassName !== null) {
$parentClassNode = $this->parsedNodesByType->findClass($parentClassName);
if ($parentClassNode !== null) {
$parentMethodNode = $parentClassNode->getMethod($methodName);
// @todo validate type is conflicting
// parent class method in local scope → it's ok
if ($parentMethodNode !== null) {
// parent method has no type → we cannot change it here
return isset($parentMethodNode->params[$paramPosition]) && $parentMethodNode->params[$paramPosition]->type === null;
}
// if not, look for it's parent parent - @todo recursion
}
if (method_exists($parentClassName, $methodName)) {
// @todo validate type is conflicting
// parent class method in external scope → it's not ok
return true;
// if not, look for it's parent parent - @todo recursion
}
}
$classNode = $classMethod->getAttribute(AttributeKey::CLASS_NODE);
if (! $classNode instanceof Class_ && ! $classNode instanceof Interface_) {
return false;
}
$interfaceNames = $this->getClassLikeNodeParentInterfaceNames($classNode);
foreach ($interfaceNames as $interfaceName) {
$interface = $this->parsedNodesByType->findInterface($interfaceName);
if ($interface !== null) {
// parent class method in local scope → it's ok
// @todo validate type is conflicting
if ($interface->getMethod($methodName) !== null) {
return false;
}
}
if (method_exists($interfaceName, $methodName)) {
// parent class method in external scope → it's not ok
// @todo validate type is conflicting
return true;
}
}
return false;
}
/**
* @param Name|NullableType|UnionType|Identifier $possibleSubtype
* @param Name|NullableType|UnionType|Identifier $type
*/
protected function isSubtypeOf(Node $possibleSubtype, Node $type): bool
{
// skip until PHP 8 is out
if ($possibleSubtype instanceof UnionType || $type instanceof UnionType) {
return false;
}
$type = $type instanceof NullableType ? $type->type : $type;
if ($possibleSubtype instanceof NullableType) {
$possibleSubtype = $possibleSubtype->type;
}
$possibleSubtype = $possibleSubtype->toString();
$type = $type->toString();
if (is_a($possibleSubtype, $type, true)) {
return true;
}
if (in_array($possibleSubtype, ['array', 'Traversable'], true) && $type === 'iterable') {
return true;
}
if (in_array($possibleSubtype, ['array', 'ArrayIterator'], true) && $type === 'countable') {
return true;
}
if ($type === $possibleSubtype) {
return true;
}
return ctype_upper($possibleSubtype[0]) && $type === 'object';
}
/**
* @return Name|NullableType|Identifier|UnionType|null
*/
@ -175,65 +87,10 @@ abstract class AbstractTypeDeclarationRector extends AbstractRector
return null;
}
if ($type instanceof SelfObjectType) {
$type = new ObjectType($type->getClassName());
} elseif ($type instanceof StaticType) {
if ($type instanceof SelfObjectType || $type instanceof StaticType) {
$type = new ObjectType($type->getClassName());
}
return $this->staticTypeMapper->mapPHPStanTypeToPhpParserNode($type);
}
private function hasParentClassOrImplementsInterface(ClassMethod $classMethod): bool
{
$classNode = $classMethod->getAttribute(AttributeKey::CLASS_NODE);
if ($classNode === null) {
return false;
}
if ($classNode instanceof Class_ || $classNode instanceof Interface_) {
if ($classNode->extends) {
return true;
}
}
if ($classNode instanceof Class_) {
return (bool) $classNode->implements;
}
return false;
}
/**
* @param Class_|Interface_ $classLike
* @return string[]
*/
private function getClassLikeNodeParentInterfaceNames(ClassLike $classLike): array
{
$interfaces = [];
if ($classLike instanceof Class_) {
foreach ($classLike->implements as $implementNode) {
$interfaceName = $this->getName($implementNode);
if ($interfaceName === null) {
continue;
}
$interfaces[] = $interfaceName;
}
}
if ($classLike instanceof Interface_) {
foreach ($classLike->extends as $extendNode) {
$interfaceName = $this->getName($extendNode);
if ($interfaceName === null) {
continue;
}
$interfaces[] = $interfaceName;
}
}
return $interfaces;
}
}

10
packages/TypeDeclaration/src/Rector/FunctionLike/ParamTypeDeclarationRector.php
View File

@ -140,7 +140,10 @@ PHP
}
$position = (int) $position;
if ($node instanceof ClassMethod && $this->isChangeVendorLockedIn($node, $position)) {
if ($node instanceof ClassMethod && $this->vendorLockResolver->isParameterChangeVendorLockedIn(
$node,
$position
)) {
continue;
}
@ -150,7 +153,10 @@ PHP
if ($possibleOverrideNewReturnType !== null) {
if ($paramNode->type === null) {
$paramNode->type = $paramTypeNode;
} elseif ($this->isSubtypeOf($possibleOverrideNewReturnType, $paramNode->type)) {
} elseif ($this->phpParserTypeAnalyzer->isSubtypeOf(
$possibleOverrideNewReturnType,
$paramNode->type
)) {
// allow override
$paramNode->type = $paramTypeNode;
}

19
packages/TypeDeclaration/src/Rector/FunctionLike/ReturnTypeDeclarationRector.php
View File

@ -100,10 +100,6 @@ PHP
*/
public function refactor(Node $node): ?Node
{
if (! $this->isAtLeastPhpVersion(PhpVersionFeature::SCALAR_TYPES)) {
return null;
}
if ($this->shouldSkip($node)) {
return null;
}
@ -135,10 +131,16 @@ PHP
// should be previous overridden?
if ($node->returnType !== null) {
$isSubtype = $this->isSubtypeOf($inferredReturnNode, $node->returnType);
$isSubtype = $this->phpParserTypeAnalyzer->isSubtypeOf($inferredReturnNode, $node->returnType);
$currentType = $this->staticTypeMapper->mapPhpParserNodePHPStanType($node->returnType);
if ($node instanceof ClassMethod) {
if ($this->vendorLockResolver->isReturnChangeVendorLockedIn($node)) {
return null;
}
}
if ($this->isCurrentObjectTypeSubType($currentType, $inferedType)) {
return null;
}
@ -149,7 +151,8 @@ PHP
if ($this->isAtLeastPhpVersion(PhpVersionFeature::COVARIANT_RETURN) && $isSubtype) {
$node->returnType = $inferredReturnNode;
} elseif (! $isSubtype) { // type override
} elseif (! $isSubtype) {
// type override with correct one
$node->returnType = $inferredReturnNode;
}
} else {
@ -168,6 +171,10 @@ PHP
*/
private function shouldSkip(Node $node): bool
{
if (! $this->isAtLeastPhpVersion(PhpVersionFeature::SCALAR_TYPES)) {
return true;
}
if (! $this->overrideExistingReturnTypes) {
if ($node->returnType !== null) {
return true;

182
packages/TypeDeclaration/src/VendorLock/VendorLockResolver.php Normal file
View File

@ -0,0 +1,182 @@
<?php
declare(strict_types=1);
namespace Rector\TypeDeclaration\VendorLock;
use PhpParser\Node\Stmt\Class_;
use PhpParser\Node\Stmt\ClassMethod;
use PhpParser\Node\Stmt\Interface_;
use Rector\NodeContainer\ParsedNodesByType;
use Rector\NodeTypeResolver\Node\AttributeKey;
use Rector\PhpParser\Node\Manipulator\ClassManipulator;
use Rector\PhpParser\Node\Resolver\NameResolver;
final class VendorLockResolver
{
/**
* @var NameResolver
*/
private $nameResolver;
/**
* @var ParsedNodesByType
*/
private $parsedNodesByType;
/**
* @var ClassManipulator
*/
private $classManipulator;
public function __construct(
NameResolver $nameResolver,
ParsedNodesByType $parsedNodesByType,
ClassManipulator $classManipulator
) {
$this->nameResolver = $nameResolver;
$this->parsedNodesByType = $parsedNodesByType;
$this->classManipulator = $classManipulator;
}
public function isParameterChangeVendorLockedIn(ClassMethod $classMethod, int $paramPosition): bool
{
if (! $this->hasParentClassOrImplementsInterface($classMethod)) {
return false;
}
$methodName = $this->nameResolver->getName($classMethod);
// @todo extract to some "inherited parent method" service
/** @var string|null $parentClassName */
$parentClassName = $classMethod->getAttribute(AttributeKey::PARENT_CLASS_NAME);
if ($parentClassName !== null) {
$parentClassNode = $this->parsedNodesByType->findClass($parentClassName);
if ($parentClassNode !== null) {
$parentMethodNode = $parentClassNode->getMethod($methodName);
// @todo validate type is conflicting
// parent class method in local scope → it's ok
if ($parentMethodNode !== null) {
// parent method has no type → we cannot change it here
return isset($parentMethodNode->params[$paramPosition]) && $parentMethodNode->params[$paramPosition]->type === null;
}
// if not, look for it's parent parent - @todo recursion
}
if (method_exists($parentClassName, $methodName)) {
// @todo validate type is conflicting
// parent class method in external scope → it's not ok
return true;
// if not, look for it's parent parent - @todo recursion
}
}
$classNode = $classMethod->getAttribute(AttributeKey::CLASS_NODE);
if (! $classNode instanceof Class_ && ! $classNode instanceof Interface_) {
return false;
}
$interfaceNames = $this->classManipulator->getClassLikeNodeParentInterfaceNames($classNode);
foreach ($interfaceNames as $interfaceName) {
$interface = $this->parsedNodesByType->findInterface($interfaceName);
if ($interface !== null) {
// parent class method in local scope → it's ok
// @todo validate type is conflicting
if ($interface->getMethod($methodName) !== null) {
return false;
}
}
if (method_exists($interfaceName, $methodName)) {
// parent class method in external scope → it's not ok
// @todo validate type is conflicting
return true;
}
}
return false;
}
public function isReturnChangeVendorLockedIn(ClassMethod $classMethod): bool
{
if (! $this->hasParentClassOrImplementsInterface($classMethod)) {
return false;
}
$methodName = $this->nameResolver->getName($classMethod);
// @todo extract to some "inherited parent method" service
/** @var string|null $parentClassName */
$parentClassName = $classMethod->getAttribute(AttributeKey::PARENT_CLASS_NAME);
if ($parentClassName !== null) {
$parentClassNode = $this->parsedNodesByType->findClass($parentClassName);
if ($parentClassNode !== null) {
$parentMethodNode = $parentClassNode->getMethod($methodName);
// @todo validate type is conflicting
// parent class method in local scope → it's ok
if ($parentMethodNode !== null) {
return $parentMethodNode->returnType !== null;
}
// if not, look for it's parent parent - @todo recursion
}
if (method_exists($parentClassName, $methodName)) {
// @todo validate type is conflicting
// parent class method in external scope → it's not ok
return true;
// if not, look for it's parent parent - @todo recursion
}
}
$classNode = $classMethod->getAttribute(AttributeKey::CLASS_NODE);
if (! $classNode instanceof Class_ && ! $classNode instanceof Interface_) {
return false;
}
$interfaceNames = $this->classManipulator->getClassLikeNodeParentInterfaceNames($classNode);
foreach ($interfaceNames as $interfaceName) {
$interface = $this->parsedNodesByType->findInterface($interfaceName);
if ($interface !== null) {
// parent class method in local scope → it's ok
// @todo validate type is conflicting
if ($interface->getMethod($methodName) !== null) {
return false;
}
}
if (method_exists($interfaceName, $methodName)) {
// parent class method in external scope → it's not ok
// @todo validate type is conflicting
return true;
}
}
return false;
}
private function hasParentClassOrImplementsInterface(ClassMethod $classMethod): bool
{
$classNode = $classMethod->getAttribute(AttributeKey::CLASS_NODE);
if ($classNode === null) {
return false;
}
if ($classNode instanceof Class_ || $classNode instanceof Interface_) {
if ($classNode->extends) {
return true;
}
}
if ($classNode instanceof Class_) {
return (bool) $classNode->implements;
}
return false;
}
}

14
packages/TypeDeclaration/tests/Rector/FunctionLike/ReturnTypeDeclarationRector/Fixture/skip_parent_overriden_with_throws.php.inc Normal file
View File

@ -0,0 +1,14 @@
<?php
namespace Rector\TypeDeclaration\Tests\Rector\ClassMethod\ReturnTypeDeclarationRector\Fixture;
use Rector\Exception\ShouldNotHappenException;
use Rector\TypeDeclaration\Tests\Rector\FunctionLike\ReturnTypeDeclarationRector\Source\ParentObjectReturnInterface;
final class SkipParentOverridenWithThrows implements ParentObjectReturnInterface
{
public function hydrate(): object
{
throw new ShouldNotHappenException();
}
}

11
packages/TypeDeclaration/tests/Rector/FunctionLike/ReturnTypeDeclarationRector/Source/ParentObjectReturnInterface.php Normal file
View File

@ -0,0 +1,11 @@
<?php declare(strict_types=1);
namespace Rector\TypeDeclaration\Tests\Rector\FunctionLike\ReturnTypeDeclarationRector\Source;
interface ParentObjectReturnInterface
{
/**
* @return object
*/
public function hydrate(): object;
}

2
packages/TypeDeclaration/tests/Rector/FunctionLike/ReturnTypeDeclarationRector/Source/RealReturnedClass.php
View File

@ -4,7 +4,7 @@ declare(strict_types=1);
namespace Rector\TypeDeclaration\Tests\Rector\FunctionLike\ReturnTypeDeclarationRector\Source;
final class RealReturnedClass
final class RealReturnedClass extends ReturnedClass
{
}

2
packages/TypeDeclaration/tests/Rector/FunctionLike/ReturnTypeDeclarationRector/Source/ReturnedClass.php
View File

@ -4,7 +4,7 @@ declare(strict_types=1);
namespace Rector\TypeDeclaration\Tests\Rector\FunctionLike\ReturnTypeDeclarationRector\Source;
final class ReturnedClass
class ReturnedClass
{
}

4
phpstan.neon
View File

@ -243,3 +243,7 @@ parameters:
- '#Method Rector\\BetterPhpDocParser\\PhpDocNodeFactory\\Gedmo\\(.*?)\:\:createFromNodeAndTokens\(\) should return Rector\\BetterPhpDocParser\\PhpDocNode\\Gedmo\\(.*?)\|null but returns PHPStan\\PhpDocParser\\Ast\\PhpDoc\\PhpDocTagValueNode\|null#'
- '#Access to an undefined property PhpParser\\Node\\Identifier\|PhpParser\\Node\\Name\|PhpParser\\Node\\NullableType\:\:\$type#'
- '#Call to an undefined method PhpParser\\Node\\Identifier\|PhpParser\\Node\\Name\|PhpParser\\Node\\NullableType\:\:toString\(\)#'

56
src/PhpParser/Node/Manipulator/ClassManipulator.php
View File

@ -14,6 +14,7 @@ use PhpParser\Node\Stmt\Class_;
use PhpParser\Node\Stmt\ClassLike;
use PhpParser\Node\Stmt\ClassMethod;
use PhpParser\Node\Stmt\Expression;
use PhpParser\Node\Stmt\Interface_;
use PhpParser\Node\Stmt\Nop;
use PhpParser\Node\Stmt\Property;
use PhpParser\Node\Stmt\Trait_;
@ -422,6 +423,23 @@ final class ClassManipulator
}
}
/**
* @param Class_|Interface_ $classLike
* @return string[]
*/
public function getClassLikeNodeParentInterfaceNames(ClassLike $classLike)
{
if ($classLike instanceof Class_) {
return $this->getClassImplementedInterfaceNames($classLike);
}
if ($classLike instanceof Interface_) {
return $this->getInterfaceExtendedInterfacesNames($classLike);
}
return [];
}
private function tryInsertBeforeFirstMethod(Class_ $classNode, Stmt $stmt): bool
{
foreach ($classNode->stmts as $key => $classStmt) {
@ -556,4 +574,42 @@ final class ClassManipulator
return false;
}
/**
* @return string[]
*/
private function getClassImplementedInterfaceNames(Class_ $class): array
{
$interfaceNames = [];
foreach ($class->implements as $implementNode) {
$interfaceName = $this->nameResolver->getName($implementNode);
if ($interfaceName === null) {
continue;
}
$interfaceNames[] = $interfaceName;
}
return $interfaceNames;
}
/**
* @return string[]
*/
private function getInterfaceExtendedInterfacesNames(Interface_ $interface): array
{
$interfaceNames = [];
foreach ($interface->extends as $extendNode) {
$interfaceName = $this->nameResolver->getName($extendNode);
if ($interfaceName === null) {
continue;
}
$interfaceNames[] = $interfaceName;
}
return $interfaceNames;
}
}