Merge pull request #276 from shochdoerfer/fix/ext_attributes_param_types

Check existing ext interface for types
bitExpert · Nov 1, 2022 · f2b2f6a · f2b2f6a
2 parents bb1fec1 + b7f8346
commit f2b2f6a
Show file tree

Hide file tree

Showing 3 changed files with 85 additions and 12 deletions.
diff --git a/src/bitExpert/PHPStan/Magento/Autoload/ExtensionAutoloader.php b/src/bitExpert/PHPStan/Magento/Autoload/ExtensionAutoloader.php
@@ -20,6 +20,7 @@
 use Laminas\Code\Generator\MethodGenerator;
 use Laminas\Code\Generator\ParameterGenerator;
 use PHPStan\Cache\Cache;
+use ReflectionClass;
 
 class ExtensionAutoloader implements Autoloader
 {
@@ -67,11 +68,14 @@ public function autoload(string $class): void
 
     /**
      * Given an extension attributes interface name, generate that interface (if possible)
+     *
+     * @throws \ReflectionException
      */
     public function getFileContents(string $className): string
     {
         /** @var class-string $sourceInterface */
         $sourceInterface = rtrim(substr($className, 0, -1 * strlen('Extension')), '\\') . 'ExtensionInterface';
+        $sourceInterfaceReflection = new ReflectionClass($sourceInterface);
         /** @var class-string $attrInterface */
         $attrInterface = rtrim(substr($sourceInterface, 0, -1 * strlen('ExtensionInterface')), '\\') . 'Interface';
 
@@ -89,20 +93,57 @@ public function getFileContents(string $className): string
              * @see \Magento\Framework\Api\Code\Generator\ExtensionAttributesGenerator::_getClassMethods
              */
 
+            // check return type of method in interface and reuse it in the generated class
+            $returnType = null;
+            try {
+                $reflectionMethod = $sourceInterfaceReflection->getMethod('get' . ucfirst($propertyName));
+                $returnType = $reflectionMethod->getReturnType();
+            } catch (\Exception $e) {
+            }
+
             $generator->addMethodFromGenerator(
                 MethodGenerator::fromArray([
                     'name' => 'get' . ucfirst($propertyName),
+                    'returntype' => $returnType,
                     'docblock' => DocBlockGenerator::fromArray([
                         'tags' => [
                             new ReturnTag([$type, 'null']),
                         ],
                     ]),
                 ])
             );
+
+            // check param type of method in interface and reuse it in the generated class
+            $paramType = null;
+            try {
+                $reflectionMethod = $sourceInterfaceReflection->getMethod('set' . ucfirst($propertyName));
+                $reflectionParams = $reflectionMethod->getParameters();
+                if (isset($reflectionParams[0])) {
+                    $paramType = $reflectionParams[0]->getType();
+                    if (($paramType !== null) && $reflectionParams[0]->isOptional()) {
+                        $paramType = '?'.$paramType;
+                    }
+                }
+
+                if ($paramType !== null) {
+                    $paramType = (string) $paramType;
+                }
+            } catch (\Exception $e) {
+            }
+
+            // check return type of method in interface and reuse it in the generated class
+            $returnType = null;
+            try {
+                $reflectionMethod = $sourceInterfaceReflection->getMethod('set' . ucfirst($propertyName));
+                $returnType = $reflectionMethod->getReturnType();
+            } catch (\Exception $e) {
+            }
+
             $generator->addMethodFromGenerator(
                 MethodGenerator::fromArray([
                     'name' => 'set' . ucfirst($propertyName),
-                    'parameters' => [$propertyName],
+                    'parameters' => [new ParameterGenerator($propertyName, $paramType)],
+                    'returntype' => $returnType,
                     'docblock' => DocBlockGenerator::fromArray([
                         'tags' => [
                             new ParamTag($propertyName, [$type]),

diff --git a/src/bitExpert/PHPStan/Magento/Autoload/ExtensionInterfaceAutoloader.php b/src/bitExpert/PHPStan/Magento/Autoload/ExtensionInterfaceAutoloader.php
@@ -54,23 +54,24 @@ public function __construct(
         $this->classLoaderProvider = $classLoaderProvider;
     }
 
-    public function autoload(string $class): void
+    public function autoload(string $interfaceName): void
     {
-        if (preg_match('#ExtensionInterface$#', $class) !== 1) {
+        if (preg_match('#ExtensionInterface$#', $interfaceName) !== 1) {
             return;
         }
 
-        $cachedFilename = $this->cache->load($class, '');
-        if ($cachedFilename === null) {
-            try {
-                $this->cache->save($class, '', $this->getFileContents($class));
-                $cachedFilename = $this->cache->load($class, '');
-            } catch (\Exception $e) {
-                return;
+        // fix for PHPStan 1.7.5 and later: Classes generated by autoloaders are supposed to "win" against
+        // local classes in your project. We need to check first if classes exists locally before generating them!
+        $pathToLocalInterface = $this->classLoaderProvider->findFile($interfaceName);
+        if ($pathToLocalInterface === false) {
+            $pathToLocalInterface = $this->cache->load($interfaceName, '');
+            if ($pathToLocalInterface === null) {
+                $this->cache->save($interfaceName, '', $this->getFileContents($interfaceName));
+                $pathToLocalInterface = $this->cache->load($interfaceName, '');
             }
         }
 
-        require_once($cachedFilename);
+        require_once($pathToLocalInterface);
     }
 
     /**

diff --git a/tests/bitExpert/PHPStan/Magento/Autoload/ExtensionInterfaceAutoloaderUnitTest.php b/tests/bitExpert/PHPStan/Magento/Autoload/ExtensionInterfaceAutoloaderUnitTest.php
@@ -5,6 +5,7 @@
 use bitExpert\PHPStan\Magento\Autoload\Cache\FileCacheStorage;
 use bitExpert\PHPStan\Magento\Autoload\DataProvider\ClassLoaderProvider;
 use bitExpert\PHPStan\Magento\Autoload\DataProvider\ExtensionAttributeDataProvider;
+use InvalidArgumentException;
 use org\bovigo\vfs\vfsStream;
 use PHPStan\Cache\Cache;
 use PHPUnit\Framework\TestCase;
@@ -45,17 +46,38 @@ protected function setUp(): void
      */
     public function autoloaderIgnoresClassesWithoutExtensionInterfacePostfix(): void
     {
+        $this->classyDataProvider->expects(self::never())
+            ->method('findFile');
         $this->cache->expects(self::never())
             ->method('load');
 
         $this->autoloader->autoload('SomeClass');
     }
 
+    /**
+     * @test
+     */
+    public function autoloaderPrefersLocalFile(): void
+    {
+        $this->classyDataProvider->expects(self::once())
+            ->method('findFile')
+            ->willReturn(__DIR__ . '/HelperExtensionInterface.php');
+        $this->cache->expects(self::never())
+            ->method('load');
+
+        $this->autoloader->autoload(HelperExtensionInterface::class);
+
+        self::assertTrue(interface_exists(HelperExtensionInterface::class, false));
+    }
+
     /**
      * @test
      */
     public function autoloaderUsesCachedFileWhenFound(): void
     {
+        $this->classyDataProvider->expects(self::once())
+            ->method('findFile')
+            ->willReturn(false);
         $this->cache->expects(self::once())
             ->method('load')
             ->willReturn(__DIR__ . '/HelperExtensionInterface.php');
@@ -73,8 +95,13 @@ public function autoloaderUsesCachedFileWhenFound(): void
      */
     public function autoloadDoesNotGenerateInterfaceWhenNoAttributesExist(): void
     {
+        $this->expectException(InvalidArgumentException::class);
+
         $interfaceName = 'NonExistentExtensionInterface';
 
+        $this->classyDataProvider->expects(self::once())
+            ->method('findFile')
+            ->willReturn(false);
         $this->cache->expects(self::once())
             ->method('load')
             ->willReturn(null);
@@ -84,7 +111,6 @@ public function autoloadDoesNotGenerateInterfaceWhenNoAttributesExist(): void
             ->willReturn(false);
 
         $this->autoloader->autoload($interfaceName);
-        static::assertFalse(interface_exists($interfaceName));
     }
 
     /**
@@ -98,6 +124,10 @@ public function autoloadGeneratesInterfaceWhenNotCached(): void
         $cache = new Cache(new FileCacheStorage($root->url() . '/tmp/cache/PHPStan'));
         $autoloader = new ExtensionInterfaceAutoloader($cache, $this->extAttrDataProvider, $this->classyDataProvider);
 
+        $this->classyDataProvider->expects(self::once())
+            ->method('findFile')
+            ->willReturn(false);
+
         $this->classyDataProvider->expects(self::once())
             ->method('exists')
             ->willReturn(true);
@@ -108,6 +138,7 @@ public function autoloadGeneratesInterfaceWhenNotCached(): void
 
         $autoloader->autoload($interfaceName);
         static::assertTrue(interface_exists($interfaceName));
+
         $interfaceReflection = new \ReflectionClass($interfaceName);
         try {
             $getAttrReflection = $interfaceReflection->getMethod('getAttr');