AC-662: Create phpcs static check for AutogeneratedClassNotInConstructorTest

[jcuerdo]

Correct:

public function __construct(
    ...
    Model $model = null
) {
    $this->model = $model ?? ObjectManager::getInstance()->get(Model::class);
}

Incorrect:

public function __construct(
    ...
) {
    $this->model = ObjectManager::getInstance()->get(Model::class);
}

 
This check is not applicable to factories. If the class name (file name) ends with Factory - this check should be skipped.

[svera]

I guess there are changes missing also in ruleset, as we should ignore factory files

[svera]

Is this comment accurate?

[svera]

This can be simplified to

        return $prev &&
               $next &&
               $phpcsFile->getTokens()[$prev]['content'] === 'ObjectManager' &&
               $phpcsFile->getTokens()[$next]['content'] === 'getInstance';

[svera]

There is another case missing here, what about assigning an object manager instance to a variable?

Suggested change

	}
	}
	/**
	* @return Model
	*/
	public function objectManagerAssignedToAVariableBad(): void
	{
	$objectManager = ObjectManager::getInstance();
	$model = $objectManager->get(Model::class);
	$model->something();
	}

[jcuerdo]

This is not on the ticket. And I can see in the original sniff is not covered I guess.

[sivaschenko]

Thanks for the PR @jcuerdo ! Please see my review comments

[sivaschenko]

Suggested change

	$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;
	return $phpcsFile->findPrevious(T_STRING, $stackPtr - 1) &&
	$phpcsFile->findNext(T_STRING, $stackPtr + 1) &&
	$phpcsFile->getTokens()[$prev]['content'] === 'ObjectManager' &&
	$phpcsFile->getTokens()[$next]['content'] === 'getInstance';

[jcuerdo]

This code does not work, you are using variables $prev and $next that are not defined.

[sivaschenko]

Is this function handling alias like use Magento\Module\Class as MagentoModuleClass;?

[jcuerdo]

Not included.

[sivaschenko]

It will be easier to process use in getClassNamespace if uses will be stored this way

Suggested change

	$this->uses [] = $use;
	$this->uses[end($use)] = implode('\', $use);

[jcuerdo]

This option is easier to compare on line 122.

[sivaschenko]

This method is too long, please consider decomposition into several private functions

[jcuerdo]

@magento import pr to magento-commerce/magento-coding-standard

[m2-github-services]

@jcuerdo the Pull Request is successfully imported.