AC-662: Create phpcs static check for AutogeneratedClassNotInConstructorTest

Magento2/Sniffs/PHP/AutogeneratedClassNotInConstructorSniff.php

@@ -0,0 +1,226 @@
+<?php
+/**
+ * Copyright © Magento, Inc. All rights reserved.
+ * See COPYING.txt for license details.
+ */
+declare(strict_types = 1);
+
+namespace Magento2\Sniffs\PHP;
+
+use PHP_CodeSniffer\Files\File;
+use PHP_CodeSniffer\Sniffs\Sniff;
+
+/**
+ * Detects bad usages of ObjectManager in constructor.
+ */
+class AutogeneratedClassNotInConstructorSniff implements Sniff
+{
+    private const ERROR_CODE = 'AUTOGENERATED_CLASS_NOT_IN_CONSTRUCTOR';
+
+    /**
+     * @var array
+     */
+    private $constructorParameters = [];
+
+    /**
+     * @var array
+     */
+    private $uses = [];
+
+    /**
+     * @inheritdoc
+     */
+    public function register()
+    {
+        return [T_FUNCTION, T_DOUBLE_COLON, T_USE];
+    }
+
+    /**
+     * @inheritdoc
+     */
+    public function process(File $phpcsFile, $stackPtr)
+    {
+        if ($phpcsFile->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);
+    }
+}

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

@@ -0,0 +1,38 @@
+<?php
+/**
+ * Copyright © Magento, Inc. All rights reserved.
+ * See COPYING.txt for license details.
+ */
+declare(strict_types = 1);
+
+namespace Magento2\Tests\PHP;
+
+use Magento2\OneModel as Model;
+
+class Good
+{
+    public function __construct(
+        Model $model = null
+    )
+    {
+        $this->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();
+    }
+}

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

@@ -0,0 +1,17 @@
+<?php
+/**
+ * Copyright © Magento, Inc. All rights reserved.
+ * See COPYING.txt for license details.
+ */
+declare(strict_types = 1);
+
+namespace Magento2\Tests\PHP;
+
+class Bad
+{
+    public function __construct()
+    {
+        $this->model = ObjectManager::getInstance()->get(Model::class);
+    }
+}
+

Magento2/Tests/PHP/AutogeneratedClassNotInConstructorUnitTest.php

@@ -0,0 +1,39 @@
+<?php
+/**
+ * Copyright © Magento, Inc. All rights reserved.
+ * See COPYING.txt for license details.
+ */
+declare(strict_types = 1);
+
+namespace Magento2\Tests\PHP;
+
+use PHP_CodeSniffer\Tests\Standards\AbstractSniffUnitTest;
+
+class AutogeneratedClassNotInConstructorUnitTest extends AbstractSniffUnitTest
+{
+    /**
+     * @inheritdoc
+     */
+    public function getErrorList($filename = '')
+    {
+        if ($filename === 'AutogeneratedClassNotInConstructorUnitTest.1.php.inc') {
+            return [
+                26 => 1,
+            ];
+        }
+        if ($filename === 'AutogeneratedClassNotInConstructorUnitTest.2.php.inc') {
+            return [
+                14 => 1,
+            ];
+        }
+        return [];
+    }
+
+    /**
+     * @inheritdoc
+     */
+    public function getWarningList($filename = '')
+    {
+        return [];
+    }
+}

Magento2/ruleset.xml

@@ -130,6 +130,11 @@
         <severity>10</severity>
         <type>error</type>
     </rule>
+    <rule ref="Magento2.PHP.AutogeneratedClassNotInConstructor">
+        <include-pattern>*\.php$</include-pattern>
+        <severity>10</severity>
+        <type>error</type>
+    </rule>
 
     <rule ref="Magento2.Html.HtmlSelfClosingTags">
         <severity>10</severity>
@@ -172,6 +177,7 @@
         <exclude-pattern>*\.xml$</exclude-pattern>
     </rule>
     <rule ref="Magento2.Html.HtmlBinding">
+        <include-pattern>*\/.phtml$</include-pattern>
         <severity>9</severity>
         <type>warning</type>
         <exclude-pattern>*\.xml$</exclude-pattern>
@@ -271,7 +277,7 @@
         <type>warning</type>
         <exclude-pattern>*\.xml$</exclude-pattern>
     </rule>
-    <rule ref="Magento2.Legacy.WidgetXML.FoundObsoleteNode">
+    <rule ref="Magento2.Legacy.WidgetXML">
         <include-pattern>*\/widget.xml$</include-pattern>
         <severity>8</severity>
         <type>warning</type>