Skip to content

Commit 9805fbd

Browse files
slavvkaslavvka
authored
ENGCOM-6686: Github #26532: di:setup:compile fails with anonymous classes #26533 
2 parents 47520b6 + 4af1018 commit 9805fbd

File tree

4 files changed

+139
-92
lines changed

4 files changed

+139
-92
lines changed

‎setup/src/Magento/Setup/Module/Di/Code/Scanner/PhpScanner.php

Lines changed: 63 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -3,18 +3,18 @@
33
* Copyright © Magento, Inc. All rights reserved.
44
* See COPYING.txt for license details.
55
*/
6+
declare(strict_types=1);
7+
68
namespace Magento\Setup\Module\Di\Code\Scanner;
79

810
use Magento\Framework\Api\Code\Generator\ExtensionAttributesGenerator;
911
use Magento\Framework\Api\Code\Generator\ExtensionAttributesInterfaceGenerator;
1012
use Magento\Framework\ObjectManager\Code\Generator\Factory as FactoryGenerator;
13+
use Magento\Framework\Reflection\TypeProcessor;
1114
use Magento\Setup\Module\Di\Compiler\Log\Log;
12-
use \Magento\Framework\Reflection\TypeProcessor;
1315

1416
/**
15-
* Class PhpScanner
16-
*
17-
* @package Magento\Setup\Module\Di\Code\Scanner
17+
* Finds factory and extension attributes classes which require auto-generation.
1818
*/
1919
class PhpScanner implements ScannerInterface
2020
{
@@ -50,18 +50,31 @@ public function __construct(Log $log, TypeProcessor $typeProcessor = null)
5050
* @param string $entityType
5151
* @return string[]
5252
*/
53-
protected function _findMissingClasses($file, $classReflection, $methodName, $entityType)
53+
private function findMissingFactories($file, $classReflection, $methodName, $entityType)
5454
{
5555
$missingClasses = [];
56-
if ($classReflection->hasMethod($methodName)) {
57-
$constructor = $classReflection->getMethod($methodName);
58-
$parameters = $constructor->getParameters();
59-
/** @var $parameter \ReflectionParameter */
60-
foreach ($parameters as $parameter) {
61-
preg_match('/\[\s\<\w+?>\s([\w\\\\]+)/s', $parameter->__toString(), $matches);
62-
if (isset($matches[1]) && substr($matches[1], -strlen($entityType)) == $entityType) {
63-
$missingClassName = $matches[1];
64-
if ($this->shouldGenerateClass($missingClassName, $entityType, $file)) {
56+
if (!$classReflection->hasMethod($methodName)) {
57+
return $missingClasses;
58+
}
59+
60+
$factorySuffix = '\\' . ucfirst(FactoryGenerator::ENTITY_TYPE);
61+
$constructor = $classReflection->getMethod($methodName);
62+
$parameters = $constructor->getParameters();
63+
/** @var $parameter \ReflectionParameter */
64+
foreach ($parameters as $parameter) {
65+
preg_match('/\[\s\<\w+?>\s([\w\\\\]+)/s', $parameter->__toString(), $matches);
66+
if (isset($matches[1]) && substr($matches[1], -strlen($entityType)) == $entityType) {
67+
$missingClassName = $matches[1];
68+
if ($this->shouldGenerateClass($missingClassName, $entityType, $file)) {
69+
70+
if (substr($missingClassName, -strlen($factorySuffix)) == $factorySuffix) {
71+
$entityName = rtrim(substr($missingClassName, 0, -strlen($factorySuffix)), '\\');
72+
$this->_log->add(
73+
Log::CONFIGURATION_ERROR,
74+
$missingClassName,
75+
'Invalid Factory declaration for class ' . $entityName . ' in file ' . $file
76+
);
77+
} else {
6578
$missingClasses[] = $missingClassName;
6679
}
6780
}
@@ -110,24 +123,12 @@ protected function getSourceClassName($missingClassName, $entityType)
110123
*/
111124
protected function _fetchFactories($reflectionClass, $file)
112125
{
113-
$factorySuffix = '\\' . ucfirst(FactoryGenerator::ENTITY_TYPE);
114-
$absentFactories = $this->_findMissingClasses(
126+
$absentFactories = $this->findMissingFactories(
115127
$file,
116128
$reflectionClass,
117129
'__construct',
118130
ucfirst(FactoryGenerator::ENTITY_TYPE)
119131
);
120-
foreach ($absentFactories as $key => $absentFactory) {
121-
if (substr($absentFactory, -strlen($factorySuffix)) == $factorySuffix) {
122-
$entityName = rtrim(substr($absentFactory, 0, -strlen($factorySuffix)), '\\');
123-
$this->_log->add(
124-
Log::CONFIGURATION_ERROR,
125-
$absentFactory,
126-
'Invalid Factory declaration for class ' . $entityName . ' in file ' . $file
127-
);
128-
unset($absentFactories[$key]);
129-
}
130-
}
131132
return $absentFactories;
132133
}
133134

@@ -150,21 +151,19 @@ protected function _fetchMissingExtensionAttributesClasses($reflectionClass, $fi
150151
$missingClassName = $returnType['type'];
151152
if ($this->shouldGenerateClass($missingClassName, $entityType, $file)) {
152153
$missingExtensionInterfaces[] = $missingClassName;
154+
155+
$extension = rtrim(substr($missingClassName, 0, -strlen('Interface')), '\\');
156+
if (!class_exists($extension)) {
157+
$missingExtensionInterfaces[] = $extension;
158+
}
159+
$extensionFactory = $extension . 'Factory';
160+
if (!class_exists($extensionFactory)) {
161+
$missingExtensionInterfaces[] = $extensionFactory;
162+
}
153163
}
154164
}
155-
$missingExtensionClasses = [];
156-
$missingExtensionFactories = [];
157-
foreach ($missingExtensionInterfaces as $missingExtensionInterface) {
158-
$extension = rtrim(substr($missingExtensionInterface, 0, -strlen('Interface')), '\\');
159-
if (!class_exists($extension)) {
160-
$missingExtensionClasses[] = $extension;
161-
}
162-
$extensionFactory = $extension . 'Factory';
163-
if (!class_exists($extensionFactory)) {
164-
$missingExtensionFactories[] = $extensionFactory;
165-
}
166-
}
167-
return array_merge($missingExtensionInterfaces, $missingExtensionClasses, $missingExtensionFactories);
165+
166+
return $missingExtensionInterfaces;
168167
}
169168

170169
/**
@@ -178,11 +177,11 @@ public function collectEntities(array $files)
178177
{
179178
$output = [[]];
180179
foreach ($files as $file) {
181-
$classes = $this->_getDeclaredClasses($file);
180+
$classes = $this->getDeclaredClasses($file);
182181
foreach ($classes as $className) {
183182
$reflectionClass = new \ReflectionClass($className);
184-
$output [] = $this->_fetchFactories($reflectionClass, $file);
185-
$output [] = $this->_fetchMissingExtensionAttributesClasses($reflectionClass, $file);
183+
$output[] = $this->_fetchFactories($reflectionClass, $file);
184+
$output[] = $this->_fetchMissingExtensionAttributesClasses($reflectionClass, $file);
186185
}
187186
}
188187
return array_unique(array_merge(...$output));
@@ -211,23 +210,30 @@ protected function _fetchNamespace($tokenIterator, $count, $tokens)
211210
}
212211

213212
/**
214-
* Fetch class names from tokenized PHP file
213+
* Fetches class name from tokenized PHP file.
215214
*
216215
* @param string $namespace
217216
* @param int $tokenIterator
218217
* @param int $count
219218
* @param array $tokens
220-
* @return array
219+
* @return string|null
221220
*/
222-
protected function _fetchClasses($namespace, $tokenIterator, $count, $tokens)
221+
private function fetchClass($namespace, $tokenIterator, $count, $tokens):? string
223222
{
224-
$classes = [];
223+
// anonymous classes should be omitted
224+
if (is_array($tokens[$tokenIterator - 2]) && $tokens[$tokenIterator - 2][0] === T_NEW) {
225+
return null;
226+
}
227+
225228
for ($tokenOffset = $tokenIterator + 1; $tokenOffset < $count; ++$tokenOffset) {
226-
if ($tokens[$tokenOffset] === '{') {
227-
$classes[] = $namespace . "\\" . $tokens[$tokenIterator + 2][1];
229+
if ($tokens[$tokenOffset] !== '{') {
230+
continue;
228231
}
232+
233+
return $namespace . "\\" . $tokens[$tokenIterator + 2][1];
229234
}
230-
return $classes;
235+
236+
return null;
231237
}
232238

233239
/**
@@ -236,9 +242,9 @@ protected function _fetchClasses($namespace, $tokenIterator, $count, $tokens)
236242
* @param string $file
237243
* @return array
238244
*/
239-
protected function _getDeclaredClasses($file)
245+
private function getDeclaredClasses($file): array
240246
{
241-
$classes = [[]];
247+
$classes = [];
242248
$namespaceParts = [];
243249
// phpcs:ignore
244250
$tokens = token_get_all(file_get_contents($file));
@@ -252,10 +258,13 @@ protected function _getDeclaredClasses($file)
252258
if (($tokens[$tokenIterator][0] == T_CLASS || $tokens[$tokenIterator][0] == T_INTERFACE)
253259
&& $tokens[$tokenIterator - 1][0] != T_DOUBLE_COLON
254260
) {
255-
$classes[] = $this->_fetchClasses(join('', $namespaceParts), $tokenIterator, $count, $tokens);
261+
$class = $this->fetchClass(join('', $namespaceParts), $tokenIterator, $count, $tokens);
262+
if ($class !== null && !in_array($class, $classes)) {
263+
$classes[] = $class;
264+
}
256265
}
257266
}
258-
return array_unique(array_merge(...$classes));
267+
return $classes;
259268
}
260269

261270
/**

‎setup/src/Magento/Setup/Test/Unit/Module/Di/Code/Reader/ClassesScannerTest.php

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@
33
* Copyright © Magento, Inc. All rights reserved.
44
* See COPYING.txt for license details.
55
*/
6+
declare(strict_types=1);
7+
68
namespace Magento\Setup\Test\Unit\Module\Di\Code\Reader;
79

810
use Magento\Framework\App\Filesystem\DirectoryList;
@@ -37,6 +39,6 @@ public function testGetList()
3739
$pathToScan = str_replace('\\', '/', realpath(__DIR__ . '/../../') . '/_files/app/code/Magento/SomeModule');
3840
$actual = $this->model->getList($pathToScan);
3941
$this->assertTrue(is_array($actual));
40-
$this->assertCount(5, $actual);
42+
$this->assertCount(6, $actual);
4143
}
4244
}

‎setup/src/Magento/Setup/Test/Unit/Module/Di/Code/Scanner/PhpScannerTest.php

Lines changed: 37 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -3,74 +3,74 @@
33
* Copyright © Magento, Inc. All rights reserved.
44
* See COPYING.txt for license details.
55
*/
6+
declare(strict_types=1);
7+
68
namespace Magento\Setup\Test\Unit\Module\Di\Code\Scanner;
79

810
require_once __DIR__ . '/../../_files/app/code/Magento/SomeModule/Helper/Test.php';
911
require_once __DIR__ . '/../../_files/app/code/Magento/SomeModule/ElementFactory.php';
1012
require_once __DIR__ . '/../../_files/app/code/Magento/SomeModule/Model/DoubleColon.php';
1113
require_once __DIR__ . '/../../_files/app/code/Magento/SomeModule/Api/Data/SomeInterface.php';
14+
require_once __DIR__ . '/../../_files/app/code/Magento/SomeModule/Model/StubWithAnonymousClass.php';
1215

1316
use Magento\Framework\Reflection\TypeProcessor;
17+
use Magento\Setup\Module\Di\Code\Scanner\PhpScanner;
18+
use Magento\Setup\Module\Di\Compiler\Log\Log;
19+
use PHPUnit\Framework\MockObject\MockObject;
1420

1521
class PhpScannerTest extends \PHPUnit\Framework\TestCase
1622
{
1723
/**
18-
* @var \Magento\Setup\Module\Di\Code\Scanner\PhpScanner
24+
* @var PhpScanner
1925
*/
20-
protected $_model;
26+
private $scanner;
2127

2228
/**
2329
* @var string
2430
*/
25-
protected $_testDir;
26-
27-
/**
28-
* @var array
29-
*/
30-
protected $_testFiles = [];
31+
private $testDir;
3132

3233
/**
33-
* @var \PHPUnit_Framework_MockObject_MockObject
34+
* @var Log|MockObject
3435
*/
35-
protected $_logMock;
36+
private $log;
3637

3738
protected function setUp()
3839
{
39-
$this->_logMock = $this->createMock(\Magento\Setup\Module\Di\Compiler\Log\Log::class);
40-
$this->_model = new \Magento\Setup\Module\Di\Code\Scanner\PhpScanner($this->_logMock, new TypeProcessor());
41-
$this->_testDir = str_replace('\\', '/', realpath(__DIR__ . '/../../') . '/_files');
40+
$this->log = $this->createMock(Log::class);
41+
$this->scanner = new PhpScanner($this->log, new TypeProcessor());
42+
$this->testDir = str_replace('\\', '/', realpath(__DIR__ . '/../../') . '/_files');
4243
}
4344

4445
public function testCollectEntities()
4546
{
46-
$this->_testFiles = [
47-
$this->_testDir . '/app/code/Magento/SomeModule/Helper/Test.php',
48-
$this->_testDir . '/app/code/Magento/SomeModule/Model/DoubleColon.php',
49-
$this->_testDir . '/app/code/Magento/SomeModule/Api/Data/SomeInterface.php'
47+
$testFiles = [
48+
$this->testDir . '/app/code/Magento/SomeModule/Helper/Test.php',
49+
$this->testDir . '/app/code/Magento/SomeModule/Model/DoubleColon.php',
50+
$this->testDir . '/app/code/Magento/SomeModule/Api/Data/SomeInterface.php',
51+
$this->testDir . '/app/code/Magento/SomeModule/Model/StubWithAnonymousClass.php',
5052
];
5153

52-
$this->_logMock->expects(
53-
$this->at(0)
54-
)->method(
55-
'add'
56-
)->with(
57-
4,
58-
'Magento\SomeModule\Module\Factory',
59-
'Invalid Factory for nonexistent class Magento\SomeModule\Module in file ' . $this->_testFiles[0]
60-
);
61-
$this->_logMock->expects(
62-
$this->at(1)
63-
)->method(
64-
'add'
65-
)->with(
66-
4,
67-
'Magento\SomeModule\Element\Factory',
68-
'Invalid Factory declaration for class Magento\SomeModule\Element in file ' . $this->_testFiles[0]
69-
);
54+
$this->log->expects(self::at(0))
55+
->method('add')
56+
->with(
57+
4,
58+
'Magento\SomeModule\Module\Factory',
59+
'Invalid Factory for nonexistent class Magento\SomeModule\Module in file ' . $testFiles[0]
60+
);
61+
$this->log->expects(self::at(1))
62+
->method('add')
63+
->with(
64+
4,
65+
'Magento\SomeModule\Element\Factory',
66+
'Invalid Factory declaration for class Magento\SomeModule\Element in file ' . $testFiles[0]
67+
);
68+
69+
$result = $this->scanner->collectEntities($testFiles);
7070

71-
$this->assertEquals(
71+
self::assertEquals(
7272
['\\' . \Magento\Eav\Api\Data\AttributeExtensionInterface::class],
73-
$this->_model->collectEntities($this->_testFiles)
73+
$result
7474
);
7575
}
7676
}

‎setup/src/Magento/Setup/Test/Unit/Module/Di/_files/app/code/Magento/SomeModule/Model/StubWithAnonymousClass.php

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
<?php
2+
/**
3+
* Copyright © Magento, Inc. All rights reserved.
4+
* See COPYING.txt for license details.
5+
*/
6+
declare(strict_types=1);
7+
8+
namespace Magento\SomeModule\Model;
9+
10+
use Magento\SomeModule\DummyFactory;
11+
12+
class StubWithAnonymousClass
13+
{
14+
/**
15+
* @var DummyFactory
16+
*/
17+
private $factory;
18+
19+
public function __construct(DummyFactory $factory)
20+
{
21+
$this->factory = $factory;
22+
}
23+
24+
public function getSerializable(): \JsonSerializable
25+
{
26+
return new class() implements \JsonSerializable {
27+
/**
28+
* @inheritDoc
29+
*/
30+
public function jsonSerialize()
31+
{
32+
return [1, 2, 3];
33+
}
34+
};
35+
}
36+
}

0 commit comments

Comments
 (0)