Skip to content

Commit 7cc7adf

Browse files
faizan-shkfaizan-shk
committed
Merge branch 'AC-2841' of github.com:magento-gl/magento2ce into Hammer_PlatForm_Health_246_Scope_14102022
2 parents 9b7f7a0 + aabe917 commit 7cc7adf

File tree

7 files changed

+139
-35
lines changed

7 files changed

+139
-35
lines changed

‎lib/internal/Magento/Framework/Code/Reader/ArgumentsReader.php‎

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -181,12 +181,11 @@ public function getParentCall(\ReflectionClass $class, array $classArguments): ?
181181
return null;
182182
}
183183

184-
$isNamedArgument = false;
185184
$arguments = substr(trim($arguments), 20, -2);
186185
$arguments = explode(',', $arguments);
187-
$position = strpos(current($arguments), ':');
188-
if ($position !== false) {
189-
$isNamedArgument = true;
186+
$isNamedArgument = [];
187+
foreach ($arguments as $argumentPosition => $argumentName) {
188+
$isNamedArgument[$argumentPosition] = (bool)strpos($argumentName, ':');
190189
}
191190
array_walk($arguments, $trimFunction);
192191

@@ -197,11 +196,8 @@ public function getParentCall(\ReflectionClass $class, array $classArguments): ?
197196
'name' => $argumentName,
198197
'position' => $argumentPosition,
199198
'type' => $type,
199+
'isNamedArgument' => $isNamedArgument[$argumentPosition],
200200
];
201-
202-
if ($isNamedArgument) {
203-
$output[$argumentPosition]['isNamedArgument'] = true;
204-
}
205201
}
206202

207203
return $output;

‎lib/internal/Magento/Framework/Code/Test/Unit/Reader/ArgumentsReaderTest.php‎

Lines changed: 62 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -270,8 +270,18 @@ public function testGetParentCallWithRightArgumentsOrder()
270270
]
271271
);
272272
$expectedResult = [
273-
['name' => 'stdClassObject', 'position' => 0, 'type' => '\stdClass'],
274-
['name' => 'secondClass', 'position' => 1, 'type' => '\ClassExtendsDefaultPhpType'],
273+
[
274+
'name' => 'stdClassObject',
275+
'position' => 0,
276+
'type' => '\stdClass',
277+
'isNamedArgument' => false
278+
],
279+
[
280+
'name' => 'secondClass',
281+
'position' => 1,
282+
'type' => '\ClassExtendsDefaultPhpType',
283+
'isNamedArgument' => false
284+
],
275285
];
276286
$this->assertEquals($expectedResult, $actualResult);
277287
}
@@ -287,8 +297,17 @@ public function testGetParentCallWithWrongArgumentsOrder()
287297
]
288298
);
289299
$expectedResult = [
290-
['name' => 'secondClass', 'position' => 0, 'type' => '\ClassExtendsDefaultPhpType'],
291-
['name' => 'stdClassObject', 'position' => 1, 'type' => '\stdClass'],
300+
[
301+
'name' => 'secondClass',
302+
'position' => 0,
303+
'type' => '\ClassExtendsDefaultPhpType',
304+
'isNamedArgument' => false],
305+
[
306+
'name' => 'stdClassObject',
307+
'position' => 1,
308+
'type' => '\stdClass',
309+
'isNamedArgument' => false
310+
],
292311
];
293312
$this->assertEquals($expectedResult, $actualResult);
294313
}
@@ -304,8 +323,18 @@ public function testGetParentCallWithSeparateLineFormat()
304323
]
305324
);
306325
$expectedResult = [
307-
['name' => 'stdClassObject', 'position' => 0, 'type' => '\stdClass'],
308-
['name' => 'secondClass', 'position' => 1, 'type' => '\ClassExtendsDefaultPhpType'],
326+
[
327+
'name' => 'stdClassObject',
328+
'position' => 0,
329+
'type' => '\stdClass',
330+
'isNamedArgument' => false
331+
],
332+
[
333+
'name' => 'secondClass',
334+
'position' => 1,
335+
'type' => '\ClassExtendsDefaultPhpType',
336+
'isNamedArgument' => false
337+
],
309338
];
310339
$this->assertEquals($expectedResult, $actualResult);
311340
}
@@ -337,6 +366,33 @@ public function testGetParentCallWithNamedArguments()
337366
$this->assertEquals($expectedResult, $actualResult);
338367
}
339368

369+
public function testGetParentCallWithMixedArguments()
370+
{
371+
$class = new \ReflectionClass('ClassWithMixedArgumentsForParentCall');
372+
$actualResult = $this->_model->getParentCall(
373+
$class,
374+
[
375+
'stdClassObject' => ['type' => '\stdClass'],
376+
'runeTimeException' => ['type' => '\ClassExtendsDefaultPhpType']
377+
]
378+
);
379+
$expectedResult = [
380+
[
381+
'name' => 'stdClassObject',
382+
'position' => 0,
383+
'type' => '\stdClass',
384+
'isNamedArgument' => false
385+
],
386+
[
387+
'name' => 'runeTimeException',
388+
'position' => 1,
389+
'type' => '\ClassExtendsDefaultPhpType',
390+
'isNamedArgument' => true
391+
],
392+
];
393+
$this->assertEquals($expectedResult, $actualResult);
394+
}
395+
340396
/**
341397
* @param string $requiredType
342398
* @param string $actualType

‎lib/internal/Magento/Framework/Code/Test/Unit/Reader/_files/ClassesForArgumentsReader.php‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -280,3 +280,27 @@ public function __construct(\stdClass $stdClassObject, \ClassExtendsDefaultPhpTy
280280
$this->_runeTimeException = $runeTimeException;
281281
}
282282
}
283+
284+
class ClassWithMixedArgumentsForParentCall extends FirstClassForParentCall
285+
{
286+
/**
287+
* @var stdClass
288+
*/
289+
protected $_stdClassObject;
290+
291+
/**
292+
* @var ClassExtendsDefaultPhpType
293+
*/
294+
protected $_runeTimeException;
295+
296+
/**
297+
* @param stdClass $stdClassObject
298+
* @param ClassExtendsDefaultPhpType $runeTimeException
299+
*/
300+
public function __construct(\stdClass $stdClassObject, \ClassExtendsDefaultPhpType $runeTimeException)
301+
{
302+
parent::__construct($stdClassObject, runeTimeException: $runeTimeException);
303+
$this->_stdClassObject = $stdClassObject;
304+
$this->_runeTimeException = $runeTimeException;
305+
}
306+
}

‎lib/internal/Magento/Framework/Code/Test/Unit/Validator/ConstructorIntegrityTest.php‎

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,8 @@
55
*/
66
namespace Magento\Framework\Code\Test\Unit\Validator;
77

8-
use Magento\SomeModule\Model\NamedArguments\ChildClassTest;
8+
use Magento\SomeModule\Model\NamedArguments\NamedParametersTest;
9+
use Magento\SomeModule\Model\NamedArguments\MixedParametersTest;
910
use PHPUnit\Framework\TestCase;
1011
use Magento\Framework\Code\Validator\ConstructorIntegrity;
1112
use Magento\Framework\Exception\ValidatorException;
@@ -16,7 +17,8 @@
1617
require_once __DIR__ . '/../_files/app/code/Magento/SomeModule/Model/Four/Test.php';
1718
require_once __DIR__ . '/../_files/app/code/Magento/SomeModule/Model/Five/Test.php';
1819
require_once __DIR__ . '/../_files/app/code/Magento/SomeModule/Model/Six/Test.php';
19-
require_once __DIR__ . '/../_files/app/code/Magento/SomeModule/Model/NamedArguments/ChildClassTest.php';
20+
require_once __DIR__ . '/../_files/app/code/Magento/SomeModule/Model/NamedArguments/NamedParametersTest.php';
21+
require_once __DIR__ . '/../_files/app/code/Magento/SomeModule/Model/NamedArguments/MixedParametersTest.php';
2022
require_once __DIR__ . '/_files/ClassesForConstructorIntegrity.php';
2123
class ConstructorIntegrityTest extends TestCase
2224
{
@@ -42,7 +44,12 @@ public function testValidateIfClassHasParentConstructCall()
4244

4345
public function testValidateIfClassHasParentConstructCallWithNamedArguments()
4446
{
45-
$this->assertTrue($this->_model->validate(ChildClassTest::class));
47+
$this->assertTrue($this->_model->validate(NamedParametersTest::class));
48+
}
49+
50+
public function testValidateIfClassHasParentConstructCallWithMixedArguments()
51+
{
52+
$this->assertTrue($this->_model->validate(MixedParametersTest::class));
4653
}
4754

4855
public function testValidateIfClassHasArgumentsQtyEqualToParentClass()

‎lib/internal/Magento/Framework/Code/Test/Unit/_files/app/code/Magento/SomeModule/Model/NamedArguments/MixedParametersTest.php‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
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\NamedArguments;
9+
10+
require_once __DIR__ . '/ParentClassTest.php';
11+
12+
class MixedParametersTest extends ParentClassTest
13+
{
14+
/**
15+
* @param \stdClass $stdClassObject
16+
* @param array $arrayVariable
17+
*/
18+
public function __construct(\stdClass $stdClassObject, array $arrayVariable)
19+
{
20+
parent::__construct($stdClassObject, arrayVariable: $arrayVariable);
21+
}
22+
}

‎lib/internal/Magento/Framework/Code/Test/Unit/_files/app/code/Magento/SomeModule/Model/NamedArguments/ChildClassTest.php‎ renamed to ‎lib/internal/Magento/Framework/Code/Test/Unit/_files/app/code/Magento/SomeModule/Model/NamedArguments/NamedParametersTest.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99

1010
require_once __DIR__ . '/ParentClassTest.php';
1111

12-
class ChildClassTest extends ParentClassTest
12+
class NamedParametersTest extends ParentClassTest
1313
{
1414
/**
1515
* @param \stdClass $stdClassObject

‎lib/internal/Magento/Framework/Code/Validator/ConstructorIntegrity.php‎

Lines changed: 16 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -69,26 +69,25 @@ public function validate($className)
6969
/** Get parent class __construct arguments */
7070
$parentArguments = $this->_argumentsReader->getConstructorArguments($parent, true, true);
7171

72-
if (isset(current($callArguments)['isNamedArgument'])) {
73-
$callArguments = array_column($callArguments, null, 'name');
74-
75-
foreach ($parentArguments as $requiredArgument) {
76-
if (isset($callArguments[$requiredArgument['name']])) {
77-
$actualArgument = $callArguments[$requiredArgument['name']];
72+
foreach ($parentArguments as $index => $requiredArgument) {
73+
$reIndexedCallArguments = array_column($callArguments, null, 'name');
74+
if (isset($reIndexedCallArguments[$requiredArgument['name']])) {
75+
if ($reIndexedCallArguments[$requiredArgument['name']]['isNamedArgument'] === true) {
76+
$actualArgument = $reIndexedCallArguments[$requiredArgument['name']];
7877
$this->checkCompatibleTypes($requiredArgument['type'], $actualArgument['type'], $class);
79-
} else {
80-
$this->checkIfRequiredArgumentIsOptional($requiredArgument, $class);
78+
continue;
8179
}
8280
}
83-
} else {
84-
// Need to separate logic for unnamed arguments as we cannot consider `argument name` for unnamed arguments
85-
foreach ($parentArguments as $index => $requiredArgument) {
86-
if (isset($callArguments[$index])) {
87-
$actualArgument = $callArguments[$index];
88-
$this->checkCompatibleTypes($requiredArgument['type'], $actualArgument['type'], $class);
89-
} else {
90-
$this->checkIfRequiredArgumentIsOptional($requiredArgument, $class);
91-
}
81+
82+
if (isset($callArguments[$index]) && $callArguments[$index]['isNamedArgument'] === true) {
83+
$this->checkIfRequiredArgumentIsOptional($requiredArgument, $class);
84+
}
85+
86+
if (isset($callArguments[$index])) {
87+
$actualArgument = $callArguments[$index];
88+
$this->checkCompatibleTypes($requiredArgument['type'], $actualArgument['type'], $class);
89+
} else {
90+
$this->checkIfRequiredArgumentIsOptional($requiredArgument, $class);
9291
}
9392
}
9493

0 commit comments

Comments
 (0)