From 80e07de67f87f84c6b57251afede86de908f21ab Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jorge=20Cuerdo=20=C3=81lvarez?= Date: Tue, 14 Sep 2021 16:14:26 +0200 Subject: [PATCH 1/7] AC-662: Create phpcs static check for AutogeneratedClassNotInConstructorTest --- ...utogeneratedClassNotInConstructorSniff.php | 210 ++++++++++++++++++ ...tedClassNotInConstructorUnitTest.1.php.inc | 28 +++ ...tedClassNotInConstructorUnitTest.2.php.inc | 16 ++ ...generatedClassNotInConstructorUnitTest.php | 33 +++ 4 files changed, 287 insertions(+) create mode 100644 Magento2/Sniffs/PHP/AutogeneratedClassNotInConstructorSniff.php create mode 100644 Magento2/Tests/PHP/AutogeneratedClassNotInConstructorUnitTest.1.php.inc create mode 100644 Magento2/Tests/PHP/AutogeneratedClassNotInConstructorUnitTest.2.php.inc create mode 100644 Magento2/Tests/PHP/AutogeneratedClassNotInConstructorUnitTest.php diff --git a/Magento2/Sniffs/PHP/AutogeneratedClassNotInConstructorSniff.php b/Magento2/Sniffs/PHP/AutogeneratedClassNotInConstructorSniff.php new file mode 100644 index 00000000..b7343896 --- /dev/null +++ b/Magento2/Sniffs/PHP/AutogeneratedClassNotInConstructorSniff.php @@ -0,0 +1,210 @@ +getTokens()[$stackPtr]['type'] === 'T_USE') { + $this->registerUses($phpcsFile, $stackPtr); + } + if ($phpcsFile->getTokens()[$stackPtr]['type'] === 'T_FUNCTION') { + $this->registerConstructorParameters($phpcsFile, $stackPtr); + } + if ($phpcsFile->getTokens()[$stackPtr]['type'] === 'T_DOUBLE_COLON') { + if (!$this->isInsideConstruct($stackPtr)) { + return; + } + if (!$this->isObjectManagerGetInstance($phpcsFile, $stackPtr)) { + return; + } + + $statementStart = $phpcsFile->findStartOfStatement($stackPtr); + $statementEnd = $phpcsFile->findEndOfStatement($stackPtr); + $equalsPtr = $phpcsFile->findNext(T_EQUAL, $statementStart, $statementEnd); + + if (!$equalsPtr) { + return; + } + + $variableInParameters = false; + if ($variable = $phpcsFile->findNext(T_VARIABLE, $equalsPtr, $statementEnd)) { + $variableName = $phpcsFile->getTokens()[$variable]['content']; + foreach ($this->constructorParameters as $parameter) { + $parameterName = $parameter['name']; + if ($parameterName === $variableName) { + $variableInParameters = true; + } + } + } + + if (!$variableInParameters) { + $next = $stackPtr; + while ($next = $phpcsFile->findNext(T_DOUBLE_COLON, $next + 1, $statementEnd)) { + if ($this->getNext($phpcsFile, $next, $statementEnd, T_STRING)['content'] === 'class') { + $className = $this->getPrevious($phpcsFile, $next, T_STRING)['content']; + } + } + + $className = $this->getClassNamespace($className); + + $phpcsFile->addError( + sprintf("Class %s needs to be requested in constructor, " . + "otherwise compiler will not be able to find and generate these classes", $className), + $stackPtr, + self::ERROR_CODE + ); + } + } + } + + /** + * Check if it is a ObjectManager::getInstance + * + * @param File $phpcsFile + * @param int $stackPtr + * @return bool + */ + private function isObjectManagerGetInstance(File $phpcsFile, int $stackPtr): bool + { + $prev = $phpcsFile->findPrevious(T_STRING, $stackPtr - 1); + $next = $phpcsFile->findNext(T_STRING, $stackPtr + 1); + if ($prev && + $next && + $phpcsFile->getTokens()[$prev]['content'] === 'ObjectManager' && + $phpcsFile->getTokens()[$next]['content'] === 'getInstance') { + return true; + } + return false; + } + + /** + * Checks if the code is inside __construct + * + * @param int $stackPtr + * @return bool + */ + private function isInsideConstruct(int $stackPtr): bool + { + return $stackPtr > $this->constructorScopeOpener && $stackPtr < $this->constructorScopeCloser; + } + + /** + * Get the complete class namespace from the use's + * + * @param string $className + * @return string + */ + private function getClassNamespace(string $className): string + { + foreach ($this->uses as $use) { + if (end($use) === $className) { + $className = implode('/', $use); + } + } + return $className; + } + + /** + * Register php uses + * + * @param File $phpcsFile + * @param int $stackPtr + */ + private function registerUses(File $phpcsFile, int $stackPtr): void + { + $useEnd = $phpcsFile->findEndOfStatement($stackPtr); + $use = []; + $usePosition = $stackPtr; + while ($usePosition = $phpcsFile->findNext(T_STRING, $usePosition + 1, $useEnd)) { + $use[] = $phpcsFile->getTokens()[$usePosition]['content']; + } + $this->uses [] = $use; + } + + /** + * Register php constructor parameters + * + * @param File $phpcsFile + * @param int $stackPtr + */ + private function registerConstructorParameters(File $phpcsFile, int $stackPtr): void + { + $functionName = $phpcsFile->getDeclarationName($stackPtr); + if ($functionName == '__construct') { + $this->constructorParameters = $phpcsFile->getMethodParameters($stackPtr); + $this->constructorScopeOpener = $phpcsFile->getTokens()[$stackPtr]['scope_opener']; + $this->constructorScopeCloser = $phpcsFile->getTokens()[$stackPtr]['scope_closer']; + + } + } + + /** + * Get next token + * + * @param File $phpcsFile + * @param int $nextDoubleColumn + * @param int $statementEnd + * @param int|string|array $types + * @return mixed + */ + private function getNext(File $phpcsFile, int $nextDoubleColumn, int $statementEnd, $types) + { + return $phpcsFile->getTokens()[$phpcsFile->findNext($types, $nextDoubleColumn + 1, $statementEnd)]; + } + + /** + * Get previous token + * + * @param File $phpcsFile + * @param int $nextDoubleColumn + * @param int|string|array $types + * @return mixed + */ + private function getPrevious(File $phpcsFile, int $nextDoubleColumn, $types) + { + return $phpcsFile->getTokens()[$phpcsFile->findPrevious($types, $nextDoubleColumn - 1)]; + } +} diff --git a/Magento2/Tests/PHP/AutogeneratedClassNotInConstructorUnitTest.1.php.inc b/Magento2/Tests/PHP/AutogeneratedClassNotInConstructorUnitTest.1.php.inc new file mode 100644 index 00000000..e490e6c0 --- /dev/null +++ b/Magento2/Tests/PHP/AutogeneratedClassNotInConstructorUnitTest.1.php.inc @@ -0,0 +1,28 @@ +model = $model ?? ObjectManager::getInstance()->get(Model::class); + } + + /** + * @return Model + */ + public function otherMethodThatCallsGetInstance(): void + { + $model = ObjectManager::getInstance()->get(Model::class); + $model->something(); + } +} diff --git a/Magento2/Tests/PHP/AutogeneratedClassNotInConstructorUnitTest.2.php.inc b/Magento2/Tests/PHP/AutogeneratedClassNotInConstructorUnitTest.2.php.inc new file mode 100644 index 00000000..5b6271ca --- /dev/null +++ b/Magento2/Tests/PHP/AutogeneratedClassNotInConstructorUnitTest.2.php.inc @@ -0,0 +1,16 @@ +model = ObjectManager::getInstance()->get(Model::class); + } +} + diff --git a/Magento2/Tests/PHP/AutogeneratedClassNotInConstructorUnitTest.php b/Magento2/Tests/PHP/AutogeneratedClassNotInConstructorUnitTest.php new file mode 100644 index 00000000..4ecab492 --- /dev/null +++ b/Magento2/Tests/PHP/AutogeneratedClassNotInConstructorUnitTest.php @@ -0,0 +1,33 @@ + 1 + ]; + } + return []; + } + + /** + * @inheritdoc + */ + public function getWarningList() + { + return []; + } +} From a6d2530c8eb5300ee7d79f2d0bc3f05ac6d7b902 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jorge=20Cuerdo=20=C3=81lvarez?= Date: Tue, 14 Sep 2021 16:20:45 +0200 Subject: [PATCH 2/7] AC-662: Create phpcs static check for AutogeneratedClassNotInConstructorTest --- .../Sniffs/PHP/AutogeneratedClassNotInConstructorSniff.php | 3 ++- .../PHP/AutogeneratedClassNotInConstructorUnitTest.1.php.inc | 3 ++- .../PHP/AutogeneratedClassNotInConstructorUnitTest.2.php.inc | 3 ++- .../Tests/PHP/AutogeneratedClassNotInConstructorUnitTest.php | 5 +++-- 4 files changed, 9 insertions(+), 5 deletions(-) diff --git a/Magento2/Sniffs/PHP/AutogeneratedClassNotInConstructorSniff.php b/Magento2/Sniffs/PHP/AutogeneratedClassNotInConstructorSniff.php index b7343896..91fdc265 100644 --- a/Magento2/Sniffs/PHP/AutogeneratedClassNotInConstructorSniff.php +++ b/Magento2/Sniffs/PHP/AutogeneratedClassNotInConstructorSniff.php @@ -1,8 +1,9 @@ 1 + 14 => 1 ]; } return []; From f7b3bcc34c2df3df08efbf5406c5d06dec9e3c6f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jorge=20Cuerdo=20=C3=81lvarez?= Date: Thu, 16 Sep 2021 10:47:32 +0200 Subject: [PATCH 3/7] AC-662: Create phpcs static check for AutogeneratedClassNotInConstructorTest --- ...utogeneratedClassNotInConstructorSniff.php | 57 +++++++------------ ...tedClassNotInConstructorUnitTest.1.php.inc | 11 +++- ...generatedClassNotInConstructorUnitTest.php | 11 +++- 3 files changed, 40 insertions(+), 39 deletions(-) diff --git a/Magento2/Sniffs/PHP/AutogeneratedClassNotInConstructorSniff.php b/Magento2/Sniffs/PHP/AutogeneratedClassNotInConstructorSniff.php index 91fdc265..f2e79a1d 100644 --- a/Magento2/Sniffs/PHP/AutogeneratedClassNotInConstructorSniff.php +++ b/Magento2/Sniffs/PHP/AutogeneratedClassNotInConstructorSniff.php @@ -15,15 +15,7 @@ */ class AutogeneratedClassNotInConstructorSniff implements Sniff { - const ERROR_CODE = 'AUTOGENERATED_CLASS_NOT_IN_CONSTRUCTOR'; - /** - * @var mixed - */ - private $constructorScopeOpener; - /** - * @var mixed - */ - private $constructorScopeCloser; + private const ERROR_CODE = 'AUTOGENERATED_CLASS_NOT_IN_CONSTRUCTOR'; /** * @var array @@ -55,9 +47,6 @@ public function process(File $phpcsFile, $stackPtr) $this->registerConstructorParameters($phpcsFile, $stackPtr); } if ($phpcsFile->getTokens()[$stackPtr]['type'] === 'T_DOUBLE_COLON') { - if (!$this->isInsideConstruct($stackPtr)) { - return; - } if (!$this->isObjectManagerGetInstance($phpcsFile, $stackPtr)) { return; } @@ -73,9 +62,12 @@ public function process(File $phpcsFile, $stackPtr) $variableInParameters = false; if ($variable = $phpcsFile->findNext(T_VARIABLE, $equalsPtr, $statementEnd)) { $variableName = $phpcsFile->getTokens()[$variable]['content']; + if ($variableName === '$this') { + $variableName = $this->getNext($phpcsFile, $variable, $statementEnd, T_STRING)['content']; + } foreach ($this->constructorParameters as $parameter) { $parameterName = $parameter['name']; - if ($parameterName === $variableName) { + if ($this->variableName($parameterName) === $this->variableName($variableName)) { $variableInParameters = true; } } @@ -91,7 +83,7 @@ public function process(File $phpcsFile, $stackPtr) $className = $this->getClassNamespace($className); - $phpcsFile->addError( + $phpcsFile->addWarning( sprintf("Class %s needs to be requested in constructor, " . "otherwise compiler will not be able to find and generate these classes", $className), $stackPtr, @@ -121,17 +113,6 @@ private function isObjectManagerGetInstance(File $phpcsFile, int $stackPtr): boo return false; } - /** - * Checks if the code is inside __construct - * - * @param int $stackPtr - * @return bool - */ - private function isInsideConstruct(int $stackPtr): bool - { - return $stackPtr > $this->constructorScopeOpener && $stackPtr < $this->constructorScopeCloser; - } - /** * Get the complete class namespace from the use's * @@ -176,9 +157,6 @@ private function registerConstructorParameters(File $phpcsFile, int $stackPtr): $functionName = $phpcsFile->getDeclarationName($stackPtr); if ($functionName == '__construct') { $this->constructorParameters = $phpcsFile->getMethodParameters($stackPtr); - $this->constructorScopeOpener = $phpcsFile->getTokens()[$stackPtr]['scope_opener']; - $this->constructorScopeCloser = $phpcsFile->getTokens()[$stackPtr]['scope_closer']; - } } @@ -186,26 +164,35 @@ private function registerConstructorParameters(File $phpcsFile, int $stackPtr): * Get next token * * @param File $phpcsFile - * @param int $nextDoubleColumn - * @param int $statementEnd + * @param int $from + * @param int $to * @param int|string|array $types * @return mixed */ - private function getNext(File $phpcsFile, int $nextDoubleColumn, int $statementEnd, $types) + private function getNext(File $phpcsFile, int $from, int $to, $types) { - return $phpcsFile->getTokens()[$phpcsFile->findNext($types, $nextDoubleColumn + 1, $statementEnd)]; + return $phpcsFile->getTokens()[$phpcsFile->findNext($types, $from + 1, $to)]; } /** * Get previous token * * @param File $phpcsFile - * @param int $nextDoubleColumn + * @param int $from * @param int|string|array $types * @return mixed */ - private function getPrevious(File $phpcsFile, int $nextDoubleColumn, $types) + private function getPrevious(File $phpcsFile, int $from, $types) + { + return $phpcsFile->getTokens()[$phpcsFile->findPrevious($types, $from - 1)]; + } + + /** + * @param $parameterName + * @return array|string|string[] + */ + protected function variableName($parameterName) { - return $phpcsFile->getTokens()[$phpcsFile->findPrevious($types, $nextDoubleColumn - 1)]; + return str_replace('$', '', $parameterName); } } diff --git a/Magento2/Tests/PHP/AutogeneratedClassNotInConstructorUnitTest.1.php.inc b/Magento2/Tests/PHP/AutogeneratedClassNotInConstructorUnitTest.1.php.inc index 6889b13e..e16177e8 100644 --- a/Magento2/Tests/PHP/AutogeneratedClassNotInConstructorUnitTest.1.php.inc +++ b/Magento2/Tests/PHP/AutogeneratedClassNotInConstructorUnitTest.1.php.inc @@ -21,9 +21,18 @@ class Good /** * @return Model */ - public function otherMethodThatCallsGetInstance(): void + public function otherMethodThatCallsGetInstanceBad(): void { $model = ObjectManager::getInstance()->get(Model::class); $model->something(); } + + /** + * @return Model + */ + public function otherMethodThatCallsGetInstanceGood(): void + { + $model = $this->model ?? ObjectManager::getInstance()->get(Model::class); + $model->something(); + } } diff --git a/Magento2/Tests/PHP/AutogeneratedClassNotInConstructorUnitTest.php b/Magento2/Tests/PHP/AutogeneratedClassNotInConstructorUnitTest.php index 33a67241..5c3a2f70 100644 --- a/Magento2/Tests/PHP/AutogeneratedClassNotInConstructorUnitTest.php +++ b/Magento2/Tests/PHP/AutogeneratedClassNotInConstructorUnitTest.php @@ -14,11 +14,16 @@ class AutogeneratedClassNotInConstructorUnitTest extends AbstractSniffUnitTest /** * @inheritdoc */ - public function getErrorList($filename = '') + public function getWarningList($filename = '') { + if ($filename === 'AutogeneratedClassNotInConstructorUnitTest.1.php.inc') { + return [ + 26 => 1, + ]; + } if ($filename === 'AutogeneratedClassNotInConstructorUnitTest.2.php.inc') { return [ - 14 => 1 + 14 => 1, ]; } return []; @@ -27,7 +32,7 @@ public function getErrorList($filename = '') /** * @inheritdoc */ - public function getWarningList() + public function getErrorList() { return []; } From c06cbc22be83622d5db80d5f254a89e4e7c19132 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jorge=20Cuerdo=20=C3=81lvarez?= Date: Thu, 16 Sep 2021 10:51:13 +0200 Subject: [PATCH 4/7] AC-662: Create phpcs static check for AutogeneratedClassNotInConstructorTest --- .../PHP/AutogeneratedClassNotInConstructorSniff.php | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/Magento2/Sniffs/PHP/AutogeneratedClassNotInConstructorSniff.php b/Magento2/Sniffs/PHP/AutogeneratedClassNotInConstructorSniff.php index f2e79a1d..5b882dda 100644 --- a/Magento2/Sniffs/PHP/AutogeneratedClassNotInConstructorSniff.php +++ b/Magento2/Sniffs/PHP/AutogeneratedClassNotInConstructorSniff.php @@ -63,7 +63,7 @@ public function process(File $phpcsFile, $stackPtr) if ($variable = $phpcsFile->findNext(T_VARIABLE, $equalsPtr, $statementEnd)) { $variableName = $phpcsFile->getTokens()[$variable]['content']; if ($variableName === '$this') { - $variableName = $this->getNext($phpcsFile, $variable, $statementEnd, T_STRING)['content']; + $variableName = $this->getNext($phpcsFile, $variable, $statementEnd, T_STRING)['content']; } foreach ($this->constructorParameters as $parameter) { $parameterName = $parameter['name']; @@ -188,10 +188,12 @@ private function getPrevious(File $phpcsFile, int $from, $types) } /** - * @param $parameterName - * @return array|string|string[] + * Get name of the variable without $ + * + * @param string $parameterName + * @return string */ - protected function variableName($parameterName) + protected function variableName(string $parameterName): string { return str_replace('$', '', $parameterName); } From 1d499c26bae36cc1061ee258f02eb4baba0247c3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jorge=20Cuerdo=20=C3=81lvarez?= Date: Thu, 16 Sep 2021 12:42:39 +0200 Subject: [PATCH 5/7] AC-662: Create phpcs static check for AutogeneratedClassNotInConstructorTest --- .../PHP/AutogeneratedClassNotInConstructorSniff.php | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/Magento2/Sniffs/PHP/AutogeneratedClassNotInConstructorSniff.php b/Magento2/Sniffs/PHP/AutogeneratedClassNotInConstructorSniff.php index 5b882dda..30c4620c 100644 --- a/Magento2/Sniffs/PHP/AutogeneratedClassNotInConstructorSniff.php +++ b/Magento2/Sniffs/PHP/AutogeneratedClassNotInConstructorSniff.php @@ -11,7 +11,7 @@ use PHP_CodeSniffer\Sniffs\Sniff; /** - * Detects possible usage of 'var' language construction. + * Detects bad usages of ObjectManager in constructor. */ class AutogeneratedClassNotInConstructorSniff implements Sniff { @@ -104,13 +104,10 @@ private function isObjectManagerGetInstance(File $phpcsFile, int $stackPtr): boo { $prev = $phpcsFile->findPrevious(T_STRING, $stackPtr - 1); $next = $phpcsFile->findNext(T_STRING, $stackPtr + 1); - if ($prev && + return $prev && $next && $phpcsFile->getTokens()[$prev]['content'] === 'ObjectManager' && - $phpcsFile->getTokens()[$next]['content'] === 'getInstance') { - return true; - } - return false; + $phpcsFile->getTokens()[$next]['content'] === 'getInstance'; } /** From 504a50bef1d8902ddf0bf0f41017e1656dec4977 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jorge=20Cuerdo=20=C3=81lvarez?= Date: Tue, 21 Sep 2021 16:11:20 +0200 Subject: [PATCH 6/7] AC-662: Create phpcs static check for AutogeneratedClassNotInConstructorTest --- ...utogeneratedClassNotInConstructorSniff.php | 82 ++++++++++++------- ...tedClassNotInConstructorUnitTest.1.php.inc | 2 +- Magento2/ruleset.xml | 3 +- 3 files changed, 57 insertions(+), 30 deletions(-) diff --git a/Magento2/Sniffs/PHP/AutogeneratedClassNotInConstructorSniff.php b/Magento2/Sniffs/PHP/AutogeneratedClassNotInConstructorSniff.php index 30c4620c..0aca5737 100644 --- a/Magento2/Sniffs/PHP/AutogeneratedClassNotInConstructorSniff.php +++ b/Magento2/Sniffs/PHP/AutogeneratedClassNotInConstructorSniff.php @@ -41,7 +41,7 @@ public function register() public function process(File $phpcsFile, $stackPtr) { if ($phpcsFile->getTokens()[$stackPtr]['type'] === 'T_USE') { - $this->registerUses($phpcsFile, $stackPtr); + $this->registerUse($phpcsFile, $stackPtr); } if ($phpcsFile->getTokens()[$stackPtr]['type'] === 'T_FUNCTION') { $this->registerConstructorParameters($phpcsFile, $stackPtr); @@ -59,29 +59,9 @@ public function process(File $phpcsFile, $stackPtr) return; } - $variableInParameters = false; - if ($variable = $phpcsFile->findNext(T_VARIABLE, $equalsPtr, $statementEnd)) { - $variableName = $phpcsFile->getTokens()[$variable]['content']; - if ($variableName === '$this') { - $variableName = $this->getNext($phpcsFile, $variable, $statementEnd, T_STRING)['content']; - } - foreach ($this->constructorParameters as $parameter) { - $parameterName = $parameter['name']; - if ($this->variableName($parameterName) === $this->variableName($variableName)) { - $variableInParameters = true; - } - } - } - - if (!$variableInParameters) { + if (!$this->isVariableInConstructorParameters($phpcsFile, $equalsPtr, $statementEnd)) { $next = $stackPtr; - while ($next = $phpcsFile->findNext(T_DOUBLE_COLON, $next + 1, $statementEnd)) { - if ($this->getNext($phpcsFile, $next, $statementEnd, T_STRING)['content'] === 'class') { - $className = $this->getPrevious($phpcsFile, $next, T_STRING)['content']; - } - } - - $className = $this->getClassNamespace($className); + $className = $this->obtainClassToGetOrCreate($phpcsFile, $next, $statementEnd); $phpcsFile->addWarning( sprintf("Class %s needs to be requested in constructor, " . @@ -118,9 +98,9 @@ private function isObjectManagerGetInstance(File $phpcsFile, int $stackPtr): boo */ private function getClassNamespace(string $className): string { - foreach ($this->uses as $use) { - if (end($use) === $className) { - $className = implode('/', $use); + foreach ($this->uses as $key => $use) { + if ($key === $className) { + return $use; } } return $className; @@ -132,7 +112,7 @@ private function getClassNamespace(string $className): string * @param File $phpcsFile * @param int $stackPtr */ - private function registerUses(File $phpcsFile, int $stackPtr): void + private function registerUse(File $phpcsFile, int $stackPtr): void { $useEnd = $phpcsFile->findEndOfStatement($stackPtr); $use = []; @@ -140,7 +120,13 @@ private function registerUses(File $phpcsFile, int $stackPtr): void while ($usePosition = $phpcsFile->findNext(T_STRING, $usePosition + 1, $useEnd)) { $use[] = $phpcsFile->getTokens()[$usePosition]['content']; } - $this->uses [] = $use; + + $key = end($use); + if ($phpcsFile->findNext(T_AS, $stackPtr, $useEnd)) { + $this->uses[$key] = implode("\\", array_slice($use, 0, count($use) - 1)); + } else { + $this->uses[$key] = implode("\\", $use); + } } /** @@ -194,4 +180,44 @@ protected function variableName(string $parameterName): string { return str_replace('$', '', $parameterName); } + + /** + * @param File $phpcsFile + * @param int $equalsPtr + * @param int $statementEnd + * @return bool + */ + private function isVariableInConstructorParameters(File $phpcsFile, int $equalsPtr, int $statementEnd): bool + { + if ($variable = $phpcsFile->findNext(T_VARIABLE, $equalsPtr, $statementEnd)) { + $variableName = $phpcsFile->getTokens()[$variable]['content']; + if ($variableName === '$this') { + $variableName = $this->getNext($phpcsFile, $variable, $statementEnd, T_STRING)['content']; + } + foreach ($this->constructorParameters as $parameter) { + $parameterName = $parameter['name']; + if ($this->variableName($parameterName) === $this->variableName($variableName)) { + return true; + } + } + } + return false; + } + + /** + * @param File $phpcsFile + * @param $next + * @param int $statementEnd + * @return string + */ + private function obtainClassToGetOrCreate(File $phpcsFile, $next, int $statementEnd): string + { + while ($next = $phpcsFile->findNext(T_DOUBLE_COLON, $next + 1, $statementEnd)) { + if ($this->getNext($phpcsFile, $next, $statementEnd, T_STRING)['content'] === 'class') { + $className = $this->getPrevious($phpcsFile, $next, T_STRING)['content']; + } + } + + return $this->getClassNamespace($className); + } } diff --git a/Magento2/Tests/PHP/AutogeneratedClassNotInConstructorUnitTest.1.php.inc b/Magento2/Tests/PHP/AutogeneratedClassNotInConstructorUnitTest.1.php.inc index e16177e8..f7a461ed 100644 --- a/Magento2/Tests/PHP/AutogeneratedClassNotInConstructorUnitTest.1.php.inc +++ b/Magento2/Tests/PHP/AutogeneratedClassNotInConstructorUnitTest.1.php.inc @@ -7,7 +7,7 @@ declare(strict_types = 1); namespace Magento2\Tests\PHP; -use Magento2\Model; +use Magento2\OneModel as Model; class Good { diff --git a/Magento2/ruleset.xml b/Magento2/ruleset.xml index 212bf246..347aab6e 100644 --- a/Magento2/ruleset.xml +++ b/Magento2/ruleset.xml @@ -151,6 +151,7 @@ warning + *\/.phtml$ 9 warning @@ -247,7 +248,7 @@ 8 warning - + *\/widget.xml$ 8 warning From 7d317bafa76504117fabbae5561dfefefdd78be8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jorge=20Cuerdo=20=C3=81lvarez?= Date: Wed, 22 Sep 2021 11:00:16 +0200 Subject: [PATCH 7/7] AC-662: Create phpcs static check for AutogeneratedClassNotInConstructorTest --- .../PHP/AutogeneratedClassNotInConstructorSniff.php | 13 ++++++++----- .../AutogeneratedClassNotInConstructorUnitTest.php | 4 ++-- Magento2/ruleset.xml | 5 +++++ 3 files changed, 15 insertions(+), 7 deletions(-) diff --git a/Magento2/Sniffs/PHP/AutogeneratedClassNotInConstructorSniff.php b/Magento2/Sniffs/PHP/AutogeneratedClassNotInConstructorSniff.php index 0aca5737..f99346af 100644 --- a/Magento2/Sniffs/PHP/AutogeneratedClassNotInConstructorSniff.php +++ b/Magento2/Sniffs/PHP/AutogeneratedClassNotInConstructorSniff.php @@ -60,10 +60,9 @@ public function process(File $phpcsFile, $stackPtr) } if (!$this->isVariableInConstructorParameters($phpcsFile, $equalsPtr, $statementEnd)) { - $next = $stackPtr; - $className = $this->obtainClassToGetOrCreate($phpcsFile, $next, $statementEnd); + $className = $this->obtainClassToGetOrCreate($phpcsFile, $stackPtr, $statementEnd); - $phpcsFile->addWarning( + $phpcsFile->addError( sprintf("Class %s needs to be requested in constructor, " . "otherwise compiler will not be able to find and generate these classes", $className), $stackPtr, @@ -182,6 +181,8 @@ protected function variableName(string $parameterName): string } /** + * Checks if a variable is present in the constructor parameters + * * @param File $phpcsFile * @param int $equalsPtr * @param int $statementEnd @@ -205,12 +206,14 @@ private function isVariableInConstructorParameters(File $phpcsFile, int $equalsP } /** + * Obtain the class inside ObjectManager::getInstance()->get|create() + * * @param File $phpcsFile - * @param $next + * @param int $next * @param int $statementEnd * @return string */ - private function obtainClassToGetOrCreate(File $phpcsFile, $next, int $statementEnd): string + private function obtainClassToGetOrCreate(File $phpcsFile, int $next, int $statementEnd): string { while ($next = $phpcsFile->findNext(T_DOUBLE_COLON, $next + 1, $statementEnd)) { if ($this->getNext($phpcsFile, $next, $statementEnd, T_STRING)['content'] === 'class') { diff --git a/Magento2/Tests/PHP/AutogeneratedClassNotInConstructorUnitTest.php b/Magento2/Tests/PHP/AutogeneratedClassNotInConstructorUnitTest.php index 5c3a2f70..9122213f 100644 --- a/Magento2/Tests/PHP/AutogeneratedClassNotInConstructorUnitTest.php +++ b/Magento2/Tests/PHP/AutogeneratedClassNotInConstructorUnitTest.php @@ -14,7 +14,7 @@ class AutogeneratedClassNotInConstructorUnitTest extends AbstractSniffUnitTest /** * @inheritdoc */ - public function getWarningList($filename = '') + public function getErrorList($filename = '') { if ($filename === 'AutogeneratedClassNotInConstructorUnitTest.1.php.inc') { return [ @@ -32,7 +32,7 @@ public function getWarningList($filename = '') /** * @inheritdoc */ - public function getErrorList() + public function getWarningList($filename = '') { return []; } diff --git a/Magento2/ruleset.xml b/Magento2/ruleset.xml index 347aab6e..b33e8a7f 100644 --- a/Magento2/ruleset.xml +++ b/Magento2/ruleset.xml @@ -115,6 +115,11 @@ 10 error + + *\.php$ + 10 + error +