PHP7 return types incompatibile with generate code #6106

mikeymike · 2016-08-10T19:39:59Z

Preconditions

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

Steps to reproduce

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

Expected result

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...

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

The text was updated successfully, but these errors were encountered:

AydinHassan · 2016-08-11T11:15:53Z

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 · 2016-09-20T23:22:03Z

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.

zendframework/zend-code#91

adragus-inviqa · 2016-09-21T09:48:50Z

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 · 2016-10-13T10:39:43Z

internal issue MAGETWO-59661

mcjwsk · 2017-02-24T09:07:16Z

@tkacheva any updates on this issue ?

orlangur · 2017-02-24T09:50:32Z

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 · 2017-02-26T22:23:26Z

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 · 2017-02-26T23:45:43Z

@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 · 2017-02-27T04:52:25Z

@orlangur nope. see here: zendframework/zend-code#56

Also see the responses to my comments here: zendframework/zend-code#87 (comment)
And here: zendframework/zend-code#85

orlangur · 2017-02-27T07:15:17Z

@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 · 2017-02-27T09:23:51Z

@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 · 2017-02-27T09:32:41Z

@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 · 2017-02-27T09:39:17Z

@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 · 2017-02-27T09:57:19Z

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 · 2017-02-27T10:24:06Z

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 · 2017-02-27T10:28:15Z

Seems it is avoidable:

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

orlangur · 2017-02-27T10:39:49Z

That sounds way better) Thanks for your efforts.

mwgamble · 2017-02-27T22:42:41Z

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 · 2017-02-27T23:14:03Z

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 · 2017-03-16T15:07:00Z

Issue in progress @epson121

YevSent · 2017-03-20T07:25:23Z

#8938

… still not supported by magento magento/magento2#6106

… code #6106 - Updated Zend Code library to 3.x version and it dependencies - Refactored code generators to support PHP 7.x features - Refactored Setup module for new Zend Service Manager - Covered code generators by tests

… code #6106 - Updated reference blacklist - Added method description for event listener

… code #6106 - Reverted blacklist changes

… code #6106 - Resolved composer conflicts

… code #6106 - Resolved merge conflicts

YevSent · 2017-05-05T10:03:38Z

The fix merged into develop branch.

csdougliss · 2017-05-05T10:11:05Z

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

YevSent · 2017-05-05T12:05:52Z

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 · 2019-10-03T10:38:30Z

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 · 2019-12-18T11:40:22Z

the same problem as above

dchaykas · 2020-09-23T15:33:29Z

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.

wert2all added improvement TECH labels Aug 15, 2016

piotrekkaminski added bug report and removed improvement labels Aug 24, 2016

tkacheva added the Issue: Ready for Work Gate 4. Acknowledged. Issue is added to backlog and ready for development label Oct 13, 2016

ishakhsuvarov assigned epson121 Mar 16, 2017

jalogut added a commit to staempfli/magento2-module-image-resizer that referenced this issue Mar 20, 2017

width and height allow null types and remove return types as they are…

a08c759

… still not supported by magento magento/magento2#6106

YevSent mentioned this issue Apr 7, 2017

Checkout page dies to an exception if \Magento\Paypal\Model\Info is extended with plugins. \Magento\Backup\Model\Backup affected as well. #9167

Closed

magento-team pushed a commit that referenced this issue May 5, 2017

MAGETWO-59661: [Github] PHP7 returns types incompatible with generate…

502c956

… code #6106 - Updated reference blacklist - Added method description for event listener

magento-team pushed a commit that referenced this issue May 5, 2017

MAGETWO-59661: [Github] PHP7 returns types incompatible with generate…

431820d

… code #6106 - Reverted blacklist changes

magento-team pushed a commit that referenced this issue May 5, 2017

MAGETWO-59661: [Github] PHP7 returns types incompatible with generate…

6411eb5

… code #6106 - Resolved composer conflicts

magento-team pushed a commit that referenced this issue May 5, 2017

MAGETWO-59661: [Github] PHP7 returns types incompatible with generate…

3db5d79

… code #6106 - Resolved merge conflicts

YevSent closed this as completed May 5, 2017

orlangur marked this as a duplicate of #7620 Jul 26, 2017

orlangur mentioned this issue Jul 26, 2017

M2.1.2 : Interceptor breaks when type hinting a string #7620

Closed

This was referenced Jul 27, 2017

Remove PHP7 type hints from code SnowdogApps/magento2-menu#10

Closed

Does not compile SnowdogApps/magento2-menu#8

Closed

Ethan3600 mentioned this issue Jul 27, 2019

Error showing in commands Ethan3600/magento2-CronjobManager#128

Closed

stephan-cream mentioned this issue Aug 10, 2020

Return types incompatibile with generate code #29457

Closed

1 task

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

PHP7 return types incompatibile with generate code #6106

PHP7 return types incompatibile with generate code #6106

mikeymike commented Aug 10, 2016

AydinHassan commented Aug 11, 2016

mwgamble commented Sep 20, 2016

adragus-inviqa commented Sep 21, 2016

tkacheva commented Oct 13, 2016

mcjwsk commented Feb 24, 2017

orlangur commented Feb 24, 2017

mwgamble commented Feb 26, 2017

orlangur commented Feb 26, 2017

mwgamble commented Feb 27, 2017

orlangur commented Feb 27, 2017

AydinHassan commented Feb 27, 2017 •

edited

Loading

orlangur commented Feb 27, 2017

AydinHassan commented Feb 27, 2017

AydinHassan commented Feb 27, 2017

orlangur commented Feb 27, 2017

AydinHassan commented Feb 27, 2017

orlangur commented Feb 27, 2017

mwgamble commented Feb 27, 2017

orlangur commented Feb 27, 2017

epson121 commented Mar 16, 2017

YevSent commented Mar 20, 2017

YevSent commented May 5, 2017

csdougliss commented May 5, 2017

YevSent commented May 5, 2017

sablesoft commented Oct 3, 2019 •

edited

Loading

multipasko commented Dec 18, 2019

dchaykas commented Sep 23, 2020

PHP7 return types incompatibile with generate code #6106

PHP7 return types incompatibile with generate code #6106

Comments

mikeymike commented Aug 10, 2016

Preconditions

Steps to reproduce

Expected result

Actual result

AydinHassan commented Aug 11, 2016

mwgamble commented Sep 20, 2016

adragus-inviqa commented Sep 21, 2016

tkacheva commented Oct 13, 2016

mcjwsk commented Feb 24, 2017

orlangur commented Feb 24, 2017

mwgamble commented Feb 26, 2017

orlangur commented Feb 26, 2017

mwgamble commented Feb 27, 2017

orlangur commented Feb 27, 2017

AydinHassan commented Feb 27, 2017 • edited Loading

orlangur commented Feb 27, 2017

AydinHassan commented Feb 27, 2017

AydinHassan commented Feb 27, 2017

orlangur commented Feb 27, 2017

AydinHassan commented Feb 27, 2017

orlangur commented Feb 27, 2017

mwgamble commented Feb 27, 2017

orlangur commented Feb 27, 2017

epson121 commented Mar 16, 2017

YevSent commented Mar 20, 2017

YevSent commented May 5, 2017

csdougliss commented May 5, 2017

YevSent commented May 5, 2017

sablesoft commented Oct 3, 2019 • edited Loading

multipasko commented Dec 18, 2019

dchaykas commented Sep 23, 2020

AydinHassan commented Feb 27, 2017 •

edited

Loading

sablesoft commented Oct 3, 2019 •

edited

Loading