Skip to content

Commit

Permalink
[TASK] Make return type of Repository::findAll() more stable
Browse files Browse the repository at this point in the history
Adjust the return type of Repository::findAll based on context

References #103
  • Loading branch information
sascha-egerer committed Nov 1, 2022
1 parent 778aa50 commit ac956ae
Show file tree
Hide file tree
Showing 4 changed files with 140 additions and 20 deletions.
14 changes: 14 additions & 0 deletions phpstan.neon
Original file line number Diff line number Diff line change
Expand Up @@ -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\\<int, SaschaEgerer\\\\PhpstanTypo3\\\\Tests\\\\Unit\\\\Type\\\\QueryResultToArrayDynamicReturnTypeExtension\\\\FrontendUserGroup\\>\\|TYPO3\\\\CMS\\\\Extbase\\\\Persistence\\\\QueryResultInterface\\<SaschaEgerer\\\\PhpstanTypo3\\\\Tests\\\\Unit\\\\Type\\\\QueryResultToArrayDynamicReturnTypeExtension\\\\FrontendUserGroup\\>\\) of method TYPO3\\\\CMS\\\\Extbase\\\\Persistence\\\\Repository\\<SaschaEgerer\\\\PhpstanTypo3\\\\Tests\\\\Unit\\\\Type\\\\QueryResultToArrayDynamicReturnTypeExtension\\\\FrontendUserGroup\\>\\:\\:findAll\\(\\)$#"
count: 1
path: tests/Unit/Type/QueryResultToArrayDynamicReturnTypeExtension/data/query-result-to-array.php
17 changes: 5 additions & 12 deletions src/Type/QueryResultToArrayDynamicReturnTypeExtension.php
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

/**
Expand Down
66 changes: 63 additions & 3 deletions src/Type/RepositoryFindAllDynamicReturnTypeExtension.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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;
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ class FrontendUserGroupRepository extends Repository
/**
* @extends Repository<FrontendUserGroup>
*/
class FrontendUserCustomFindAllGroupRepository extends Repository
class FrontendUserGroupCustomFindAllRepository extends Repository
{

/**
Expand All @@ -38,6 +38,36 @@ public function findAll(): QueryResultInterface // phpcs:ignore SlevomatCodingSt

}

/**
* @extends Repository<FrontendUserGroup>
*/
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<FrontendUserGroup>
*/
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
{

Expand All @@ -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
Expand All @@ -75,12 +115,25 @@ public function showAction(): void
$myObjects
);

$queryResult = $this->myCustomFindAllRepository->findAll();
$myObjects = $queryResult->toArray();
assertType(
'array<int, SaschaEgerer\PhpstanTypo3\Tests\Unit\Type\QueryResultToArrayDynamicReturnTypeExtension\FrontendUserGroup>',
$this->myCustomFindAllRepository->findAll()->toArray()
$myObjects
);

$queryResult = $this->myCustomFindAllRepository->findAll();
$queryResult = $this->myCustomFindAllRepositoryWithoutModelAnnotation->findAll();
$myObjects = $queryResult->toArray();
assertType(
'array<int, SaschaEgerer\PhpstanTypo3\Tests\Unit\Type\QueryResultToArrayDynamicReturnTypeExtension\FrontendUserGroup>',
$myObjects
);

$queryResult = $this->myCustomFindAllRepositoryWithoutAnnotationRepository->findAll();
if (is_array($queryResult)) {
return;
}

$myObjects = $queryResult->toArray();
assertType(
'array<int, SaschaEgerer\PhpstanTypo3\Tests\Unit\Type\QueryResultToArrayDynamicReturnTypeExtension\FrontendUserGroup>',
Expand Down

0 comments on commit ac956ae

Please sign in to comment.