Merge pull request #178 from sascha-egerer/bugfix/fix-object-storage-template

Fix invalid interface implementation by dynamic return type

Author: sascha-egerer

extension.neon

@@ -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:

phpstan.neon

@@ -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

src/Type/ObjectStorageDynamicReturnTypeExtension.php

@@ -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();
+	}
+
+}

stubs/ObjectStorage.stub

@@ -2,16 +2,16 @@
 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}
  */
 class ObjectStorage implements \Iterator, \ArrayAccess
 {
     /**
-     * @param TEntity|int|string $value
-     * @return ($value is int ? TEntity|null : mixed)
+     * @param TEntity|string|int $value
+     * @phpstan-return TEntity|null
      */
-    public function offsetGet($value);
+    public function offsetGet(mixed $value);
 }

stubs/QueryResultInterface.stub

@@ -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>

tests/Unit/Type/QueryResultToArrayDynamicReturnTypeExtension/data/query-result-to-array.php

@@ -135,6 +135,12 @@ public function showAction(): void
 			'list<SaschaEgerer\PhpstanTypo3\Tests\Unit\Type\QueryResultToArrayDynamicReturnTypeExtension\FrontendUserGroup>',
 			$myObjects
 		);
+
+		$key = $queryResult->key();
+		assertType(
+			'int',
+			$key
+		);
 	}
 
 }

tests/Unit/Type/data/object-storage-stub-files.php

@@ -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));
 	}
 
 }