diff --git a/Magento2/Sniffs/PHP/AutogeneratedClassNotInConstructorSniff.php b/Magento2/Sniffs/PHP/AutogeneratedClassNotInConstructorSniff.php new file mode 100644 index 00000000..f99346af --- /dev/null +++ b/Magento2/Sniffs/PHP/AutogeneratedClassNotInConstructorSniff.php @@ -0,0 +1,226 @@ +getTokens()[$stackPtr]['type'] === 'T_USE') { + $this->registerUse($phpcsFile, $stackPtr); + } + if ($phpcsFile->getTokens()[$stackPtr]['type'] === 'T_FUNCTION') { + $this->registerConstructorParameters($phpcsFile, $stackPtr); + } + if ($phpcsFile->getTokens()[$stackPtr]['type'] === 'T_DOUBLE_COLON') { + if (!$this->isObjectManagerGetInstance($phpcsFile, $stackPtr)) { + return; + } + + $statementStart = $phpcsFile->findStartOfStatement($stackPtr); + $statementEnd = $phpcsFile->findEndOfStatement($stackPtr); + $equalsPtr = $phpcsFile->findNext(T_EQUAL, $statementStart, $statementEnd); + + if (!$equalsPtr) { + return; + } + + if (!$this->isVariableInConstructorParameters($phpcsFile, $equalsPtr, $statementEnd)) { + $className = $this->obtainClassToGetOrCreate($phpcsFile, $stackPtr, $statementEnd); + + $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); + return $prev && + $next && + $phpcsFile->getTokens()[$prev]['content'] === 'ObjectManager' && + $phpcsFile->getTokens()[$next]['content'] === 'getInstance'; + } + + /** + * Get the complete class namespace from the use's + * + * @param string $className + * @return string + */ + private function getClassNamespace(string $className): string + { + foreach ($this->uses as $key => $use) { + if ($key === $className) { + return $use; + } + } + return $className; + } + + /** + * Register php uses + * + * @param File $phpcsFile + * @param int $stackPtr + */ + private function registerUse(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']; + } + + $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); + } + } + + /** + * 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); + } + } + + /** + * Get next token + * + * @param File $phpcsFile + * @param int $from + * @param int $to + * @param int|string|array $types + * @return mixed + */ + private function getNext(File $phpcsFile, int $from, int $to, $types) + { + return $phpcsFile->getTokens()[$phpcsFile->findNext($types, $from + 1, $to)]; + } + + /** + * Get previous token + * + * @param File $phpcsFile + * @param int $from + * @param int|string|array $types + * @return mixed + */ + private function getPrevious(File $phpcsFile, int $from, $types) + { + return $phpcsFile->getTokens()[$phpcsFile->findPrevious($types, $from - 1)]; + } + + /** + * Get name of the variable without $ + * + * @param string $parameterName + * @return string + */ + protected function variableName(string $parameterName): string + { + return str_replace('$', '', $parameterName); + } + + /** + * Checks if a variable is present in the constructor parameters + * + * @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; + } + + /** + * Obtain the class inside ObjectManager::getInstance()->get|create() + * + * @param File $phpcsFile + * @param int $next + * @param int $statementEnd + * @return 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') { + $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 new file mode 100644 index 00000000..f7a461ed --- /dev/null +++ b/Magento2/Tests/PHP/AutogeneratedClassNotInConstructorUnitTest.1.php.inc @@ -0,0 +1,38 @@ +model = $model ?? ObjectManager::getInstance()->get(Model::class); + } + + /** + * @return Model + */ + 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.2.php.inc b/Magento2/Tests/PHP/AutogeneratedClassNotInConstructorUnitTest.2.php.inc new file mode 100644 index 00000000..a72f5bb0 --- /dev/null +++ b/Magento2/Tests/PHP/AutogeneratedClassNotInConstructorUnitTest.2.php.inc @@ -0,0 +1,17 @@ +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..9122213f --- /dev/null +++ b/Magento2/Tests/PHP/AutogeneratedClassNotInConstructorUnitTest.php @@ -0,0 +1,39 @@ + 1, + ]; + } + if ($filename === 'AutogeneratedClassNotInConstructorUnitTest.2.php.inc') { + return [ + 14 => 1, + ]; + } + return []; + } + + /** + * @inheritdoc + */ + public function getWarningList($filename = '') + { + return []; + } +} diff --git a/Magento2/ruleset.xml b/Magento2/ruleset.xml index e1771984..f6683dad 100644 --- a/Magento2/ruleset.xml +++ b/Magento2/ruleset.xml @@ -130,6 +130,11 @@ 10 error + + *\.php$ + 10 + error + 10 @@ -172,6 +177,7 @@ *\.xml$ + *\/.phtml$ 9 warning *\.xml$ @@ -271,7 +277,7 @@ warning *\.xml$ - + *\/widget.xml$ 8 warning