Make use of ClassMethodReturnVendorLockResolver (#377)

Co-authored-by: GitHub Action <action@github.com>
This commit is contained in:
Tomas Votruba 2021-07-04 23:04:09 +02:00 committed by GitHub
parent a9b1bbba88
commit 904a5a7a1c
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
41 changed files with 108 additions and 264 deletions

View File

@ -11,12 +11,10 @@ use PHPStan\Reflection\FunctionVariantWithPhpDocs;
use PHPStan\Type\MixedType;
use Rector\NodeNameResolver\NodeNameResolver;
use Rector\NodeTypeResolver\Node\AttributeKey;
use Rector\VendorLocker\Reflection\MethodReflectionContractAnalyzer;
final class ClassMethodReturnVendorLockResolver
{
public function __construct(
private MethodReflectionContractAnalyzer $methodReflectionContractAnalyzer,
private NodeNameResolver $nodeNameResolver
) {
}
@ -33,33 +31,29 @@ final class ClassMethodReturnVendorLockResolver
return false;
}
if (count($classReflection->getAncestors()) === 1) {
return false;
}
$methodName = $this->nodeNameResolver->getName($classMethod);
if ($this->isVendorLockedByParentClass($classReflection, $methodName)) {
if ($this->isVendorLockedByAncestors($classReflection, $methodName)) {
return true;
}
if ($classReflection->isTrait()) {
return false;
}
return $this->methodReflectionContractAnalyzer->hasInterfaceContract($classReflection, $methodName);
return $classReflection->isTrait();
}
private function isVendorLockedByParentClass(ClassReflection $classReflection, string $methodName): bool
private function isVendorLockedByAncestors(ClassReflection $classReflection, string $methodName): bool
{
foreach ($classReflection->getParents() as $parentClassReflections) {
$nativeClassReflection = $parentClassReflections->getNativeReflection();
foreach ($classReflection->getAncestors() as $ancestorClassReflections) {
if ($ancestorClassReflections === $classReflection) {
continue;
}
$nativeClassReflection = $ancestorClassReflections->getNativeReflection();
// this should avoid detecting @method as real method
if (! $nativeClassReflection->hasMethod($methodName)) {
continue;
}
$parentClassMethodReflection = $parentClassReflections->getNativeMethod($methodName);
$parentClassMethodReflection = $ancestorClassReflections->getNativeMethod($methodName);
$parametersAcceptor = $parentClassMethodReflection->getVariants()[0];
if (! $parametersAcceptor instanceof FunctionVariantWithPhpDocs) {
continue;

View File

@ -1,64 +0,0 @@
<?php
declare(strict_types=1);
namespace Rector\VendorLocker\NodeVendorLocker;
use PhpParser\Node\Stmt\ClassMethod;
use PHPStan\Analyser\Scope;
use PHPStan\Reflection\ClassReflection;
use PHPStan\Reflection\Php\PhpMethodReflection;
use Rector\NodeNameResolver\NodeNameResolver;
use Rector\NodeTypeResolver\Node\AttributeKey;
use Rector\VendorLocker\Reflection\MethodReflectionContractAnalyzer;
final class ClassMethodVendorLockResolver
{
public function __construct(
private MethodReflectionContractAnalyzer $methodReflectionContractAnalyzer,
private NodeNameResolver $nodeNameResolver
) {
}
/**
* Checks for:
* - interface required methods
* - abstract classes reqired method
*
* Prevent:
* - removing class methods, that breaks the code
*/
public function isRemovalVendorLocked(ClassMethod $classMethod): bool
{
$classMethodName = $this->nodeNameResolver->getName($classMethod);
/** @var Scope $scope */
$scope = $classMethod->getAttribute(AttributeKey::SCOPE);
$classReflection = $scope->getClassReflection();
if (! $classReflection instanceof ClassReflection) {
return false;
}
if ($this->methodReflectionContractAnalyzer->hasInterfaceContract($classReflection, $classMethodName)) {
return true;
}
foreach ($classReflection->getParents() as $parentClassReflection) {
if (! $parentClassReflection->hasMethod($classMethodName)) {
continue;
}
$methodReflection = $parentClassReflection->getNativeMethod($classMethodName);
if (! $methodReflection instanceof PhpMethodReflection) {
continue;
}
if ($methodReflection->isAbstract()) {
return true;
}
}
return false;
}
}

View File

@ -1,21 +0,0 @@
<?php
declare(strict_types=1);
namespace Rector\VendorLocker\Reflection;
use PHPStan\Reflection\ClassReflection;
final class MethodReflectionContractAnalyzer
{
public function hasInterfaceContract(ClassReflection $classReflection, string $methodName): bool
{
foreach ($classReflection->getInterfaces() as $interfaceReflection) {
if ($interfaceReflection->hasMethod($methodName)) {
return true;
}
}
return false;
}
}

View File

@ -90,7 +90,6 @@ return static function (ContainerConfigurator $containerConfigurator): void {
ReturnTypeDeclarationRector::class => [
__DIR__ . '/packages/PHPStanStaticTypeMapper/TypeMapper/ArrayTypeMapper.php',
__DIR__ . '/packages/PHPStanStaticTypeMapper/TypeMapper/ObjectTypeMapper.php',
__DIR__ . '/src/DependencyInjection/Loader/ConfigurableCallValuesCollectingPhpFileLoader.php',
],
AddVoidReturnTypeWhereNoReturnRector::class => [

View File

@ -0,0 +1,29 @@
<?php
namespace Rector\Tests\TypeDeclaration\Rector\ClassMethod\AddVoidReturnTypeWhereNoReturnRector\Fixture;
use Symfony\Component\DependencyInjection\Loader\PhpFileLoader;
final class ExternalImplicitVoidOverride extends PhpFileLoader
{
public function load($resource, string $type = null)
{
}
}
?>
-----
<?php
namespace Rector\Tests\TypeDeclaration\Rector\ClassMethod\AddVoidReturnTypeWhereNoReturnRector\Fixture;
use Symfony\Component\DependencyInjection\Loader\PhpFileLoader;
final class ExternalImplicitVoidOverride extends PhpFileLoader
{
public function load($resource, string $type = null): void
{
}
}
?>

View File

@ -1,14 +0,0 @@
<?php
namespace Rector\Tests\TypeDeclaration\Rector\ClassMethod\AddVoidReturnTypeWhereNoReturnRector\Fixture;
use Symfony\Component\DependencyInjection\Loader\PhpFileLoader;
final class SkipFromExternal extends PhpFileLoader
{
public function load($resource, string $type = null)
{
}
}
?>

View File

@ -4,7 +4,7 @@ namespace Rector\Tests\TypeDeclaration\Rector\ClassMethod\AddVoidReturnTypeWhere
trait Foo
{
public function load($resource, string $type = null)
public function load($resource, string $type = null): string
{
}
}

View File

@ -0,0 +1,29 @@
<?php
namespace Rector\Tests\TypeDeclaration\Rector\FunctionLike\ReturnTypeDeclarationRector\Fixture;
use Rector\Tests\TypeDeclaration\Rector\FunctionLike\ReturnTypeDeclarationRector\Source\ExternalForVoidInterface;
final class ExternalVoidInterface implements ExternalForVoidInterface
{
public function run()
{
}
}
?>
-----
<?php
namespace Rector\Tests\TypeDeclaration\Rector\FunctionLike\ReturnTypeDeclarationRector\Fixture;
use Rector\Tests\TypeDeclaration\Rector\FunctionLike\ReturnTypeDeclarationRector\Source\ExternalForVoidInterface;
final class ExternalVoidInterface implements ExternalForVoidInterface
{
public function run(): void
{
}
}
?>

View File

@ -1,12 +0,0 @@
<?php
namespace Rector\Tests\TypeDeclaration\Rector\FunctionLike\ReturnTypeDeclarationRector\Fixture;
use Rector\Tests\TypeDeclaration\Rector\FunctionLike\ReturnTypeDeclarationRector\Source\vendor\ExternalForVoid;
final class SkipFromExternalVoid extends ExternalForVoid
{
public function run()
{
}
}

View File

@ -1,12 +0,0 @@
<?php
namespace Rector\Tests\TypeDeclaration\Rector\FunctionLike\ReturnTypeDeclarationRector\Fixture;
use Rector\Tests\TypeDeclaration\Rector\FunctionLike\ReturnTypeDeclarationRector\Source\ExternalForVoidInterface;
final class SkipFromExternalVoidInterface implements ExternalForVoidInterface
{
public function run()
{
}
}

View File

@ -1,12 +0,0 @@
<?php
declare(strict_types=1);
namespace Rector\Tests\TypeDeclaration\Rector\FunctionLike\ReturnTypeDeclarationRector\Source\vendor;
class ExternalForVoid
{
public function run()
{
}
}

View File

@ -6,7 +6,7 @@ namespace Rector\Tests\TypeDeclaration\Rector\FunctionLike\ReturnTypeDeclaration
trait ExternalForVoidTrait
{
public function run()
public function run(): bool
{
}
}

View File

@ -70,7 +70,7 @@ CODE_SAMPLE
/**
* @param FuncCall $node
*/
public function refactor(Node $node): ?Node
public function refactor(Node $node): FuncCall
{
foreach ($this->replacedArguments as $replacedArgument) {
if (! $this->isName($node->name, $replacedArgument->getFunction())) {

View File

@ -70,7 +70,7 @@ CODE_SAMPLE
/**
* @param FuncCall $node
*/
public function refactor(Node $node): ?Node
public function refactor(Node $node): FuncCall
{
foreach ($this->functionArgumentSwaps as $functionArgumentSwap) {
if (! $this->isName($node, $functionArgumentSwap->getFunction())) {

View File

@ -58,7 +58,7 @@ CODE_SAMPLE
/**
* @param NotEqual $node
*/
public function refactor(Node $node): ?Node
public function refactor(Node $node): NotEqual
{
// invoke override to default "!="
$node->setAttribute(AttributeKey::ORIGINAL_NODE, null);

View File

@ -68,9 +68,9 @@ CODE_SAMPLE
/**
* @param ClassConst|Property $node
* @return Node|Node[]|null
* @return Node[]|null
*/
public function refactor(Node $node)
public function refactor(Node $node): ?array
{
if ($node instanceof ClassConst) {
if (count($node->consts) < 2) {

View File

@ -83,7 +83,7 @@ CODE_SAMPLE
/**
* @param ClassMethod $node
*/
public function refactor(Node $node): ?Node
public function refactor(Node $node): ClassMethod
{
foreach ($this->methodsByType as $type => $methods) {
if (! $this->isObjectType($node, new ObjectType($type))) {

View File

@ -66,7 +66,7 @@ CODE_SAMPLE
/**
* @param String_ $node
*/
public function refactor(Node $node): ?Node
public function refactor(Node $node): String_
{
$doubleQuoteCount = substr_count($node->value, '"');
$singleQuoteCount = substr_count($node->value, "'");

View File

@ -66,9 +66,8 @@ CODE_SAMPLE
/**
* @param If_ $node
* @return null|If_
*/
public function refactor(Node $node)
public function refactor(Node $node): ?If_
{
$scope = $node->getAttribute(AttributeKey::SCOPE);

View File

@ -62,7 +62,7 @@ CODE_SAMPLE
* @param TryCatch $node
* @return Stmt[]|null
*/
public function refactor(Node $node)
public function refactor(Node $node): ?array
{
if (count($node->catches) !== 1) {
return null;

View File

@ -58,7 +58,7 @@ CODE_SAMPLE
* @param Return_ $node
* @return null|Expression[]|Return_[]
*/
public function refactor(Node $node)
public function refactor(Node $node): ?array
{
if (! $node->expr instanceof MethodCall) {
return null;

View File

@ -5,6 +5,7 @@ declare(strict_types=1);
namespace Rector\DowngradePhp53\Rector\Dir;
use PhpParser\Node;
use PhpParser\Node\Expr\FuncCall;
use PhpParser\Node\Scalar\MagicConst\Dir;
use PhpParser\Node\Scalar\MagicConst\File;
use Rector\Core\Rector\AbstractRector;
@ -57,7 +58,7 @@ CODE_SAMPLE
/**
* @param Dir $node
*/
public function refactor(Node $node): ?Node
public function refactor(Node $node): FuncCall
{
return $this->nodeFactory->createFuncCall('dirname', [new File()]);
}

View File

@ -51,7 +51,7 @@ CODE_SAMPLE
/**
* @param Coalesce $node
*/
public function refactor(Node $node): ?Node
public function refactor(Node $node): Ternary
{
$if = $node->left;
$else = $node->right;

View File

@ -49,9 +49,9 @@ CODE_SAMPLE
/**
* @param GroupUse $node
* @return Use_[]|null
* @return Use_[]
*/
public function refactor(Node $node)
public function refactor(Node $node): array
{
$prefix = $this->getName($node->prefix);

View File

@ -70,7 +70,7 @@ CODE_SAMPLE
/**
* @param Spaceship $node
*/
public function refactor(Node $node): ?Node
public function refactor(Node $node): FuncCall
{
$leftVariableParam = new Variable('left');
$rightVariableParam = new Variable('right');

View File

@ -56,7 +56,7 @@ CODE_SAMPLE
/**
* @param ClassConst $node
*/
public function refactor(Node $node): ?Node
public function refactor(Node $node): ClassConst
{
$this->visibilityManipulator->removeVisibility($node);

View File

@ -6,6 +6,7 @@ namespace Rector\DowngradePhp74\Rector\ArrowFunction;
use PhpParser\Node;
use PhpParser\Node\Expr\ArrowFunction;
use PhpParser\Node\Expr\Closure;
use PhpParser\Node\Stmt\Return_;
use Rector\Core\Rector\AbstractRector;
use Rector\Php72\NodeFactory\AnonymousFunctionFactory;
@ -66,7 +67,7 @@ CODE_SAMPLE
/**
* @param ArrowFunction $node
*/
public function refactor(Node $node): ?Node
public function refactor(Node $node): Closure
{
$stmts = [new Return_($node->expr)];

View File

@ -46,7 +46,7 @@ CODE_SAMPLE
/**
* @param AssignCoalesce $node
*/
public function refactor(Node $node): ?Node
public function refactor(Node $node): Assign
{
return new Assign($node->var, new Coalesce($node->var, $node->expr));
}

View File

@ -56,7 +56,7 @@ CODE_SAMPLE
/**
* @param NullsafeMethodCall|NullsafePropertyFetch $node
*/
public function refactor(Node $node): ?Node
public function refactor(Node $node): Ternary
{
$tempVarName = $this->variableNaming->resolveFromNodeWithScopeCountAndFallbackName(
$node->var,

View File

@ -77,9 +77,9 @@ CODE_SAMPLE
/**
* @param If_ $node
* @return Stmt[]|Node|null
* @return Stmt[]|null
*/
public function refactor(Node $node)
public function refactor(Node $node): ?array
{
$nextNode = $node->getAttribute(AttributeKey::NEXT_NODE);
if (! $nextNode instanceof Return_) {

View File

@ -56,7 +56,7 @@ CODE_SAMPLE
/**
* @param FuncCall $node
*/
public function refactor(Node $node): ?Node
public function refactor(Node $node): FuncCall
{
if ($this->isName($node, 'mysql_create_db')) {
return $this->processMysqlCreateDb($node);

View File

@ -68,7 +68,7 @@ CODE_SAMPLE
/**
* @param Switch_ $node
*/
public function refactor(Node $node): ?Node
public function refactor(Node $node): Switch_
{
foreach ($node->cases as $case) {
foreach ($case->stmts as $key => $caseStmt) {

View File

@ -53,7 +53,7 @@ CODE_SAMPLE
/**
* @param FuncCall $node
*/
public function refactor(Node $node): ?Node
public function refactor(Node $node): FuncCall
{
foreach ($node->args as $nodeArg) {
if ($nodeArg->byRef) {

View File

@ -112,7 +112,7 @@ CODE_SAMPLE
/**
* @param Class_ $node
*/
public function refactor(Node $node): ?Node
public function refactor(Node $node): Class_
{
$this->matchedObjectTypes = [];

View File

@ -72,7 +72,7 @@ CODE_SAMPLE
/**
* @param ClassConstFetch $node
*/
public function refactor(Node $node): ?Node
public function refactor(Node $node): ClassConstFetch
{
foreach ($this->renameClassConstFetches as $renameClassConstFetch) {
if (! $this->isObjectType($node->class, $renameClassConstFetch->getOldObjectType())) {

View File

@ -86,9 +86,8 @@ CODE_SAMPLE
/**
* @param MethodCall $node
* @return Node|Node[]|null
*/
public function refactor(Node $node)
public function refactor(Node $node): ?MethodCall
{
foreach ($this->callableInMethodCallToVariable as $singleCallableInMethodCallToVariable) {
if (! $this->isObjectType($node->var, $singleCallableInMethodCallToVariable->getObjectType())) {

View File

@ -12,6 +12,7 @@ use PhpParser\Node\Stmt\Function_;
use Rector\Core\Rector\AbstractRector;
use Rector\Core\ValueObject\PhpVersionFeature;
use Rector\TypeDeclaration\TypeInferer\SilentVoidResolver;
use Rector\VendorLocker\NodeVendorLocker\ClassMethodReturnVendorLockResolver;
use Symplify\RuleDocGenerator\ValueObject\CodeSample\CodeSample;
use Symplify\RuleDocGenerator\ValueObject\RuleDefinition;
@ -21,7 +22,8 @@ use Symplify\RuleDocGenerator\ValueObject\RuleDefinition;
final class AddVoidReturnTypeWhereNoReturnRector extends AbstractRector
{
public function __construct(
private SilentVoidResolver $silentVoidResolver
private SilentVoidResolver $silentVoidResolver,
private ClassMethodReturnVendorLockResolver $classMethodReturnVendorLockResolver
) {
}
@ -84,6 +86,10 @@ CODE_SAMPLE
return null;
}
if ($node instanceof ClassMethod && $this->classMethodReturnVendorLockResolver->isVendorLocked($node)) {
return null;
}
$node->returnType = new Identifier('void');
return $node;
}

View File

@ -15,7 +15,6 @@ use PhpParser\Node\UnionType as PhpParserUnionType;
use PHPStan\Type\MixedType;
use PHPStan\Type\Type;
use PHPStan\Type\UnionType;
use Rector\Core\NodeAnalyzer\ExternalFullyQualifiedAnalyzer;
use Rector\Core\Rector\AbstractRector;
use Rector\Core\ValueObject\PhpVersionFeature;
use Rector\NodeTypeResolver\Node\AttributeKey;
@ -48,7 +47,6 @@ final class ReturnTypeDeclarationRector extends AbstractRector
private VendorLockResolver $vendorLockResolver,
private PhpParserTypeAnalyzer $phpParserTypeAnalyzer,
private ObjectTypeComparator $objectTypeComparator,
private ExternalFullyQualifiedAnalyzer $externalFullyQualifiedAnalyzer
) {
}
@ -200,10 +198,6 @@ CODE_SAMPLE
ClassMethod | Function_ $functionLike,
Name | NullableType | PhpParserUnionType $inferredReturnNode
): void {
if ($this->isExternalVoid($functionLike, $inferredReturnNode)) {
return;
}
if ($functionLike->returnType === null) {
$functionLike->returnType = $inferredReturnNode;
return;
@ -221,22 +215,6 @@ CODE_SAMPLE
}
}
private function isExternalVoid(
ClassMethod | Function_ $functionLike,
Name | NullableType | PhpParserUnionType $inferredReturnNode
): bool {
$classLike = $functionLike->getAttribute(AttributeKey::CLASS_NODE);
if (! $classLike instanceof Class_) {
return false;
}
if (! $this->externalFullyQualifiedAnalyzer->hasVendorLocatedDependency($classLike)) {
return false;
}
return $functionLike->returnType === null && $this->isName($inferredReturnNode, 'void');
}
private function isNullableTypeSubType(Type $currentType, Type $inferedType): bool
{
if (! $currentType instanceof UnionType) {

View File

@ -7,7 +7,6 @@ namespace Rector\TypeDeclaration\TypeInferer;
use PhpParser\Node\Expr\Closure;
use PhpParser\Node\Expr\Yield_;
use PhpParser\Node\Stmt;
use PhpParser\Node\Stmt\Class_;
use PhpParser\Node\Stmt\ClassMethod;
use PhpParser\Node\Stmt\Expression;
use PhpParser\Node\Stmt\Function_;
@ -17,7 +16,6 @@ use PhpParser\Node\Stmt\Switch_;
use PhpParser\Node\Stmt\Throw_;
use PhpParser\Node\Stmt\Trait_;
use PhpParser\Node\Stmt\TryCatch;
use Rector\Core\NodeAnalyzer\ExternalFullyQualifiedAnalyzer;
use Rector\Core\PhpParser\Node\BetterNodeFinder;
use Rector\NodeTypeResolver\Node\AttributeKey;
@ -25,7 +23,6 @@ final class SilentVoidResolver
{
public function __construct(
private BetterNodeFinder $betterNodeFinder,
private ExternalFullyQualifiedAnalyzer $externalFullyQualifiedAnalyzer
) {
}
@ -48,12 +45,6 @@ final class SilentVoidResolver
return false;
}
if ($classLike instanceof Class_ && $this->externalFullyQualifiedAnalyzer->hasVendorLocatedDependency(
$classLike
)) {
return false;
}
/** @var Return_[] $returns */
$returns = $this->betterNodeFinder->findInstanceOf((array) $functionLike->stmts, Return_::class);
foreach ($returns as $return) {

View File

@ -23,7 +23,7 @@ final class ConfigurableCallValuesCollectingPhpFileLoader extends PhpFileLoader
/**
* @param mixed $resource
*/
public function load($resource, string $type = null)
public function load($resource, string $type = null): void
{
// this call collects root values
$this->collectConfigureCallsFromJustImportedConfigurableRectorDefinitions();
@ -39,7 +39,7 @@ final class ConfigurableCallValuesCollectingPhpFileLoader extends PhpFileLoader
$ignoreErrors = false,
$sourceResource = null,
$exclude = null
) {
): void {
// this call collects root values
$this->collectConfigureCallsFromJustImportedConfigurableRectorDefinitions();

View File

@ -1,47 +0,0 @@
<?php
declare(strict_types=1);
namespace Rector\Core\NodeAnalyzer;
use PhpParser\Node;
use PHPStan\Analyser\Scope;
use PHPStan\Reflection\ClassReflection;
use Rector\NodeTypeResolver\Node\AttributeKey;
final class ExternalFullyQualifiedAnalyzer
{
/**
* Is in a class that depends on a class, interface or trait located in vendor?
*/
public function hasVendorLocatedDependency(Node $node): bool
{
$scope = $node->getAttribute(AttributeKey::SCOPE);
if (! $scope instanceof Scope) {
return false;
}
$classReflection = $scope->getClassReflection();
if (! $classReflection instanceof ClassReflection) {
return false;
}
foreach ($classReflection->getAncestors() as $ancestorClassReflection) {
if ($classReflection === $ancestorClassReflection) {
continue;
}
$fileName = $ancestorClassReflection->getFileName();
if ($fileName === false) {
continue;
}
// file is located in vendor → out of modifiable scope
if (str_contains($fileName, '/vendor/')) {
return true;
}
}
return false;
}
}