Skip to content

Commit 8e4a639

Browse files
jcuerdojcuerdo
authored
Merge pull request #71 from magento-commerce/imported-jcuerdo-magento-coding-standard-273
[Imported] AC-662: Create phpcs static check for AutogeneratedClassNotInConstructorTest
2 parents c5dbf03 + 65eda98 commit 8e4a639

5 files changed

+327
-1
lines changed

‎Magento2/Sniffs/PHP/AutogeneratedClassNotInConstructorSniff.php

+226
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,226 @@
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 Magento2\Sniffs\PHP;
9+
10+
use PHP_CodeSniffer\Files\File;
11+
use PHP_CodeSniffer\Sniffs\Sniff;
12+
13+
/**
14+
* Detects bad usages of ObjectManager in constructor.
15+
*/
16+
class AutogeneratedClassNotInConstructorSniff implements Sniff
17+
{
18+
private const ERROR_CODE = 'AUTOGENERATED_CLASS_NOT_IN_CONSTRUCTOR';
19+
20+
/**
21+
* @var array
22+
*/
23+
private $constructorParameters = [];
24+
25+
/**
26+
* @var array
27+
*/
28+
private $uses = [];
29+
30+
/**
31+
* @inheritdoc
32+
*/
33+
public function register()
34+
{
35+
return [T_FUNCTION, T_DOUBLE_COLON, T_USE];
36+
}
37+
38+
/**
39+
* @inheritdoc
40+
*/
41+
public function process(File $phpcsFile, $stackPtr)
42+
{
43+
if ($phpcsFile->getTokens()[$stackPtr]['type'] === 'T_USE') {
44+
$this->registerUse($phpcsFile, $stackPtr);
45+
}
46+
if ($phpcsFile->getTokens()[$stackPtr]['type'] === 'T_FUNCTION') {
47+
$this->registerConstructorParameters($phpcsFile, $stackPtr);
48+
}
49+
if ($phpcsFile->getTokens()[$stackPtr]['type'] === 'T_DOUBLE_COLON') {
50+
if (!$this->isObjectManagerGetInstance($phpcsFile, $stackPtr)) {
51+
return;
52+
}
53+
54+
$statementStart = $phpcsFile->findStartOfStatement($stackPtr);
55+
$statementEnd = $phpcsFile->findEndOfStatement($stackPtr);
56+
$equalsPtr = $phpcsFile->findNext(T_EQUAL, $statementStart, $statementEnd);
57+
58+
if (!$equalsPtr) {
59+
return;
60+
}
61+
62+
if (!$this->isVariableInConstructorParameters($phpcsFile, $equalsPtr, $statementEnd)) {
63+
$className = $this->obtainClassToGetOrCreate($phpcsFile, $stackPtr, $statementEnd);
64+
65+
$phpcsFile->addError(
66+
sprintf("Class %s needs to be requested in constructor, " .
67+
"otherwise compiler will not be able to find and generate these classes", $className),
68+
$stackPtr,
69+
self::ERROR_CODE
70+
);
71+
}
72+
}
73+
}
74+
75+
/**
76+
* Check if it is a ObjectManager::getInstance
77+
*
78+
* @param File $phpcsFile
79+
* @param int $stackPtr
80+
* @return bool
81+
*/
82+
private function isObjectManagerGetInstance(File $phpcsFile, int $stackPtr): bool
83+
{
84+
$prev = $phpcsFile->findPrevious(T_STRING, $stackPtr - 1);
85+
$next = $phpcsFile->findNext(T_STRING, $stackPtr + 1);
86+
return $prev &&
87+
$next &&
88+
$phpcsFile->getTokens()[$prev]['content'] === 'ObjectManager' &&
89+
$phpcsFile->getTokens()[$next]['content'] === 'getInstance';
90+
}
91+
92+
/**
93+
* Get the complete class namespace from the use's
94+
*
95+
* @param string $className
96+
* @return string
97+
*/
98+
private function getClassNamespace(string $className): string
99+
{
100+
foreach ($this->uses as $key => $use) {
101+
if ($key === $className) {
102+
return $use;
103+
}
104+
}
105+
return $className;
106+
}
107+
108+
/**
109+
* Register php uses
110+
*
111+
* @param File $phpcsFile
112+
* @param int $stackPtr
113+
*/
114+
private function registerUse(File $phpcsFile, int $stackPtr): void
115+
{
116+
$useEnd = $phpcsFile->findEndOfStatement($stackPtr);
117+
$use = [];
118+
$usePosition = $stackPtr;
119+
while ($usePosition = $phpcsFile->findNext(T_STRING, $usePosition + 1, $useEnd)) {
120+
$use[] = $phpcsFile->getTokens()[$usePosition]['content'];
121+
}
122+
123+
$key = end($use);
124+
if ($phpcsFile->findNext(T_AS, $stackPtr, $useEnd)) {
125+
$this->uses[$key] = implode("\\", array_slice($use, 0, count($use) - 1));
126+
} else {
127+
$this->uses[$key] = implode("\\", $use);
128+
}
129+
}
130+
131+
/**
132+
* Register php constructor parameters
133+
*
134+
* @param File $phpcsFile
135+
* @param int $stackPtr
136+
*/
137+
private function registerConstructorParameters(File $phpcsFile, int $stackPtr): void
138+
{
139+
$functionName = $phpcsFile->getDeclarationName($stackPtr);
140+
if ($functionName == '__construct') {
141+
$this->constructorParameters = $phpcsFile->getMethodParameters($stackPtr);
142+
}
143+
}
144+
145+
/**
146+
* Get next token
147+
*
148+
* @param File $phpcsFile
149+
* @param int $from
150+
* @param int $to
151+
* @param int|string|array $types
152+
* @return mixed
153+
*/
154+
private function getNext(File $phpcsFile, int $from, int $to, $types)
155+
{
156+
return $phpcsFile->getTokens()[$phpcsFile->findNext($types, $from + 1, $to)];
157+
}
158+
159+
/**
160+
* Get previous token
161+
*
162+
* @param File $phpcsFile
163+
* @param int $from
164+
* @param int|string|array $types
165+
* @return mixed
166+
*/
167+
private function getPrevious(File $phpcsFile, int $from, $types)
168+
{
169+
return $phpcsFile->getTokens()[$phpcsFile->findPrevious($types, $from - 1)];
170+
}
171+
172+
/**
173+
* Get name of the variable without $
174+
*
175+
* @param string $parameterName
176+
* @return string
177+
*/
178+
protected function variableName(string $parameterName): string
179+
{
180+
return str_replace('$', '', $parameterName);
181+
}
182+
183+
/**
184+
* Checks if a variable is present in the constructor parameters
185+
*
186+
* @param File $phpcsFile
187+
* @param int $equalsPtr
188+
* @param int $statementEnd
189+
* @return bool
190+
*/
191+
private function isVariableInConstructorParameters(File $phpcsFile, int $equalsPtr, int $statementEnd): bool
192+
{
193+
if ($variable = $phpcsFile->findNext(T_VARIABLE, $equalsPtr, $statementEnd)) {
194+
$variableName = $phpcsFile->getTokens()[$variable]['content'];
195+
if ($variableName === '$this') {
196+
$variableName = $this->getNext($phpcsFile, $variable, $statementEnd, T_STRING)['content'];
197+
}
198+
foreach ($this->constructorParameters as $parameter) {
199+
$parameterName = $parameter['name'];
200+
if ($this->variableName($parameterName) === $this->variableName($variableName)) {
201+
return true;
202+
}
203+
}
204+
}
205+
return false;
206+
}
207+
208+
/**
209+
* Obtain the class inside ObjectManager::getInstance()->get|create()
210+
*
211+
* @param File $phpcsFile
212+
* @param int $next
213+
* @param int $statementEnd
214+
* @return string
215+
*/
216+
private function obtainClassToGetOrCreate(File $phpcsFile, int $next, int $statementEnd): string
217+
{
218+
while ($next = $phpcsFile->findNext(T_DOUBLE_COLON, $next + 1, $statementEnd)) {
219+
if ($this->getNext($phpcsFile, $next, $statementEnd, T_STRING)['content'] === 'class') {
220+
$className = $this->getPrevious($phpcsFile, $next, T_STRING)['content'];
221+
}
222+
}
223+
224+
return $this->getClassNamespace($className);
225+
}
226+
}

‎Magento2/Tests/PHP/AutogeneratedClassNotInConstructorUnitTest.1.php.inc

+38
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
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 Magento2\Tests\PHP;
9+
10+
use Magento2\OneModel as Model;
11+
12+
class Good
13+
{
14+
public function __construct(
15+
Model $model = null
16+
)
17+
{
18+
$this->model = $model ?? ObjectManager::getInstance()->get(Model::class);
19+
}
20+
21+
/**
22+
* @return Model
23+
*/
24+
public function otherMethodThatCallsGetInstanceBad(): void
25+
{
26+
$model = ObjectManager::getInstance()->get(Model::class);
27+
$model->something();
28+
}
29+
30+
/**
31+
* @return Model
32+
*/
33+
public function otherMethodThatCallsGetInstanceGood(): void
34+
{
35+
$model = $this->model ?? ObjectManager::getInstance()->get(Model::class);
36+
$model->something();
37+
}
38+
}

‎Magento2/Tests/PHP/AutogeneratedClassNotInConstructorUnitTest.2.php.inc

+17
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
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 Magento2\Tests\PHP;
9+
10+
class Bad
11+
{
12+
public function __construct()
13+
{
14+
$this->model = ObjectManager::getInstance()->get(Model::class);
15+
}
16+
}
17+

‎Magento2/Tests/PHP/AutogeneratedClassNotInConstructorUnitTest.php

+39
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
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 Magento2\Tests\PHP;
9+
10+
use PHP_CodeSniffer\Tests\Standards\AbstractSniffUnitTest;
11+
12+
class AutogeneratedClassNotInConstructorUnitTest extends AbstractSniffUnitTest
13+
{
14+
/**
15+
* @inheritdoc
16+
*/
17+
public function getErrorList($filename = '')
18+
{
19+
if ($filename === 'AutogeneratedClassNotInConstructorUnitTest.1.php.inc') {
20+
return [
21+
26 => 1,
22+
];
23+
}
24+
if ($filename === 'AutogeneratedClassNotInConstructorUnitTest.2.php.inc') {
25+
return [
26+
14 => 1,
27+
];
28+
}
29+
return [];
30+
}
31+
32+
/**
33+
* @inheritdoc
34+
*/
35+
public function getWarningList($filename = '')
36+
{
37+
return [];
38+
}
39+
}

‎Magento2/ruleset.xml

+7-1
Original file line numberDiff line numberDiff line change
@@ -130,6 +130,11 @@
130130
<severity>10</severity>
131131
<type>error</type>
132132
</rule>
133+
<rule ref="Magento2.PHP.AutogeneratedClassNotInConstructor">
134+
<include-pattern>*\.php$</include-pattern>
135+
<severity>10</severity>
136+
<type>error</type>
137+
</rule>
133138

134139
<rule ref="Magento2.Html.HtmlSelfClosingTags">
135140
<severity>10</severity>
@@ -172,6 +177,7 @@
172177
<exclude-pattern>*\.xml$</exclude-pattern>
173178
</rule>
174179
<rule ref="Magento2.Html.HtmlBinding">
180+
<include-pattern>*\/.phtml$</include-pattern>
175181
<severity>9</severity>
176182
<type>warning</type>
177183
<exclude-pattern>*\.xml$</exclude-pattern>
@@ -271,7 +277,7 @@
271277
<type>warning</type>
272278
<exclude-pattern>*\.xml$</exclude-pattern>
273279
</rule>
274-
<rule ref="Magento2.Legacy.WidgetXML.FoundObsoleteNode">
280+
<rule ref="Magento2.Legacy.WidgetXML">
275281
<include-pattern>*\/widget.xml$</include-pattern>
276282
<severity>8</severity>
277283
<type>warning</type>

0 commit comments

Comments
 (0)