From ac956aee32ae534c9f0034638d81ab5d6f3a1317 Mon Sep 17 00:00:00 2001 From: Sascha Egerer Date: Tue, 1 Nov 2022 14:59:21 +0100 Subject: [PATCH] [TASK] Make return type of Repository::findAll() more stable Adjust the return type of Repository::findAll based on context References #103 --- phpstan.neon | 14 ++++ ...esultToArrayDynamicReturnTypeExtension.php | 17 ++--- ...itoryFindAllDynamicReturnTypeExtension.php | 66 ++++++++++++++++++- .../data/query-result-to-array.php | 63 ++++++++++++++++-- 4 files changed, 140 insertions(+), 20 deletions(-) diff --git a/phpstan.neon b/phpstan.neon index ee4370b..12d242c 100644 --- a/phpstan.neon +++ b/phpstan.neon @@ -22,3 +22,17 @@ parameters: message: "#^Calling PHPStan\\\\Reflection\\\\InitializerExprTypeResolver\\:\\:getClassConstFetchType\\(\\) is not covered by backward compatibility promise\\. The method might change in a minor PHPStan version\\.$#" count: 1 path: src/Rule/ValidatorResolverOptionsRule.php + - + message: "#^Method SaschaEgerer\\\\PhpstanTypo3\\\\Tests\\\\Unit\\\\Type\\\\QueryResultToArrayDynamicReturnTypeExtension\\\\FrontendUserGroupCustomFindAllWithoutModelTypeRepository\\:\\:findAll\\(\\) return type with generic interface TYPO3\\\\CMS\\\\Extbase\\\\Persistence\\\\QueryResultInterface does not specify its types\\: ModelType$#" + count: 1 + path: tests/Unit/Type/QueryResultToArrayDynamicReturnTypeExtension/data/query-result-to-array.php + + - + message: "#^PHPDoc tag @var for variable \\$queryResult contains generic class TYPO3\\\\CMS\\\\Extbase\\\\Persistence\\\\Generic\\\\QueryResult but does not specify its types\\: ModelType$#" + count: 2 + path: tests/Unit/Type/QueryResultToArrayDynamicReturnTypeExtension/data/query-result-to-array.php + + - + message: "#^Return type \\(TYPO3\\\\CMS\\\\Extbase\\\\Persistence\\\\QueryResultInterface\\) of method SaschaEgerer\\\\PhpstanTypo3\\\\Tests\\\\Unit\\\\Type\\\\QueryResultToArrayDynamicReturnTypeExtension\\\\FrontendUserGroupCustomFindAllWithoutModelTypeRepository\\:\\:findAll\\(\\) should be covariant with return type \\(array\\\\|TYPO3\\\\CMS\\\\Extbase\\\\Persistence\\\\QueryResultInterface\\\\) of method TYPO3\\\\CMS\\\\Extbase\\\\Persistence\\\\Repository\\\\:\\:findAll\\(\\)$#" + count: 1 + path: tests/Unit/Type/QueryResultToArrayDynamicReturnTypeExtension/data/query-result-to-array.php diff --git a/src/Type/QueryResultToArrayDynamicReturnTypeExtension.php b/src/Type/QueryResultToArrayDynamicReturnTypeExtension.php index 753d59d..294aea4 100644 --- a/src/Type/QueryResultToArrayDynamicReturnTypeExtension.php +++ b/src/Type/QueryResultToArrayDynamicReturnTypeExtension.php @@ -49,21 +49,14 @@ public function getTypeFromMethodCall( } if ($resultType instanceof GenericObjectType) { - $modelType = $resultType->getTypes(); + $modelType = $resultType->getTypes()[0] ?? new ErrorType(); } else { - $classReflection = $scope->getClassReflection(); - if ($classReflection === null) { - return new ErrorType(); - } - - $modelName = $this->translateRepositoryNameToModelName( - $classReflection->getName() - ); - - $modelType = [new ObjectType($modelName)]; + $modelType = $methodReflection->getDeclaringClass() + ->getPossiblyIncompleteActiveTemplateTypeMap() + ->getType('ModelType') ?? new ErrorType(); } - return new ArrayType(new IntegerType(), $modelType[0]); + return new ArrayType(new IntegerType(), $modelType); } /** diff --git a/src/Type/RepositoryFindAllDynamicReturnTypeExtension.php b/src/Type/RepositoryFindAllDynamicReturnTypeExtension.php index 556aec6..136dcec 100644 --- a/src/Type/RepositoryFindAllDynamicReturnTypeExtension.php +++ b/src/Type/RepositoryFindAllDynamicReturnTypeExtension.php @@ -4,14 +4,19 @@ use PhpParser\Node\Expr\MethodCall; use PHPStan\Analyser\Scope; +use PHPStan\PhpDoc\Tag\ExtendsTag; use PHPStan\Reflection\MethodReflection; use PHPStan\Reflection\ParametersAcceptorSelector; use PHPStan\Type\DynamicMethodReturnTypeExtension; +use PHPStan\Type\ErrorType; use PHPStan\Type\Generic\GenericObjectType; +use PHPStan\Type\Generic\TemplateType; use PHPStan\Type\ObjectType; use PHPStan\Type\Type; +use PHPStan\Type\TypeTraverser; use PHPStan\Type\TypeWithClassName; use SaschaEgerer\PhpstanTypo3\Helpers\Typo3ClassNamingUtilityTrait; +use TYPO3\CMS\Extbase\DomainObject\DomainObjectInterface; use TYPO3\CMS\Extbase\Persistence\QueryResultInterface; use TYPO3\CMS\Extbase\Persistence\Repository; @@ -39,18 +44,73 @@ public function getTypeFromMethodCall( ): Type { $variableType = $scope->getType($methodCall->var); + $methodReturnType = ParametersAcceptorSelector::selectSingle($methodReflection->getVariants())->getReturnType(); + if (!$variableType instanceof TypeWithClassName) { + return $methodReturnType; + } - if (!$variableType instanceof TypeWithClassName - || $methodReflection->getDeclaringClass()->getName() !== Repository::class) { - return ParametersAcceptorSelector::selectSingle($methodReflection->getVariants())->getReturnType(); + $methodReturnTypeGeneric = $this->getGenericTypes($methodReturnType)[0] ?? null; + if ( + $methodReturnTypeGeneric instanceof GenericObjectType && + ($methodReturnTypeGeneric->getTypes()[0] ?? null) instanceof ObjectType && + $methodReturnTypeGeneric->getTypes()[0]->getClassName() !== DomainObjectInterface::class + ) { + if ($methodReflection->getDeclaringClass()->getName() !== Repository::class) { + return $methodReturnType; + } + return $methodReturnTypeGeneric; } /** @var class-string $className */ $className = $variableType->getClassName(); + // if we have a custom findAll method... + if ($methodReflection->getDeclaringClass()->getName() !== Repository::class) { + if ($methodReturnType->getIterableValueType() instanceof ObjectType) { + return $methodReturnType; + } + + if ($variableType->getClassReflection() !== null) { + $repositoryExtendsTags = $variableType->getClassReflection()->getExtendsTags()[Repository::class] ?? null; + if ($repositoryExtendsTags instanceof ExtendsTag && $repositoryExtendsTags->getType() instanceof GenericObjectType) { + return new GenericObjectType(QueryResultInterface::class, [$repositoryExtendsTags->getType()->getTypes()[0] ?? new ErrorType()]); + } + } + /** @var class-string $className */ + $className = $methodReflection->getDeclaringClass()->getName(); + } + $modelName = $this->translateRepositoryNameToModelName($className); return new GenericObjectType(QueryResultInterface::class, [new ObjectType($modelName)]); } + /** + * @return GenericObjectType[] + */ + private function getGenericTypes(Type $baseType): array + { + $genericObjectTypes = []; + TypeTraverser::map($baseType, static function (Type $type, callable $traverse) use (&$genericObjectTypes): Type { + if ($type instanceof GenericObjectType) { + $resolvedType = TypeTraverser::map($type, static function (Type $type, callable $traverse): Type { + if ($type instanceof TemplateType) { + return $traverse($type->getBound()); + } + return $traverse($type); + }); + if (!$resolvedType instanceof GenericObjectType) { + throw new \PHPStan\ShouldNotHappenException(); + } + $genericObjectTypes[] = $resolvedType; + $traverse($type); + return $type; + } + $traverse($type); + return $type; + }); + + return $genericObjectTypes; + } + } diff --git a/tests/Unit/Type/QueryResultToArrayDynamicReturnTypeExtension/data/query-result-to-array.php b/tests/Unit/Type/QueryResultToArrayDynamicReturnTypeExtension/data/query-result-to-array.php index 1b4e541..4f8dced 100644 --- a/tests/Unit/Type/QueryResultToArrayDynamicReturnTypeExtension/data/query-result-to-array.php +++ b/tests/Unit/Type/QueryResultToArrayDynamicReturnTypeExtension/data/query-result-to-array.php @@ -23,7 +23,7 @@ class FrontendUserGroupRepository extends Repository /** * @extends Repository */ -class FrontendUserCustomFindAllGroupRepository extends Repository +class FrontendUserGroupCustomFindAllRepository extends Repository { /** @@ -38,6 +38,36 @@ public function findAll(): QueryResultInterface // phpcs:ignore SlevomatCodingSt } +/** + * @extends Repository + */ +class FrontendUserGroupCustomFindAllWithoutModelTypeRepository extends Repository +{ + + public function findAll(): QueryResultInterface // phpcs:ignore SlevomatCodingStandard.TypeHints.ReturnTypeHint.MissingAnyTypeHint + { + $queryResult = null; // phpcs:ignore SlevomatCodingStandard.Variables.UselessVariable.UselessVariable + /** @var QueryResult $queryResult */ + return $queryResult; + } + +} + +/** + * @extends Repository + */ +class FrontendUserGroupCustomFindAllWithoutAnnotationRepository extends Repository +{ + + public function findAll() // phpcs:ignore SlevomatCodingStandard.TypeHints.ReturnTypeHint.MissingAnyTypeHint + { + $queryResult = null; // phpcs:ignore SlevomatCodingStandard.Variables.UselessVariable.UselessVariable + /** @var QueryResult $queryResult */ + return $queryResult; + } + +} + class FrontendUserGroup extends AbstractEntity { @@ -49,16 +79,26 @@ class MyController extends ActionController /** @var FrontendUserGroupRepository */ private $myRepository; - /** @var FrontendUserCustomFindAllGroupRepository */ + /** @var FrontendUserGroupCustomFindAllRepository */ private $myCustomFindAllRepository; + /** @var FrontendUserGroupCustomFindAllWithoutModelTypeRepository */ + private $myCustomFindAllRepositoryWithoutModelAnnotation; + + /** @var FrontendUserGroupCustomFindAllWithoutAnnotationRepository */ + private $myCustomFindAllRepositoryWithoutAnnotationRepository; + public function __construct( FrontendUserGroupRepository $myRepository, - FrontendUserCustomFindAllGroupRepository $myCustomFindAllRepository + FrontendUserGroupCustomFindAllRepository $myCustomFindAllRepository, + FrontendUserGroupCustomFindAllWithoutModelTypeRepository $myCustomFindAllRepositoryWithoutModelAnnotation, + FrontendUserGroupCustomFindAllWithoutAnnotationRepository $myCustomFindAllRepositoryWithoutAnnotationRepository ) { $this->myRepository = $myRepository; $this->myCustomFindAllRepository = $myCustomFindAllRepository; + $this->myCustomFindAllRepositoryWithoutModelAnnotation = $myCustomFindAllRepositoryWithoutModelAnnotation; + $this->myCustomFindAllRepositoryWithoutAnnotationRepository = $myCustomFindAllRepositoryWithoutAnnotationRepository; } public function showAction(): void @@ -75,12 +115,25 @@ public function showAction(): void $myObjects ); + $queryResult = $this->myCustomFindAllRepository->findAll(); + $myObjects = $queryResult->toArray(); assertType( 'array', - $this->myCustomFindAllRepository->findAll()->toArray() + $myObjects ); - $queryResult = $this->myCustomFindAllRepository->findAll(); + $queryResult = $this->myCustomFindAllRepositoryWithoutModelAnnotation->findAll(); + $myObjects = $queryResult->toArray(); + assertType( + 'array', + $myObjects + ); + + $queryResult = $this->myCustomFindAllRepositoryWithoutAnnotationRepository->findAll(); + if (is_array($queryResult)) { + return; + } + $myObjects = $queryResult->toArray(); assertType( 'array',