From ba0013f2928b928ab6d4b9ec97594e98a4d4d88a Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Sat, 29 Jul 2023 13:35:08 +0200 Subject: [PATCH 1/7] Added failling tests regarding unknown parents --- .../Fixture/skip_unknown_parent.php.inc | 13 +++++++++++++ 1 file changed, 13 insertions(+) create mode 100644 rules-tests/TypeDeclaration/Rector/ClassMethod/StrictArrayParamDimFetchRector/Fixture/skip_unknown_parent.php.inc diff --git a/rules-tests/TypeDeclaration/Rector/ClassMethod/StrictArrayParamDimFetchRector/Fixture/skip_unknown_parent.php.inc b/rules-tests/TypeDeclaration/Rector/ClassMethod/StrictArrayParamDimFetchRector/Fixture/skip_unknown_parent.php.inc new file mode 100644 index 00000000000..6e5f772ade7 --- /dev/null +++ b/rules-tests/TypeDeclaration/Rector/ClassMethod/StrictArrayParamDimFetchRector/Fixture/skip_unknown_parent.php.inc @@ -0,0 +1,13 @@ + Date: Sat, 29 Jul 2023 13:37:40 +0200 Subject: [PATCH 2/7] Create skip_unknown_parent.php.inc --- .../Fixture/skip_unknown_parent.php.inc | 9 +++++++++ 1 file changed, 9 insertions(+) create mode 100644 rules-tests/TypeDeclaration/Rector/ClassMethod/ReturnTypeFromStrictParamRector/Fixture/skip_unknown_parent.php.inc diff --git a/rules-tests/TypeDeclaration/Rector/ClassMethod/ReturnTypeFromStrictParamRector/Fixture/skip_unknown_parent.php.inc b/rules-tests/TypeDeclaration/Rector/ClassMethod/ReturnTypeFromStrictParamRector/Fixture/skip_unknown_parent.php.inc new file mode 100644 index 00000000000..53d9b5349d3 --- /dev/null +++ b/rules-tests/TypeDeclaration/Rector/ClassMethod/ReturnTypeFromStrictParamRector/Fixture/skip_unknown_parent.php.inc @@ -0,0 +1,9 @@ + Date: Sat, 29 Jul 2023 13:39:51 +0200 Subject: [PATCH 3/7] Create skip_unknown_parent.php.inc --- .../Fixture/skip_unknown_parent.php.inc | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) create mode 100644 rules-tests/TypeDeclaration/Rector/ClassMethod/ParamTypeByMethodCallTypeRector/Fixture/skip_unknown_parent.php.inc diff --git a/rules-tests/TypeDeclaration/Rector/ClassMethod/ParamTypeByMethodCallTypeRector/Fixture/skip_unknown_parent.php.inc b/rules-tests/TypeDeclaration/Rector/ClassMethod/ParamTypeByMethodCallTypeRector/Fixture/skip_unknown_parent.php.inc new file mode 100644 index 00000000000..55f270faef7 --- /dev/null +++ b/rules-tests/TypeDeclaration/Rector/ClassMethod/ParamTypeByMethodCallTypeRector/Fixture/skip_unknown_parent.php.inc @@ -0,0 +1,19 @@ +someTypedService->run($value); + } +} From bde674726954628d6b93d8e45024d58b3a4b55e5 Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Sat, 29 Jul 2023 13:41:32 +0200 Subject: [PATCH 4/7] Create skip_unknown_parent_class.php.inc --- .../Fixture/skip_unknown_parent_class.php.inc | 13 +++++++++++++ 1 file changed, 13 insertions(+) create mode 100644 rules-tests/TypeDeclaration/Rector/Param/ParamTypeFromStrictTypedPropertyRector/Fixture/skip_unknown_parent_class.php.inc diff --git a/rules-tests/TypeDeclaration/Rector/Param/ParamTypeFromStrictTypedPropertyRector/Fixture/skip_unknown_parent_class.php.inc b/rules-tests/TypeDeclaration/Rector/Param/ParamTypeFromStrictTypedPropertyRector/Fixture/skip_unknown_parent_class.php.inc new file mode 100644 index 00000000000..057c7471f46 --- /dev/null +++ b/rules-tests/TypeDeclaration/Rector/Param/ParamTypeFromStrictTypedPropertyRector/Fixture/skip_unknown_parent_class.php.inc @@ -0,0 +1,13 @@ +items = $args; + } +} From 4061246a78300f75cceedcf3a2352ae98b32c5a4 Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Mon, 7 Aug 2023 11:49:59 +0200 Subject: [PATCH 5/7] fix --- .../UnresolvableClassParentException.php | 9 ++++ .../ParentClassMethodTypeOverrideGuard.php | 52 ++++++++++++++++--- .../AddParamTypeFromPropertyTypeRector.php | 2 +- .../ParamTypeByMethodCallTypeRector.php | 2 +- .../ReturnTypeFromStrictParamRector.php | 2 +- .../StrictArrayParamDimFetchRector.php | 2 +- .../StrictStringParamConcatRector.php | 2 +- ...ParamTypeFromStrictTypedPropertyRector.php | 2 +- 8 files changed, 61 insertions(+), 12 deletions(-) create mode 100644 packages/VendorLocker/Exception/UnresolvableClassParentException.php diff --git a/packages/VendorLocker/Exception/UnresolvableClassParentException.php b/packages/VendorLocker/Exception/UnresolvableClassParentException.php new file mode 100644 index 00000000000..44cae56c339 --- /dev/null +++ b/packages/VendorLocker/Exception/UnresolvableClassParentException.php @@ -0,0 +1,9 @@ +getParentClassMethod($classMethod) instanceof MethodReflection; + try { + $parentClassMethod = $this->resolveParentClassMethod($classMethod); + + return $parentClassMethod instanceof MethodReflection; + } catch (UnresolvableClassParentException) { + // we don't know all involved parents. + return null; + } } public function getParentClassMethod(ClassMethod $classMethod): ?MethodReflection + { + try { + $parentClassMethod = $this->resolveParentClassMethod($classMethod); + + return $parentClassMethod; + } catch (UnresolvableClassParentException) { + // we don't know all involved parents. + throw new ShouldNotHappenException('Unable to resolve involved class. You are likely missing hasParentClassMethod() before calling getParentClassMethod().'); + } + } + + /** + * @throws UnresolvableClassParentException + */ + private function resolveParentClassMethod(ClassMethod $classMethod): ?MethodReflection { $classReflection = $this->reflectionResolver->resolveClassReflection($classMethod); if (! $classReflection instanceof ClassReflection) { - return null; + // we can't resolve the class, so we don't know. + throw new UnresolvableClassParentException(); } /** @var string $methodName */ $methodName = $this->nodeNameResolver->getName($classMethod); - $parentClassReflection = $classReflection->getParentClass(); - while ($parentClassReflection instanceof ClassReflection) { + $currentClassReflection = $classReflection; + while ($this->hasClassParent($currentClassReflection)) { + $parentClassReflection = $currentClassReflection->getParentClass(); + if (!$parentClassReflection instanceof ClassReflection) { + // per AST we have a parent class, but our reflection classes are not able to load its class definition/signature + throw new UnresolvableClassParentException(); + } + if ($parentClassReflection->hasNativeMethod($methodName)) { return $parentClassReflection->getNativeMethod($methodName); } - $parentClassReflection = $parentClassReflection->getParentClass(); + $currentClassReflection = $parentClassReflection; } foreach ($classReflection->getInterfaces() as $interfaceReflection) { @@ -57,6 +90,13 @@ public function getParentClassMethod(ClassMethod $classMethod): ?MethodReflectio return null; } + private function hasClassParent(ClassReflection $classReflection):bool { + $nativeReflection = $classReflection->getNativeReflection(); + $betterReflectionClass = $this->privatesAccessor->getPrivateProperty($nativeReflection, 'betterReflectionClass'); + $parentClassName = $this->privatesAccessor->getPrivateProperty($betterReflectionClass, 'parentClassName'); + return $parentClassName !== null; + } + public function shouldSkipReturnTypeChange(ClassMethod $classMethod, Type $parentType): bool { if ($classMethod->returnType === null) { diff --git a/rules/TypeDeclaration/Rector/ClassMethod/AddParamTypeFromPropertyTypeRector.php b/rules/TypeDeclaration/Rector/ClassMethod/AddParamTypeFromPropertyTypeRector.php index f8d16f41a43..41c0cab3fe1 100644 --- a/rules/TypeDeclaration/Rector/ClassMethod/AddParamTypeFromPropertyTypeRector.php +++ b/rules/TypeDeclaration/Rector/ClassMethod/AddParamTypeFromPropertyTypeRector.php @@ -123,7 +123,7 @@ function (Node $node) use ($paramName): bool { continue; } - if ($this->parentClassMethodTypeOverrideGuard->hasParentClassMethod($node)) { + if ($this->parentClassMethodTypeOverrideGuard->hasParentClassMethod($node) !== false) { return null; } diff --git a/rules/TypeDeclaration/Rector/ClassMethod/ParamTypeByMethodCallTypeRector.php b/rules/TypeDeclaration/Rector/ClassMethod/ParamTypeByMethodCallTypeRector.php index a316b784867..f7f5fa358ff 100644 --- a/rules/TypeDeclaration/Rector/ClassMethod/ParamTypeByMethodCallTypeRector.php +++ b/rules/TypeDeclaration/Rector/ClassMethod/ParamTypeByMethodCallTypeRector.php @@ -146,7 +146,7 @@ private function shouldSkipClassMethod(ClassMethod $classMethod): bool return true; } - return $this->parentClassMethodTypeOverrideGuard->hasParentClassMethod($classMethod); + return $this->parentClassMethodTypeOverrideGuard->hasParentClassMethod($classMethod) !== false; } private function mirrorParamType( diff --git a/rules/TypeDeclaration/Rector/ClassMethod/ReturnTypeFromStrictParamRector.php b/rules/TypeDeclaration/Rector/ClassMethod/ReturnTypeFromStrictParamRector.php index caf2ef3a48b..fee82c31a30 100644 --- a/rules/TypeDeclaration/Rector/ClassMethod/ReturnTypeFromStrictParamRector.php +++ b/rules/TypeDeclaration/Rector/ClassMethod/ReturnTypeFromStrictParamRector.php @@ -202,7 +202,7 @@ private function shouldSkipNode(ClassMethod|Function_|Closure $node): bool } if ($node instanceof ClassMethod) { - if ($this->parentClassMethodTypeOverrideGuard->hasParentClassMethod($node)) { + if ($this->parentClassMethodTypeOverrideGuard->hasParentClassMethod($node) !== false) { return true; } diff --git a/rules/TypeDeclaration/Rector/ClassMethod/StrictArrayParamDimFetchRector.php b/rules/TypeDeclaration/Rector/ClassMethod/StrictArrayParamDimFetchRector.php index 576a590b5fe..ec2e97ea955 100644 --- a/rules/TypeDeclaration/Rector/ClassMethod/StrictArrayParamDimFetchRector.php +++ b/rules/TypeDeclaration/Rector/ClassMethod/StrictArrayParamDimFetchRector.php @@ -76,7 +76,7 @@ public function refactor(Node $node): ?Node { $hasChanged = false; - if ($node instanceof ClassMethod && $this->parentClassMethodTypeOverrideGuard->hasParentClassMethod($node)) { + if ($node instanceof ClassMethod && $this->parentClassMethodTypeOverrideGuard->hasParentClassMethod($node) !== false) { return null; } diff --git a/rules/TypeDeclaration/Rector/ClassMethod/StrictStringParamConcatRector.php b/rules/TypeDeclaration/Rector/ClassMethod/StrictStringParamConcatRector.php index e8ff5d77f2b..f19113d7c11 100644 --- a/rules/TypeDeclaration/Rector/ClassMethod/StrictStringParamConcatRector.php +++ b/rules/TypeDeclaration/Rector/ClassMethod/StrictStringParamConcatRector.php @@ -75,7 +75,7 @@ public function getNodeTypes(): array */ public function refactor(Node $node): ?Node { - if ($node instanceof ClassMethod && $this->parentClassMethodTypeOverrideGuard->hasParentClassMethod($node)) { + if ($node instanceof ClassMethod && $this->parentClassMethodTypeOverrideGuard->hasParentClassMethod($node) !== false) { return null; } diff --git a/rules/TypeDeclaration/Rector/Param/ParamTypeFromStrictTypedPropertyRector.php b/rules/TypeDeclaration/Rector/Param/ParamTypeFromStrictTypedPropertyRector.php index 37adc45e238..fe6ff60f39a 100644 --- a/rules/TypeDeclaration/Rector/Param/ParamTypeFromStrictTypedPropertyRector.php +++ b/rules/TypeDeclaration/Rector/Param/ParamTypeFromStrictTypedPropertyRector.php @@ -115,7 +115,7 @@ private function decorateParamWithType(ClassMethod $classMethod, Param $param, S return null; } - if ($this->parentClassMethodTypeOverrideGuard->hasParentClassMethod($classMethod)) { + if ($this->parentClassMethodTypeOverrideGuard->hasParentClassMethod($classMethod) !== false) { return null; } From 0f9cec0949cc9ffd9be27c7820f0158c66a97a6c Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Mon, 7 Aug 2023 12:02:43 +0200 Subject: [PATCH 6/7] fix --- packages/VendorLocker/ParentClassMethodTypeOverrideGuard.php | 1 + .../Fixture/skip_unknown_parent.php.inc | 2 +- rules/Privatization/Guard/ParentPropertyLookupGuard.php | 1 + src/PhpParser/Node/Value/ValueResolver.php | 1 + 4 files changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/VendorLocker/ParentClassMethodTypeOverrideGuard.php b/packages/VendorLocker/ParentClassMethodTypeOverrideGuard.php index 107a2fd9f76..d31d44c6d08 100644 --- a/packages/VendorLocker/ParentClassMethodTypeOverrideGuard.php +++ b/packages/VendorLocker/ParentClassMethodTypeOverrideGuard.php @@ -91,6 +91,7 @@ private function resolveParentClassMethod(ClassMethod $classMethod): ?MethodRefl } private function hasClassParent(ClassReflection $classReflection):bool { + // XXX rework this hack, after https://github.com/phpstan/phpstan-src/pull/2563 landed $nativeReflection = $classReflection->getNativeReflection(); $betterReflectionClass = $this->privatesAccessor->getPrivateProperty($nativeReflection, 'betterReflectionClass'); $parentClassName = $this->privatesAccessor->getPrivateProperty($betterReflectionClass, 'parentClassName'); diff --git a/rules-tests/TypeDeclaration/Rector/ClassMethod/ParamTypeByMethodCallTypeRector/Fixture/skip_unknown_parent.php.inc b/rules-tests/TypeDeclaration/Rector/ClassMethod/ParamTypeByMethodCallTypeRector/Fixture/skip_unknown_parent.php.inc index 55f270faef7..41285f968dc 100644 --- a/rules-tests/TypeDeclaration/Rector/ClassMethod/ParamTypeByMethodCallTypeRector/Fixture/skip_unknown_parent.php.inc +++ b/rules-tests/TypeDeclaration/Rector/ClassMethod/ParamTypeByMethodCallTypeRector/Fixture/skip_unknown_parent.php.inc @@ -5,7 +5,7 @@ namespace Rector\Tests\TypeDeclaration\Rector\ClassMethod\ParamTypeByMethodCallT use Rector\Tests\TypeDeclaration\Rector\ClassMethod\ParamTypeByMethodCallTypeRector\Source\GoOneWayInterface; use Rector\Tests\TypeDeclaration\Rector\ClassMethod\ParamTypeByMethodCallTypeRector\Source\SomeTypedService; -final class SkipUnknownParent implements UnknownParentClass +final class SkipUnknownParent extends UnknownParentClass { public function __construct( private SomeTypedService $someTypedService diff --git a/rules/Privatization/Guard/ParentPropertyLookupGuard.php b/rules/Privatization/Guard/ParentPropertyLookupGuard.php index cffad4a4448..1935800134f 100644 --- a/rules/Privatization/Guard/ParentPropertyLookupGuard.php +++ b/rules/Privatization/Guard/ParentPropertyLookupGuard.php @@ -48,6 +48,7 @@ public function isLegal(Property $property, ?ClassReflection $classReflection): return false; } + // XXX rework this hack, after https://github.com/phpstan/phpstan-src/pull/2563 landed $nativeReflection = $classReflection->getNativeReflection(); $betterReflectionClass = $this->privatesAccessor->getPrivateProperty( $nativeReflection, diff --git a/src/PhpParser/Node/Value/ValueResolver.php b/src/PhpParser/Node/Value/ValueResolver.php index 17eb848866f..d10bcdded66 100644 --- a/src/PhpParser/Node/Value/ValueResolver.php +++ b/src/PhpParser/Node/Value/ValueResolver.php @@ -333,6 +333,7 @@ private function resolveClassFromSelfStaticParent(ClassConstFetch $classConstFet ); } + // XXX rework this hack, after https://github.com/phpstan/phpstan-src/pull/2563 landed // ensure parent class name still resolved even not autoloaded $nativeReflection = $classReflection->getNativeReflection(); $betterReflectionClass = $this->privatesAccessor->getPrivateProperty( From d3ab03fc9865c8dd0adf41cb3b91a6e5c368cfdc Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Mon, 7 Aug 2023 12:06:07 +0200 Subject: [PATCH 7/7] rename exception class --- ...tException.php => UnresolvableClassException.php} | 2 +- .../ParentClassMethodTypeOverrideGuard.php | 12 ++++++------ 2 files changed, 7 insertions(+), 7 deletions(-) rename packages/VendorLocker/Exception/{UnresolvableClassParentException.php => UnresolvableClassException.php} (55%) diff --git a/packages/VendorLocker/Exception/UnresolvableClassParentException.php b/packages/VendorLocker/Exception/UnresolvableClassException.php similarity index 55% rename from packages/VendorLocker/Exception/UnresolvableClassParentException.php rename to packages/VendorLocker/Exception/UnresolvableClassException.php index 44cae56c339..2936f4e1bab 100644 --- a/packages/VendorLocker/Exception/UnresolvableClassParentException.php +++ b/packages/VendorLocker/Exception/UnresolvableClassException.php @@ -4,6 +4,6 @@ namespace Rector\VendorLocker\Exception; -final class UnresolvableClassParentException extends \Exception +final class UnresolvableClassException extends \Exception { } diff --git a/packages/VendorLocker/ParentClassMethodTypeOverrideGuard.php b/packages/VendorLocker/ParentClassMethodTypeOverrideGuard.php index d31d44c6d08..266dbc5223f 100644 --- a/packages/VendorLocker/ParentClassMethodTypeOverrideGuard.php +++ b/packages/VendorLocker/ParentClassMethodTypeOverrideGuard.php @@ -14,7 +14,7 @@ use Rector\NodeNameResolver\NodeNameResolver; use Rector\NodeTypeResolver\TypeComparator\TypeComparator; use Rector\StaticTypeMapper\StaticTypeMapper; -use Rector\VendorLocker\Exception\UnresolvableClassParentException; +use Rector\VendorLocker\Exception\UnresolvableClassException; final class ParentClassMethodTypeOverrideGuard { @@ -33,7 +33,7 @@ public function hasParentClassMethod(ClassMethod $classMethod): ?bool $parentClassMethod = $this->resolveParentClassMethod($classMethod); return $parentClassMethod instanceof MethodReflection; - } catch (UnresolvableClassParentException) { + } catch (UnresolvableClassException) { // we don't know all involved parents. return null; } @@ -45,21 +45,21 @@ public function getParentClassMethod(ClassMethod $classMethod): ?MethodReflectio $parentClassMethod = $this->resolveParentClassMethod($classMethod); return $parentClassMethod; - } catch (UnresolvableClassParentException) { + } catch (UnresolvableClassException) { // we don't know all involved parents. throw new ShouldNotHappenException('Unable to resolve involved class. You are likely missing hasParentClassMethod() before calling getParentClassMethod().'); } } /** - * @throws UnresolvableClassParentException + * @throws UnresolvableClassException */ private function resolveParentClassMethod(ClassMethod $classMethod): ?MethodReflection { $classReflection = $this->reflectionResolver->resolveClassReflection($classMethod); if (! $classReflection instanceof ClassReflection) { // we can't resolve the class, so we don't know. - throw new UnresolvableClassParentException(); + throw new UnresolvableClassException(); } /** @var string $methodName */ @@ -69,7 +69,7 @@ private function resolveParentClassMethod(ClassMethod $classMethod): ?MethodRefl $parentClassReflection = $currentClassReflection->getParentClass(); if (!$parentClassReflection instanceof ClassReflection) { // per AST we have a parent class, but our reflection classes are not able to load its class definition/signature - throw new UnresolvableClassParentException(); + throw new UnresolvableClassException(); } if ($parentClassReflection->hasNativeMethod($methodName)) {