Magento2: PHP7 return types incompatibile with generate code

[mikeymike]

Preconditions

1. PHP 7.0.8 (any compatible PHP 7 release should be sufficient though)

Steps to reproduce

1. Blank install
2. Create a new block and add a method with a return type in PHP7 format e.g : string
3. Use the block in some layout somewhere
4. Hit the frontend to use the block

Expected result

1. The page should load and the block code be executed correctly

Actual result

Instead you get a fatal error from PHP because the generated interceptor class doesn't have a return type at all.

PHP Fatal error:  Declaration of XX\YY\Block\CustomBlock\Interceptor::someMethod() must be compatible with XX\YY\Block\CustomBlock\::someMethod(): bool in .../var/generation/XX/YY/Block/CustomBlock/Interceptor.php on line X

---

This of course can be a tricky one to resolve purely based on the fact you wouldn't want to break BC with PHP 5.X.

The only resolutions I can think of at this point is...

1. Check the version at runtime
2. Set the PHP version (like we set the deploy mode)

[AydinHassan]

It seems the version of zendframework/zend-code Magento 2 ships with does not support return type hints in code generation, nor is it easy to update the version of zendframework/zend-code due to conflicting dependencies.

We decided to temporarily patch this and bring in some zendframework/zend-code features to add return type hints. Here is a patch if anyone else wants this feature: https://gist.github.com/AydinHassan/064f4bbd33fc118f9fa655811df6a660.

Note that you will need to put it in your own module and rename the namespaces.

[mwgamble]

As the scanner code in the zend-code library is now set to be deprecated and removed, Magento should look towards replacing this altogether with another library.

https://github.com/zendframework/zend-code/issues/91

[adragus-inviqa]

M2 took "ownership" of ZF code, but I agree that the package should be replaced. But that's even more refactoring.
Maybe use another strategy for PHP 7+ (\Magento\Framework\Code\Generator\CodeGeneratorInterface) and keep the current one for older versions.

[tkacheva]

internal issue MAGETWO-59661

[mcjwsk]

@tkacheva any updates on this issue ?

[orlangur]

Create a new block and add a method with a return type in PHP7 format e.g : string

Interesting case :) Never thought of writing PHP5-incompatible code within Magento 2.

@AydinHassan

nor is it easy to update the version of zendframework/zend-code due to conflicting dependencies

https://github.com/zendframework/zend-code/blob/release-3.1.0/composer.json#L17 which dependencies?

@mwgamble

As the scanner code in the zend-code library is now set to be deprecated and removed, Magento should look towards replacing this altogether with another library

Why _another_ library if it is BC break anyway? Why not simply stick to zend-code 3.x?

At first glance https://github.com/zendframework/zend-code/blob/master/doc/book/migration.md does not seem to be really painful.

If you generate code that should run in PHP 5.x, it is advisable to ...

And this sentence looks exactly like the our case: we can use fancy new PHP7 features while all generated code will remain PHP5-compatible.

[mwgamble]

Why another library if it is BC break anyway? Why not simply stick to zend-code 3.x?

Because zend-code 3.x will never have support for scanning code that uses scalar type declarations.

[orlangur]

@mwgamble aren't they supported since version 3.0.0? https://github.com/zendframework/zend-code/blob/master/CHANGELOG.md#300---2016-01-13

[mwgamble]

@orlangur nope. see here: https://github.com/zendframework/zend-code/issues/56

Also see the responses to my comments here: https://github.com/zendframework/zend-code/pull/87#issuecomment-241127637
And here: https://github.com/zendframework/zend-code/issues/85

[orlangur]

@mwgamble, thanks for the details, I'll look closer into it. If scanner will not be adopted in 3.x and then deprecated-removed in 4.x-5.x it's a problem :)

So, it's better switch to BetterReflection library for us?

[AydinHassan]

@orlangur with zend-code requirement set to ^3:

  Problem 1
    - zendframework/zend-di 2.4.9 requires zendframework/zend-code 2.4.9 -> satisfiable by zendframework/zend-code[2.4.9] but these conflict with your requirements or minimum-stability.
    - zendframework/zend-di 2.4.8 requires zendframework/zend-code 2.4.8 -> satisfiable by zendframework/zend-code[2.4.8] but these conflict with your requirements or minimum-stability.
    - zendframework/zend-di 2.4.7 requires zendframework/zend-code 2.4.7 -> satisfiable by zendframework/zend-code[2.4.7] but these conflict with your requirements or minimum-stability.
    - zendframework/zend-di 2.4.6 requires zendframework/zend-code 2.4.6 -> satisfiable by zendframework/zend-code[2.4.6] but these conflict with your requirements or minimum-stability.
    - zendframework/zend-di 2.4.11 requires zendframework/zend-code 2.4.11 -> satisfiable by zendframework/zend-code[2.4.11] but these conflict with your requirements or minimum-stability.
    - zendframework/zend-di 2.4.10 requires zendframework/zend-code 2.4.10 -> satisfiable by zendframework/zend-code[2.4.10] but these conflict with your requirements or minimum-stability.
    - zendframework/zend-di 2.4.10 requires zendframework/zend-code 2.4.10 -> satisfiable by zendframework/zend-code[2.4.10] but these conflict with your requirements or minimum-stability.
    - Installation request for zendframework/zend-di ~2.4.6 -> satisfiable by zendframework/zend-di[2.4.10, 2.4.11, 2.4.6, 2.4.7, 2.4.8, 2.4.9].

On latest develop btw.

[orlangur]

@AydinHassan, yeah, I tried it as well, zend-code v3 is not stable yet: https://github.com/zendframework/zend-code/blob/master/composer.json#L30

And as it requires zendframework/zend-eventmanager 2.6 or higher current 2.4 ZF2 components will need to be updated as well.

These changes does not seem to be painful.

[AydinHassan]

@orlangur I'm not sure that means it's unstable - it's tagged as a stable release and uses stable dependencies. If you look at the blame for those lines - it's from 2+ years ago. In any case Composer only uses that info from the root package - It looks like that could have been added for local development of the package.

Regarding the zend-eventmanager package upgrade - it requires literally all of the zend-* packages to be updated. Not sure on Magento's policy of upgrading dependencies - but based on how old the 2.4 components being used are (18+ months) - sounds pretty painful to me 😄

[AydinHassan]

Minimal set of changes to pull in zend-code 3.x was, for me:

diff --git a/composer.json b/composer.json
index 6f5db49..2ec5b81 100644
--- a/composer.json
+++ b/composer.json
@@ -9,31 +9,31 @@
     ],
     "require": {
         "php": "~5.6.5|7.0.2|7.0.4|~7.0.6",
-        "zendframework/zend-stdlib": "~2.4.6",
-        "zendframework/zend-code": "~2.4.6",
-        "zendframework/zend-server": "~2.4.6",
-        "zendframework/zend-soap": "~2.4.6",
-        "zendframework/zend-uri": "~2.4.6",
-        "zendframework/zend-validator": "~2.4.6",
-        "zendframework/zend-crypt": "~2.4.6",
-        "zendframework/zend-console": "~2.4.6",
-        "zendframework/zend-modulemanager": "~2.4.6",
-        "zendframework/zend-mvc": "~2.4.6",
-        "zendframework/zend-text": "~2.4.6",
-        "zendframework/zend-i18n": "~2.4.6",
-        "zendframework/zend-eventmanager": "~2.4.6",
-        "zendframework/zend-view": "~2.4.6",
-        "zendframework/zend-servicemanager": "~2.4.6",
-        "zendframework/zend-json": "~2.4.6",
-        "zendframework/zend-config": "~2.4.6",
-        "zendframework/zend-form": "~2.4.6",
-        "zendframework/zend-di": "~2.4.6",
-        "zendframework/zend-serializer": "~2.4.6",
-        "zendframework/zend-log": "~2.4.6",
-        "zendframework/zend-http": "~2.4.6",
-        "zendframework/zend-db": "~2.4.6",
-        "zendframework/zend-captcha": "~2.4.6",
-        "zendframework/zend-session": "~2.4.6",
+        "zendframework/zend-stdlib": "^3",
+        "zendframework/zend-code": "^3",
+        "zendframework/zend-server": "^2.6",
+        "zendframework/zend-soap": "^2.6",
+        "zendframework/zend-uri": "^2.5",
+        "zendframework/zend-validator": "^2.6",
+        "zendframework/zend-crypt": "^2.6",
+        "zendframework/zend-console": "^2.6",
+        "zendframework/zend-modulemanager": "^2.7",
+        "zendframework/zend-mvc": "^2.7",
+        "zendframework/zend-text": "^2.6",
+        "zendframework/zend-i18n": "^2.7",
+        "zendframework/zend-eventmanager": "^3",
+        "zendframework/zend-view": "^2.8",
+        "zendframework/zend-servicemanager": "^2.7",
+        "zendframework/zend-json": "^2.6",
+        "zendframework/zend-config": "^2.6",
+        "zendframework/zend-form": "^2.10",
+        "zendframework/zend-di": "^2.6",
+        "zendframework/zend-serializer": "^2.7",
+        "zendframework/zend-log": "^2.9",
+        "zendframework/zend-http": "^2.6",
+        "zendframework/zend-db": "^2.8",
+        "zendframework/zend-captcha": "^2.7",
+        "zendframework/zend-session": "^2.7",
         "magento/zendframework1": "~1.12.16",
         "colinmollenhour/credis": "1.6",
         "colinmollenhour/php-redis-session-abstract": "1.2",

Maybe we don't need to take all of the deps to the latest 2.x releases - but assuming there is a strict semver policy on those packages - it should be fine. I didn't run the unit tests to see if anything actually breaks yet tho.

[orlangur]

Thanks! I was bored and did not come to an installable set of packages.

"zendframework/zend-stdlib": "^3"
"zendframework/zend-eventmanager": "^3",

Unavoidable?

https://github.com/zendframework/zend-stdlib/blob/master/doc/book/migration.md CallbackHandler is used in Setup which does not have separate composer.json currently unfortunately.

[AydinHassan]

Seems it is avoidable:

^2.6 for zendframework/zend-eventmanager and ^2.7 for zendframework/zend-stdlib works!

[orlangur]

That sounds way better) Thanks for your efforts.

[mwgamble]

I highly recommend Nikita Popov's PHP Parser library: https://github.com/nikic/PHP-Parser

It has full support for everything Magento 2 needs, and is actively maintained by one of the core developers of the PHP language itself.

[orlangur]

Yeah, I really enjoyed using it for various code analysis, there is no surprise BetterReflection is built on top of it: https://github.com/Roave/BetterReflection/blob/master/composer.json#L7

You think it is an overkill and it's better to have own simple implementation on top of PHP-Parser instead of using reflection library?

[epson121]

Issue in progress @epson121

[joni-jones]

https://github.com/magento/magento2/pull/8938

[joni-jones]

The fix merged into develop branch.

[craigcarnell]

@joni-jones any chance 7.1 support will be backported to 2.1?

[joni-jones]

Hi, @craigcarnell, unfortunately, I can't provide any information about backporting this functionality to 2.1.x version. At this moment this functionality will be only in 2.2.

[sablesoft]

I use Magento v2.3.2
I need to enable MAGE_PROFILER=2, but got errors like this:

Fatal error: Declaration of Magento\Framework\App\Request\PathInfo\Logger::getPathInfo(string $requestUri, string $baseUrl) must be compatible with Magento\Framework\App\Request\PathInfo::getPathInfo(string $requestUri, string $baseUrl): string in /var/www/magento/generated/code/Magento/Framework/App/Request/PathInfo/Logger.php on line 83

It looks like the same problem but with loggers generation in this time.
How can I fix it?

[multipasko]

the same problem as above

[dchaykas]

Magento 2.3.3

Fatal error: Declaration of Magento\Framework\App\Request\PathInfo\Logger::getPathInfo(string $requestUri, string $baseUrl) must be compatible with Magento\Framework\App\Request\PathInfo::getPathInfo(string $requestUri, string $baseUrl): string in /srv/www/alberta_dev/magento/generated/code/Magento/Framework/App/Request/PathInfo/Logger.php on line 0

As soon as SetEnv MAGE_PROFILER 2 is enabled in .htaccess.