Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix invalid interface implementation by dynamic return type #178

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions extension.neon
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,10 @@ services:
class: SaschaEgerer\PhpstanTypo3\Type\ValidatorResolverDynamicReturnTypeExtension
tags:
- phpstan.broker.dynamicMethodReturnTypeExtension
-
class: SaschaEgerer\PhpstanTypo3\Type\ObjectStorageDynamicReturnTypeExtension
tags:
- phpstan.broker.dynamicMethodReturnTypeExtension
-
class: SaschaEgerer\PhpstanTypo3\Type\ContextDynamicReturnTypeExtension
arguments:
Expand Down
15 changes: 0 additions & 15 deletions phpstan.neon
Original file line number Diff line number Diff line change
Expand Up @@ -18,25 +18,10 @@ parameters:
- '*tests/*/Source/*'
- '*tests/*/data/*'
ignoreErrors:
-
message: '#^Class TYPO3\\CMS\\Core\\Context\\[a-zA-Z]* not found\.#'
path: src/Type/ContextDynamicReturnTypeExtension.php
-
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
-
message: '#^Although PHPStan\\Reflection\\Php\\PhpPropertyReflection is covered by backward compatibility promise, this instanceof assumption might break because it''s not guaranteed to always stay the same\.$#'
identifier: phpstanApi.instanceofAssumption
Expand Down
56 changes: 56 additions & 0 deletions src/Type/ObjectStorageDynamicReturnTypeExtension.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
<?php declare(strict_types = 1);

namespace SaschaEgerer\PhpstanTypo3\Type;

use PhpParser\Node\Arg;
use PhpParser\Node\Expr\MethodCall;
use PHPStan\Analyser\Scope;
use PHPStan\Reflection\MethodReflection;
use PHPStan\Type\DynamicMethodReturnTypeExtension;
use PHPStan\Type\IntegerType;
use PHPStan\Type\MixedType;
use PHPStan\Type\StringType;
use PHPStan\Type\Type;
use TYPO3\CMS\Extbase\Persistence\ObjectStorage;

class ObjectStorageDynamicReturnTypeExtension implements DynamicMethodReturnTypeExtension
{

public function getClass(): string
{
return ObjectStorage::class;
}

public function isMethodSupported(
MethodReflection $methodReflection
): bool
{
return $methodReflection->getName() === 'offsetGet';
}

public function getTypeFromMethodCall(
MethodReflection $methodReflection,
MethodCall $methodCall,
Scope $scope
): ?Type
{
$firstArgument = $methodCall->args[0] ?? null;

if (!$firstArgument instanceof Arg) {
return null;
}

$argumentType = $scope->getType($firstArgument->value);

if ((new StringType())->isSuperTypeOf($argumentType)->yes()) {
return $methodReflection->getVariants()[0]->getReturnType();
}

if ((new IntegerType())->isSuperTypeOf($argumentType)->yes()) {
return $methodReflection->getVariants()[0]->getReturnType();
}

return new MixedType();
}

}
18 changes: 8 additions & 10 deletions stubs/ObjectStorage.stub
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
namespace TYPO3\CMS\Extbase\Persistence;

/**
* @template TEntity
* @template TEntity of object
* @implements \ArrayAccess<string, TEntity>
* @implements \Iterator<string, TEntity>
* @phpstan-type ObjectStorageInternal array{obj: TEntity, inf: mixed}
Expand All @@ -21,29 +21,27 @@ class ObjectStorage implements \Iterator, \ArrayAccess
public function offsetSet($value, $information);

/**
* @param TEntity|int|string $value
* @return bool
* @param TEntity|string|int $value
* @phpstan-return TEntity|null
*/
public function offsetExists($value);
public function offsetGet(mixed $value);

/**
* @param TEntity|int|string $value
* @return bool
*/
public function offsetUnset($value);
public function offsetExists($value);

/**
* @param TEntity|int|string $value
* @return ($value is int ? TEntity|null : mixed)
*/
public function offsetGet($value);
public function offsetUnset($value);

/**
* This is different from the SplObjectStorage as the key in this implementation is the object hash (string).
* @phpstan-ignore-next-line See https://forge.typo3.org/issues/98146
* @return string
*/
// @phpstan-ignore-next-line See https://forge.typo3.org/issues/98146
public function key();
public function key(): string;

/**
* @return list<TEntity>
Expand Down
2 changes: 1 addition & 1 deletion stubs/QueryResultInterface.stub
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
namespace TYPO3\CMS\Extbase\Persistence;

/**
* @template TKey
* @template TKey of int
* @template TValue of object
* @extends \Iterator<TKey, TValue>
* @extends \ArrayAccess<TKey, TValue>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,12 @@ public function showAction(): void
'list<SaschaEgerer\PhpstanTypo3\Tests\Unit\Type\QueryResultToArrayDynamicReturnTypeExtension\FrontendUserGroup>',
$myObjects
);

$key = $queryResult->key();
assertType(
'int',
$key
);
}

}
30 changes: 20 additions & 10 deletions tests/Unit/Type/data/object-storage-stub-files.php
Original file line number Diff line number Diff line change
Expand Up @@ -13,30 +13,40 @@
class MyModel extends AbstractEntity
{

public function foo(): void
/**
* @var ObjectStorage<self>
*/
protected ObjectStorage $testStorage;

public function checkObjectStorageType(): void
{
$myModel = new self();
/** @var ObjectStorage<MyModel> $objectStorage */
$objectStorage = new ObjectStorage();
$objectStorage->attach($myModel);

assertType('TYPO3\CMS\Extbase\Persistence\ObjectStorage<' . self::class . '>', $objectStorage);
}

foreach ($objectStorage as $key => $value) {

public function checkIteration(): void
{
foreach ($this->testStorage as $key => $value) {
assertType('string', $key);
assertType(self::class, $value);
}
}

assertType(self::class . '|null', $objectStorage->offsetGet(0));
public function checkArrayAccess(): void
{
assertType(self::class . '|null', $this->testStorage->offsetGet(0));
assertType(self::class . '|null', $this->testStorage->offsetGet('0'));
assertType(self::class . '|null', $this->testStorage->current());
assertType(self::class . '|null', $this->testStorage[0]);

// We ignore errors in the next line as this will produce an
// "Offset 0 does not exist on TYPO3\CMS\Extbase\Persistence\ObjectStorage<ObjectStorage\My\Test\Extension\Domain\Model\MyModel>
// due to the weird implementation of ArrayAccess in ObjectStorage::offsetGet()
// @phpstan-ignore-next-line
assertType(self::class . '|null', $objectStorage[0]);
$myModel = new self();

assertType('mixed', $objectStorage->offsetGet($myModel));
assertType('mixed', $this->testStorage->offsetGet($this->testStorage->current()));
assertType('mixed', $this->testStorage->offsetGet($myModel));
}

}