Skip to content

Commit fd3f2a2

Browse files
YevSentYevSent
committed
Github #26532: di:setup:compile fails with anonymous classes
 - Fixed parsing for anonymous classes during di:compile run - Small code optimizations - Extended and refactored related unit tests
1 parent 29cb41d commit fd3f2a2

File tree

4 files changed

+128
-82
lines changed

4 files changed

+128
-82
lines changed

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

Lines changed: 47 additions & 39 deletions
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\Module\Di\Code\Scanner;
79

810
use Magento\Framework\Api\Code\Generator\ExtensionAttributesGenerator;
@@ -12,9 +14,7 @@
1214
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
{
@@ -53,15 +53,28 @@ public function __construct(Log $log, TypeProcessor $typeProcessor = null)
5353
protected function _findMissingClasses($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);
114126
$absentFactories = $this->_findMissingClasses(
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
/**
@@ -223,8 +222,17 @@ protected function _fetchClasses($namespace, $tokenIterator, $count, $tokens)
223222
{
224223
$classes = [];
225224
for ($tokenOffset = $tokenIterator + 1; $tokenOffset < $count; ++$tokenOffset) {
226-
if ($tokens[$tokenOffset] === '{') {
227-
$classes[] = $namespace . "\\" . $tokens[$tokenIterator + 2][1];
225+
if ($tokens[$tokenOffset] !== '{') {
226+
continue;
227+
}
228+
// anonymous classes should be omitted
229+
if (is_array($tokens[$tokenIterator - 2]) && $tokens[$tokenIterator - 2][0] === T_NEW) {
230+
continue;
231+
}
232+
233+
$class = $namespace . "\\" . $tokens[$tokenIterator + 2][1];
234+
if (!in_array($class, $classes)) {
235+
$classes[] = $class;
228236
}
229237
}
230238
return $classes;

‎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: 42 additions & 42 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-
namespace Magento\Setup\Test\Unit\Module\Di\Code\Scanner;
6+
declare(strict_types=1);
77

8-
require_once __DIR__ . '/../../_files/app/code/Magento/SomeModule/Helper/Test.php';
9-
require_once __DIR__ . '/../../_files/app/code/Magento/SomeModule/ElementFactory.php';
10-
require_once __DIR__ . '/../../_files/app/code/Magento/SomeModule/Model/DoubleColon.php';
11-
require_once __DIR__ . '/../../_files/app/code/Magento/SomeModule/Api/Data/SomeInterface.php';
8+
namespace Magento\Setup\Test\Unit\Module\Di\Code\Scanner;
129

1310
use Magento\Framework\Reflection\TypeProcessor;
11+
use Magento\Setup\Module\Di\Code\Scanner\PhpScanner;
12+
use Magento\Setup\Module\Di\Compiler\Log\Log;
13+
use PHPUnit\Framework\MockObject\MockObject;
1414

1515
class PhpScannerTest extends \PHPUnit\Framework\TestCase
1616
{
1717
/**
18-
* @var \Magento\Setup\Module\Di\Code\Scanner\PhpScanner
18+
* @var PhpScanner
1919
*/
20-
protected $_model;
20+
private $scanner;
2121

2222
/**
2323
* @var string
2424
*/
25-
protected $_testDir;
26-
27-
/**
28-
* @var array
29-
*/
30-
protected $_testFiles = [];
25+
private $testDir;
3126

3227
/**
33-
* @var \PHPUnit_Framework_MockObject_MockObject
28+
* @var Log|MockObject
3429
*/
35-
protected $_logMock;
30+
private $log;
3631

3732
protected function setUp()
3833
{
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');
34+
$this->log = $this->createMock(Log::class);
35+
$this->scanner = new PhpScanner($this->log, new TypeProcessor());
36+
$this->testDir = str_replace('\\', '/', realpath(__DIR__ . '/../../') . '/_files');
4237
}
4338

4439
public function testCollectEntities()
4540
{
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'
41+
require_once __DIR__ . '/../../_files/app/code/Magento/SomeModule/Helper/Test.php';
42+
require_once __DIR__ . '/../../_files/app/code/Magento/SomeModule/ElementFactory.php';
43+
require_once __DIR__ . '/../../_files/app/code/Magento/SomeModule/Model/DoubleColon.php';
44+
require_once __DIR__ . '/../../_files/app/code/Magento/SomeModule/Api/Data/SomeInterface.php';
45+
require_once __DIR__ . '/../../_files/app/code/Magento/SomeModule/Model/StubWithAnonymousClass.php';
46+
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)