Fixed undefined variable error in AutogeneratedClassNotInConstructorSniff

Magento2/Sniffs/PHP/AutogeneratedClassNotInConstructorSniff.php

@@ -17,210 +17,191 @@ class AutogeneratedClassNotInConstructorSniff implements Sniff
 {
     private const ERROR_CODE = 'AutogeneratedClassNotInConstructor';
 
-    /**
-     * @var array
-     */
-    private $constructorParameters = [];
-
-    /**
-     * @var array
-     */
-    private $uses = [];
+    private const AUTOGENERATED_CLASS_SUFFIXES = [
+        'Factory'
+    ];
 
     /**
      * @inheritdoc
      */
     public function register()
     {
-        return [T_FUNCTION, T_DOUBLE_COLON, T_USE];
+        return [T_DOUBLE_COLON];
     }
 
     /**
      * @inheritdoc
      */
     public function process(File $phpcsFile, $stackPtr)
     {
-        if ($phpcsFile->getTokens()[$stackPtr]['type'] === 'T_USE') {
-            $this->registerUse($phpcsFile, $stackPtr);
+        if (!$this->isClass($phpcsFile)) {
+            return;
         }
-        if ($phpcsFile->getTokens()[$stackPtr]['type'] === 'T_FUNCTION') {
-            $this->registerConstructorParameters($phpcsFile, $stackPtr);
+
+        $tokens = $phpcsFile->getTokens();
+
+        if ($tokens[$stackPtr - 1]['content'] !== 'ObjectManager'
+            && $tokens[$stackPtr + 1]['content'] !== 'getInstance'
+        ) {
+            return;
         }
-        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 (!isset($tokens[$stackPtr + 4]) || $tokens[$stackPtr + 4]['code'] !== T_SEMICOLON) {
+            $this->validateRequestedClass(
+                $phpcsFile,
+                $phpcsFile->findNext(T_OBJECT_OPERATOR, $stackPtr)
+            );
+            return;
+        }
 
-            if (!$equalsPtr) {
-                return;
-            }
+        $objectManagerVariableName = $this->getObjectManagerVariableName($phpcsFile, $stackPtr);
 
-            if (!$this->isVariableInConstructorParameters($phpcsFile, $equalsPtr, $statementEnd)) {
-                $className = $this->obtainClassToGetOrCreate($phpcsFile, $stackPtr, $statementEnd);
+        if (!$objectManagerVariableName) {
+            return;
+        }
 
-                $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
-                );
-            }
+        $variablePosition = $phpcsFile->findNext(T_VARIABLE, $stackPtr, null, false, $objectManagerVariableName);
+        if ($variablePosition) {
+            $this->validateRequestedClass($phpcsFile, $phpcsFile->findNext(T_OBJECT_OPERATOR, $variablePosition));
         }
     }
 
     /**
-     * Check if it is a ObjectManager::getInstance
+     * Check if the class is instantiated via get/create method, it is autogenerated and present in constructor
      *
      * @param File $phpcsFile
-     * @param int $stackPtr
-     * @return bool
+     * @param int $arrowPosition
      */
-    private function isObjectManagerGetInstance(File $phpcsFile, int $stackPtr): bool
+    private function validateRequestedClass(File $phpcsFile, int $arrowPosition): void
     {
-        $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';
+        $requestedClass = $this->getRequestedClass($phpcsFile, $arrowPosition);
+
+        if (!$requestedClass
+            || !$this->isClassAutogenerated($requestedClass)
+            || $this->isConstructorParameter($phpcsFile, $requestedClass)
+        ) {
+            return;
+        }
+
+        $phpcsFile->addError(
+            sprintf(
+                'Class %s needs to be requested in constructor, ' .
+                'otherwise compiler will not be able to find and generate this class',
+                $requestedClass
+            ),
+            $arrowPosition,
+            self::ERROR_CODE
+        );
     }
 
     /**
-     * Get the complete class namespace from the use's
+     * Does the class have the suffix common for autogenerated classes e.g. Factory
      *
      * @param string $className
-     * @return string
+     * @return bool
      */
-    private function getClassNamespace(string $className): string
+    private function isClassAutogenerated(string $className): bool
     {
-        foreach ($this->uses as $key => $use) {
-            if ($key === $className) {
-                return $use;
+        foreach (self::AUTOGENERATED_CLASS_SUFFIXES as $suffix) {
+            if (substr($className, -strlen($suffix)) === $suffix) {
+                return true;
             }
         }
-        return $className;
+        return false;
     }
 
     /**
-     * Register php uses
+     * Get the variable name to which the ObjectManager::getInstance() result is assigned
      *
      * @param File $phpcsFile
      * @param int $stackPtr
+     * @return string|null
      */
-    private function registerUse(File $phpcsFile, int $stackPtr): void
+    private function getObjectManagerVariableName(File $phpcsFile, int $stackPtr): ?string
     {
-        $useEnd = $phpcsFile->findEndOfStatement($stackPtr);
-        $use = [];
-        $usePosition = $stackPtr;
-        while ($usePosition = $phpcsFile->findNext(T_STRING, $usePosition + 1, $useEnd)) {
-            $use[] = $phpcsFile->getTokens()[$usePosition]['content'];
+        $matches = [];
+        $found = preg_match(
+            '/(\$[A-Za-z]+) ?= ?ObjectManager::getInstance\(\);/',
+            $phpcsFile->getTokensAsString($stackPtr - 5, 10),
+            $matches
+        );
+
+        if (!$found || !isset($matches[1])) {
+            return null;
         }
 
-        $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);
-        }
+        return $matches[1];
     }
 
     /**
-     * Register php constructor parameters
+     * Get class name requested from ObjectManager
      *
      * @param File $phpcsFile
-     * @param int $stackPtr
+     * @param int $callerPosition
+     * @return string|null
      */
-    private function registerConstructorParameters(File $phpcsFile, int $stackPtr): void
+    private function getRequestedClass(File $phpcsFile, int $callerPosition): ?string
     {
-        $functionName = $phpcsFile->getDeclarationName($stackPtr);
-        if ($functionName == '__construct') {
-            $this->constructorParameters = $phpcsFile->getMethodParameters($stackPtr);
+        $matches = [];
+        $found = preg_match(
+            '/->(get|create)\(([A-Za-z\\\]+)::class/',
+            $phpcsFile->getTokensAsString($callerPosition, $phpcsFile->findNext(T_CLOSE_PARENTHESIS, $callerPosition)),
+            $matches
+        );
+
+        if (!$found || !isset($matches[2])) {
+            return null;
         }
-    }
 
-    /**
-     * 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)];
+        return $matches[2];
     }
 
     /**
-     * Get previous token
+     * Does the file contain class declaration
      *
      * @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
+     * @return bool
      */
-    protected function variableName(string $parameterName): string
+    private function isClass(File $phpcsFile): bool
     {
-        return str_replace('$', '', $parameterName);
+        foreach ($phpcsFile->getTokens() as $token) {
+            if ($token['code'] === T_CLASS) {
+                return true;
+            }
+        }
+        return false;
     }
 
     /**
-     * Checks if a variable is present in the constructor parameters
+     * Get an array of constructor parameters
      *
      * @param File $phpcsFile
-     * @param int $equalsPtr
-     * @param int $statementEnd
-     * @return bool
+     * @return array
      */
-    private function isVariableInConstructorParameters(File $phpcsFile, int $equalsPtr, int $statementEnd): bool
+    private function getConstructorParameters(File $phpcsFile): array
     {
-        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;
-                }
+        foreach ($phpcsFile->getTokens() as $stackPtr => $token) {
+            if ($token['code'] === T_FUNCTION && $phpcsFile->getDeclarationName($stackPtr) === '__construct') {
+                return $phpcsFile->getMethodParameters($stackPtr);
             }
         }
-        return false;
+        return [];
     }
 
     /**
-     * Obtain the class inside ObjectManager::getInstance()->get|create()
+     * Is the class name present between constructor parameters
      *
      * @param File $phpcsFile
-     * @param int $next
-     * @param int $statementEnd
-     * @return string
+     * @param string $className
+     * @return bool
      */
-    private function obtainClassToGetOrCreate(File $phpcsFile, int $next, int $statementEnd): string
+    private function isConstructorParameter(File $phpcsFile, string $className): bool
     {
-        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'];
+        foreach ($this->getConstructorParameters($phpcsFile) as $parameter) {
+            if (strpos($parameter['content'], $className) !== false) {
+                return true;
             }
         }
-
-        return $this->getClassNamespace($className);
+        return false;
     }
 }

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

@@ -12,19 +12,19 @@ use Magento2\OneModel as Model;
 class Good
 {
     public function __construct(
-        Model $model = null
+        ModelFactory $model = null
     )
     {
-        $this->model = $model ?? ObjectManager::getInstance()->get(Model::class);
+        $this->model = $model ?? ObjectManager::getInstance()->get(ModelFactory::class);
     }
 
     /**
      * @return Model
      */
     public function otherMethodThatCallsGetInstanceBad(): void
     {
-        $model = ObjectManager::getInstance()->get(Model::class);
-        $model->something();
+        $modelFactory = ObjectManager::getInstance()->get(OtherFactory::class);
+        $modelFactory->create();
     }
 
     /**
@@ -35,4 +35,18 @@ class Good
         $model = $this->model ?? ObjectManager::getInstance()->get(Model::class);
         $model->something();
     }
+
+    public function variablePositive(): void
+    {
+        $objectManager = ObjectManager::getInstance();
+        $this->_entityFactory = $objectManager->get(EntityFactoryInterface::class);
+    }
+
+    public function variableNegative(): void
+    {
+        $objectManager = ObjectManager::getInstance();
+        $this->_entityFactory = $objectManager->get(EntityFactory::class);
+    }
 }
+
+

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

@@ -7,11 +7,34 @@ declare(strict_types = 1);
 
 namespace Magento2\Tests\PHP;
 
+use Magento\Catalog\Model\Product;
+use Magento\Eav\Api\AttributeRepositoryInterface;
+use Magento\Eav\Setup\AddOptionToAttribute;
+use Magento\Eav\Setup\EavSetup;
+use Magento\Eav\Setup\EavSetupFactory;
+use Magento\Framework\App\ObjectManager;
+use Magento\Framework\Setup\ModuleDataSetupInterface;
+
 class Bad
 {
     public function __construct()
     {
-        $this->model = ObjectManager::getInstance()->get(Model::class);
+        $this->model = ObjectManager::getInstance()->get(ModelFactory::class);
+    }
+
+    protected function setUp(): void
+    {
+        ObjectManager::getInstance()
+            ->get(EavSetupFactory::class);
+        $objectManager = ObjectManager::getInstance();
+
+        /** @var EavSetup $eavSetup */
+        $this->eavSetup = $objectManager
+            ->get(EavSetupFactory::class)
+            ->create(['setup' => $this->setup]);
+
+        ObjectManager::getInstance()
+            ->get(\Full\Class\NameFactory::class);
     }
 }