Skip to content

Commit b51317b

Browse files
Den4ikDen4ik
committed
Refactoring + added unit tests
Signed-off-by: Denis Kopylov <[email protected]>
1 parent da17d7e commit b51317b

File tree

2 files changed

+190
-24
lines changed

2 files changed

+190
-24
lines changed

‎dev/tests/static/framework/Magento/CodeMessDetector/Rule/UnusedCode/UnusedFormalParameter.php

Lines changed: 8 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,12 @@
11
<?php
2-
declare(strict_types=1);
32
/**
43
* Copyright © Magento, Inc. All rights reserved.
54
* See COPYING.txt for license details.
65
*/
6+
declare(strict_types=1);
77

88
namespace Magento\CodeMessDetector\Rule\UnusedCode;
99

10-
use PDepend\Source\AST\ASTParameter;
1110
use PHPMD\AbstractNode;
1211
use PHPMD\Node\ClassNode;
1312
use PHPMD\Node\MethodNode;
@@ -52,7 +51,7 @@ private function removeVariablesUsedInPlugins(AbstractNode $node)
5251
*/
5352
foreach (['around', 'after'] as $pluginMethodPrefix) {
5453
if ($this->isFunctionNameStartingWith($node, $pluginMethodPrefix)) {
55-
$this->removeVariablesByCount($node, 2);
54+
$this->removeVariablesByCount(2);
5655

5756
break;
5857
}
@@ -63,7 +62,7 @@ private function removeVariablesUsedInPlugins(AbstractNode $node)
6362
* that should be ignored
6463
*/
6564
if ($this->isFunctionNameStartingWith($node, 'before')) {
66-
$this->removeVariablesByCount($node, 1);
65+
$this->removeVariablesByCount(1);
6766
}
6867
}
6968

@@ -79,40 +78,25 @@ private function isFunctionNameStartingWith(MethodNode $node, string $name): boo
7978
return (0 === strpos($node->getImage(), $name));
8079
}
8180

82-
/**
83-
* Get first $numberOfParams parameters of method
84-
*
85-
* @param MethodNode $node
86-
* @param int $numberOfParams
87-
* @return array
88-
*/
89-
private function getMethodParametersByLength(MethodNode $node, int $numberOfParams): array
90-
{
91-
return array_slice($node->getNode()->getParameters(), 0, $numberOfParams);
92-
}
93-
9481
/**
9582
* Remove first $countOfRemovingVariables from given node
9683
*
97-
* @param MethodNode $node
9884
* @param int $countOfRemovingVariables
9985
*/
100-
private function removeVariablesByCount(MethodNode $node, int $countOfRemovingVariables)
86+
private function removeVariablesByCount(int $countOfRemovingVariables)
10187
{
102-
$methodParameters = $this->getMethodParametersByLength($node, $countOfRemovingVariables);
103-
/** @var ASTParameter $methodParameter */
104-
foreach ($methodParameters as $methodParameter) {
105-
unset($this->nodes[$methodParameter->getName()]);
106-
}
88+
array_splice($this->nodes, 0, $countOfRemovingVariables);
10789
}
10890

10991
/**
11092
* Check if namespace contain "Plugin"
93+
* Case sensitive ignored
94+
*
11195
* @param $class
11296
* @return bool
11397
*/
11498
private function isPluginClass($class): bool
11599
{
116-
return (stripos($class, 'Plugin') !== false);
100+
return (stripos($class, 'plugin') !== false);
117101
}
118102
}

‎dev/tests/static/framework/Magento/CodeMessDetector/Test/Unit/Rule/UnusedCode/UnusedFormalParameterTest.php

Lines changed: 182 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,182 @@
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\CodeMessDetector\Test\Unit\Rule\UnusedCode;
9+
10+
use Magento\CodeMessDetector\Rule\UnusedCode\UnusedFormalParameter;
11+
use PHPMD\Node\ASTNode;
12+
use PHPMD\Node\MethodNode;
13+
use PHPMD\Report;
14+
use PHPUnit\Framework\MockObject\MockObject as MockObject;
15+
use PHPUnit\Framework\TestCase;
16+
17+
/**
18+
* @SuppressWarnings(PHPMD.UnusedFormalParameter)
19+
*/
20+
class UnusedFormalParameterTest extends TestCase
21+
{
22+
private const FAKE_PLUGIN_NAMESPACE = 'Magento\CodeMessDetector\Test\UnusedCode\Plugin';
23+
private const FAKE_NAMESPACE = 'Magento\CodeMessDetector\Test\UnusedCode';
24+
25+
/**
26+
*
27+
* @dataProvider getCases
28+
*/
29+
public function testApply($methodName, $methodParams, $namespace, $expectViolation)
30+
{
31+
$node = $this->createMethodNodeMock($methodName, $methodParams, $namespace);
32+
$rule = new UnusedFormalParameter();
33+
$this->expectsRuleViolation($rule, $expectViolation);
34+
$rule->apply($node);
35+
}
36+
37+
/**
38+
* Prepare method node mock
39+
*
40+
* @param $methodName
41+
* @param $methodParams
42+
* @param $namespace
43+
* @return MethodNode|MockObject
44+
*/
45+
private function createMethodNodeMock($methodName, $methodParams, $namespace)
46+
{
47+
$methodNode = $this->createConfiguredMock(
48+
MethodNode::class,
49+
[
50+
'getName' => $methodName,
51+
'getImage' => $methodName,
52+
'isAbstract' => false,
53+
'isDeclaration' => true
54+
]
55+
);
56+
57+
/*$methodNode->expects($this->once())
58+
->method('getDocComment')
59+
->willReturn('');*/
60+
61+
$variableDeclarators = [];
62+
foreach ($methodParams as $methodParam) {
63+
$variableDeclarator = $this->createASTNodeMock();
64+
$variableDeclarator->method('getImage')
65+
->willReturn($methodParam);
66+
67+
$variableDeclarators[] = $variableDeclarator;
68+
}
69+
$parametersMock = $this->createASTNodeMock();
70+
$parametersMock->expects($this->once())
71+
->method('findChildrenOfType')
72+
->with('VariableDeclarator')
73+
->willReturn($variableDeclarators);
74+
75+
/**
76+
* Declare mock result for findChildrenOfType
77+
* with Dummy for removeCompoundVariables and removeVariablesUsedByFuncGetArgs
78+
*/
79+
$methodNode->expects($this->atLeastOnce())
80+
->method('findChildrenOfType')
81+
->withConsecutive(['FormalParameters'], ['CompoundVariable'], ['FunctionPostfix'])
82+
->willReturnOnConsecutiveCalls([$parametersMock], [], []);
83+
84+
// Dummy result for removeRegularVariables
85+
$methodNode->expects($this->once())
86+
->method('findChildrenOfTypeVariable')
87+
->willReturn([]);
88+
89+
$classNode = $this->createASTNodeMock();
90+
$classNode->expects($this->once())
91+
->method('getNamespaceName')
92+
->willReturn($namespace);
93+
$methodNode->expects($this->once())
94+
->method('getParentType')
95+
->willReturn($classNode);
96+
97+
return $methodNode;
98+
}
99+
100+
/**
101+
* Create ASTNode mock
102+
*
103+
* @return ASTNode|MockObject
104+
*/
105+
private function createASTNodeMock()
106+
{
107+
return $this->createMock(ASTNode::class);
108+
}
109+
110+
/**
111+
* @param UnusedFormalParameter $rule
112+
* @param bool $expects
113+
*/
114+
private function expectsRuleViolation(UnusedFormalParameter $rule, bool $expects)
115+
{
116+
/** @var Report|MockObject $reportMock */
117+
$reportMock = $this->createMock(Report::class);
118+
if ($expects) {
119+
$violationExpectation = $this->atLeastOnce();
120+
} else {
121+
$violationExpectation = $this->never();
122+
}
123+
$reportMock->expects($violationExpectation)
124+
->method('addRuleViolation');
125+
$rule->setReport($reportMock);
126+
}
127+
128+
/**
129+
* @return array
130+
*/
131+
public function getCases(): array
132+
{
133+
return [
134+
// Plugin methods
135+
[
136+
'beforePluginMethod',
137+
[
138+
'subject'
139+
],
140+
self::FAKE_PLUGIN_NAMESPACE,
141+
false
142+
],
143+
[
144+
'aroundPluginMethod',
145+
[
146+
'subject',
147+
'proceed'
148+
],
149+
self::FAKE_PLUGIN_NAMESPACE,
150+
false
151+
],
152+
[
153+
'aroundPluginMethod',
154+
[
155+
'subject',
156+
'result'
157+
],
158+
self::FAKE_PLUGIN_NAMESPACE,
159+
false
160+
],
161+
// Plugin method that contain unused parameter
162+
[
163+
'someMethod',
164+
[
165+
'unusedParameter'
166+
],
167+
self::FAKE_PLUGIN_NAMESPACE,
168+
true
169+
],
170+
// Non plugin method
171+
[
172+
'someMethod',
173+
[
174+
'subject',
175+
'result'
176+
],
177+
self::FAKE_NAMESPACE,
178+
true
179+
]
180+
];
181+
}
182+
}

0 commit comments

Comments
 (0)