Skip to content

Commit f2b2f6a

Browse files
authored
Merge pull request #276 from shochdoerfer/fix/ext_attributes_param_types
Check existing ext interface for types
2 parents bb1fec1 + b7f8346 commit f2b2f6a

File tree

3 files changed

+85
-12
lines changed

3 files changed

+85
-12
lines changed

src/bitExpert/PHPStan/Magento/Autoload/ExtensionAutoloader.php

Lines changed: 42 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
use Laminas\Code\Generator\MethodGenerator;
2121
use Laminas\Code\Generator\ParameterGenerator;
2222
use PHPStan\Cache\Cache;
23+
use ReflectionClass;
2324

2425
class ExtensionAutoloader implements Autoloader
2526
{
@@ -67,11 +68,14 @@ public function autoload(string $class): void
6768

6869
/**
6970
* Given an extension attributes interface name, generate that interface (if possible)
71+
*
72+
* @throws \ReflectionException
7073
*/
7174
public function getFileContents(string $className): string
7275
{
7376
/** @var class-string $sourceInterface */
7477
$sourceInterface = rtrim(substr($className, 0, -1 * strlen('Extension')), '\\') . 'ExtensionInterface';
78+
$sourceInterfaceReflection = new ReflectionClass($sourceInterface);
7579
/** @var class-string $attrInterface */
7680
$attrInterface = rtrim(substr($sourceInterface, 0, -1 * strlen('ExtensionInterface')), '\\') . 'Interface';
7781

@@ -89,20 +93,57 @@ public function getFileContents(string $className): string
8993
* @see \Magento\Framework\Api\Code\Generator\ExtensionAttributesGenerator::_getClassMethods
9094
*/
9195

96+
// check return type of method in interface and reuse it in the generated class
97+
$returnType = null;
98+
try {
99+
$reflectionMethod = $sourceInterfaceReflection->getMethod('get' . ucfirst($propertyName));
100+
$returnType = $reflectionMethod->getReturnType();
101+
} catch (\Exception $e) {
102+
}
103+
92104
$generator->addMethodFromGenerator(
93105
MethodGenerator::fromArray([
94106
'name' => 'get' . ucfirst($propertyName),
107+
'returntype' => $returnType,
95108
'docblock' => DocBlockGenerator::fromArray([
96109
'tags' => [
97110
new ReturnTag([$type, 'null']),
98111
],
99112
]),
100113
])
101114
);
115+
116+
// check param type of method in interface and reuse it in the generated class
117+
$paramType = null;
118+
try {
119+
$reflectionMethod = $sourceInterfaceReflection->getMethod('set' . ucfirst($propertyName));
120+
$reflectionParams = $reflectionMethod->getParameters();
121+
if (isset($reflectionParams[0])) {
122+
$paramType = $reflectionParams[0]->getType();
123+
if (($paramType !== null) && $reflectionParams[0]->isOptional()) {
124+
$paramType = '?'.$paramType;
125+
}
126+
}
127+
128+
if ($paramType !== null) {
129+
$paramType = (string) $paramType;
130+
}
131+
} catch (\Exception $e) {
132+
}
133+
134+
// check return type of method in interface and reuse it in the generated class
135+
$returnType = null;
136+
try {
137+
$reflectionMethod = $sourceInterfaceReflection->getMethod('set' . ucfirst($propertyName));
138+
$returnType = $reflectionMethod->getReturnType();
139+
} catch (\Exception $e) {
140+
}
141+
102142
$generator->addMethodFromGenerator(
103143
MethodGenerator::fromArray([
104144
'name' => 'set' . ucfirst($propertyName),
105-
'parameters' => [$propertyName],
145+
'parameters' => [new ParameterGenerator($propertyName, $paramType)],
146+
'returntype' => $returnType,
106147
'docblock' => DocBlockGenerator::fromArray([
107148
'tags' => [
108149
new ParamTag($propertyName, [$type]),

src/bitExpert/PHPStan/Magento/Autoload/ExtensionInterfaceAutoloader.php

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -54,23 +54,24 @@ public function __construct(
5454
$this->classLoaderProvider = $classLoaderProvider;
5555
}
5656

57-
public function autoload(string $class): void
57+
public function autoload(string $interfaceName): void
5858
{
59-
if (preg_match('#ExtensionInterface$#', $class) !== 1) {
59+
if (preg_match('#ExtensionInterface$#', $interfaceName) !== 1) {
6060
return;
6161
}
6262

63-
$cachedFilename = $this->cache->load($class, '');
64-
if ($cachedFilename === null) {
65-
try {
66-
$this->cache->save($class, '', $this->getFileContents($class));
67-
$cachedFilename = $this->cache->load($class, '');
68-
} catch (\Exception $e) {
69-
return;
63+
// fix for PHPStan 1.7.5 and later: Classes generated by autoloaders are supposed to "win" against
64+
// local classes in your project. We need to check first if classes exists locally before generating them!
65+
$pathToLocalInterface = $this->classLoaderProvider->findFile($interfaceName);
66+
if ($pathToLocalInterface === false) {
67+
$pathToLocalInterface = $this->cache->load($interfaceName, '');
68+
if ($pathToLocalInterface === null) {
69+
$this->cache->save($interfaceName, '', $this->getFileContents($interfaceName));
70+
$pathToLocalInterface = $this->cache->load($interfaceName, '');
7071
}
7172
}
7273

73-
require_once($cachedFilename);
74+
require_once($pathToLocalInterface);
7475
}
7576

7677
/**

tests/bitExpert/PHPStan/Magento/Autoload/ExtensionInterfaceAutoloaderUnitTest.php

Lines changed: 32 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
use bitExpert\PHPStan\Magento\Autoload\Cache\FileCacheStorage;
66
use bitExpert\PHPStan\Magento\Autoload\DataProvider\ClassLoaderProvider;
77
use bitExpert\PHPStan\Magento\Autoload\DataProvider\ExtensionAttributeDataProvider;
8+
use InvalidArgumentException;
89
use org\bovigo\vfs\vfsStream;
910
use PHPStan\Cache\Cache;
1011
use PHPUnit\Framework\TestCase;
@@ -45,17 +46,38 @@ protected function setUp(): void
4546
*/
4647
public function autoloaderIgnoresClassesWithoutExtensionInterfacePostfix(): void
4748
{
49+
$this->classyDataProvider->expects(self::never())
50+
->method('findFile');
4851
$this->cache->expects(self::never())
4952
->method('load');
5053

5154
$this->autoloader->autoload('SomeClass');
5255
}
5356

57+
/**
58+
* @test
59+
*/
60+
public function autoloaderPrefersLocalFile(): void
61+
{
62+
$this->classyDataProvider->expects(self::once())
63+
->method('findFile')
64+
->willReturn(__DIR__ . '/HelperExtensionInterface.php');
65+
$this->cache->expects(self::never())
66+
->method('load');
67+
68+
$this->autoloader->autoload(HelperExtensionInterface::class);
69+
70+
self::assertTrue(interface_exists(HelperExtensionInterface::class, false));
71+
}
72+
5473
/**
5574
* @test
5675
*/
5776
public function autoloaderUsesCachedFileWhenFound(): void
5877
{
78+
$this->classyDataProvider->expects(self::once())
79+
->method('findFile')
80+
->willReturn(false);
5981
$this->cache->expects(self::once())
6082
->method('load')
6183
->willReturn(__DIR__ . '/HelperExtensionInterface.php');
@@ -73,8 +95,13 @@ public function autoloaderUsesCachedFileWhenFound(): void
7395
*/
7496
public function autoloadDoesNotGenerateInterfaceWhenNoAttributesExist(): void
7597
{
98+
$this->expectException(InvalidArgumentException::class);
99+
76100
$interfaceName = 'NonExistentExtensionInterface';
77101

102+
$this->classyDataProvider->expects(self::once())
103+
->method('findFile')
104+
->willReturn(false);
78105
$this->cache->expects(self::once())
79106
->method('load')
80107
->willReturn(null);
@@ -84,7 +111,6 @@ public function autoloadDoesNotGenerateInterfaceWhenNoAttributesExist(): void
84111
->willReturn(false);
85112

86113
$this->autoloader->autoload($interfaceName);
87-
static::assertFalse(interface_exists($interfaceName));
88114
}
89115

90116
/**
@@ -98,6 +124,10 @@ public function autoloadGeneratesInterfaceWhenNotCached(): void
98124
$cache = new Cache(new FileCacheStorage($root->url() . '/tmp/cache/PHPStan'));
99125
$autoloader = new ExtensionInterfaceAutoloader($cache, $this->extAttrDataProvider, $this->classyDataProvider);
100126

127+
$this->classyDataProvider->expects(self::once())
128+
->method('findFile')
129+
->willReturn(false);
130+
101131
$this->classyDataProvider->expects(self::once())
102132
->method('exists')
103133
->willReturn(true);
@@ -108,6 +138,7 @@ public function autoloadGeneratesInterfaceWhenNotCached(): void
108138

109139
$autoloader->autoload($interfaceName);
110140
static::assertTrue(interface_exists($interfaceName));
141+
111142
$interfaceReflection = new \ReflectionClass($interfaceName);
112143
try {
113144
$getAttrReflection = $interfaceReflection->getMethod('getAttr');

0 commit comments

Comments
 (0)