Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

M2.1: "setup:di:compile" doesn't work with the "::class" keyword #5629

Closed
adragus-inviqa opened this issue Jul 13, 2016 · 7 comments
Closed

Comments

@adragus-inviqa
Copy link
Contributor

adragus-inviqa commented Jul 13, 2016

The same problem as #1722.

Steps to reproduce

  1. Install Magento 2.1
  2. Create a module, somewhere having something like $objectManager->get(\My\Class::class).
  3. run php bin/magento setup:di:compile -vvv

Expected result

  1. A success message.

Actual result

  1. Exception (below).
  2. Realised that maybe this is the reason core devs are not using ::class yet in the M2 codebase. :trollface:
  [Exception]
  Notice: Uninitialized string offset: 1 in /var/www/play.wth/setup/src/Magento/Setup/Module/Di/Code/Reader/FileScanner.php on line 328



Exception trace:
 () at /var/www/play.wth/vendor/magento/framework/App/ErrorHandler.php:61
 Magento\Framework\App\ErrorHandler->handler() at /var/www/play.wth/setup/src/Magento/Setup/Module/Di/Code/Reader/FileScanner.php:328
 Magento\Setup\Module\Di\Code\Reader\FileScanner->scan() at /var/www/play.wth/vendor/zendframework/zend-code/src/Scanner/TokenArrayScanner.php:134
 Zend\Code\Scanner\TokenArrayScanner->getClassNames() at /var/www/play.wth/setup/src/Magento/Setup/Module/Di/Code/Reader/ClassesScanner.php:67
 Magento\Setup\Module\Di\Code\Reader\ClassesScanner->getList() at /var/www/play.wth/setup/src/Magento/Setup/Module/Di/App/Task/Operation/RepositoryGenerator.php:61
 Magento\Setup\Module\Di\App\Task\Operation\RepositoryGenerator->doOperation() at /var/www/play.wth/setup/src/Magento/Setup/Module/Di/App/Task/Manager.php:56
 Magento\Setup\Module\Di\App\Task\Manager->process() at /var/www/play.wth/setup/src/Magento/Setup/Console/Command/DiCompileCommand.php:196
 Magento\Setup\Console\Command\DiCompileCommand->execute() at /var/www/play.wth/vendor/symfony/console/Symfony/Component/Console/Command/Command.php:257
 Symfony\Component\Console\Command\Command->run() at /var/www/play.wth/vendor/symfony/console/Symfony/Component/Console/Application.php:874
 Symfony\Component\Console\Application->doRunCommand() at /var/www/play.wth/vendor/symfony/console/Symfony/Component/Console/Application.php:195
 Symfony\Component\Console\Application->doRun() at /var/www/play.wth/vendor/magento/framework/Console/Cli.php:96
 Magento\Framework\Console\Cli->doRun() at /var/www/play.wth/vendor/symfony/console/Symfony/Component/Console/Application.php:126
 Symfony\Component\Console\Application->run() at /var/www/play.wth/bin/magento:23


setup:di:compile

Btw, although zendframework/zend-code#70 has the exact same error, I don't think the PR for it is well suited for us.

I think it's better to strenghten this condition in a similar manner as this one: 4cfc4ab.

@stamster
Copy link

stamster commented Jul 14, 2016

The core issue might be default PHP behaviour, from official PHP manual:

The class name resolution using ::class is a compile time transformation. That means at the time the class name string is created no autoloading has happened yet. As a consequence, class names are expanded even if the class does not exist. No error is issued in that case.

What is the exact point of using such 🆑 via ObjectManager?

use \Magento\Framework\App\ObjectManager as Instance;

$drpMsDataHlp = Instance::getInstance()->get(\Unirgy\DropshipMicrosite\Helper\Data);
// versus:
$drpMsDataHlp = Instance::getInstance()->get(\Unirgy\DropshipMicrosite\Helper\Data::class);

?

@Vinai
Copy link
Contributor

Vinai commented Jul 14, 2016

This is NOT valid PHP (try it):

$drpMsDataHlp = Instance::getInstance()->get(\Unirgy\DropshipMicrosite\Helper\Data);

The characters \Unirgy\DropshipMicrosite\Helper\Data are an undefined symbol. They would have to be enclosed in single or double quotes for example to represent a string.

On the other hand, with the ::class constant this IS valid PHP:

$drpMsDataHlp = Instance::getInstance()->get(\Unirgy\DropshipMicrosite\Helper\Data::class);

The characters \Unirgy\DropshipMicrosite\Helper\Data::class represent a string literal, just like '\Unirgy\DropshipMicrosite\Helper\Data' (note the surrounding single quotes).

The benefit of using the ::class constant is that it enables static code analysis tools (like PHPStorm, for example) to gather usage information, and apply class name refactorings better. This in turn makes navigating the source code and refactoring during development quicker and less error prone.

As far as I can see the error @adragus-inviqa reported is an unhandled PHP tokenizer return value, not a issue of the PHP engine.

@stamster
Copy link

stamster commented Jul 14, 2016

@Vinai: I made a mistake by copy/pasting entire row here, with removal of ::class part. You are correct normally. BTW, you provided very good clarification!

@adragus-inviqa
Copy link
Contributor Author

In all fairness, \Unirgy\DropshipMicrosite\Helper\Data could be the Data constant on the \Unirgy\DropshipMicrosite\Helper class.

So Instance::getInstance()->get(\Unirgy\DropshipMicrosite\Helper\Data); is valid syntax. But can be a runtime-error. 😸

</nitpick>

@Vinai
Copy link
Contributor

Vinai commented Jul 14, 2016

@adragus-inviqa: I believe that would need to be \Unirgy\DropshipMicrosite\Helper::Data.

Quick test:

namespace Foo {
    class Bar
    {
        const Baz = 'qux';
    }
}

namespace {
    echo \Foo\Bar\Baz . PHP_EOL;
}

gives

PHP Fatal error:  Undefined constant 'Foo\Bar\Baz'

The code echo \Foo\Bar::Baz . PHP_EOL; is valid though.

</nitnitpicking>

@adragus-inviqa
Copy link
Contributor Author

adragus-inviqa commented Jul 14, 2016

Yeah, my nitpicking was moot, as I was refering to a constant.

@adragus-inviqa
Copy link
Contributor Author

Closing, as it seems to be fixed in #1722

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants