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

FIX issue #33 - Returning void on method makes the whole class unable… #993

Conversation

phoenix128
Copy link
Contributor

FIx for issue magento-engcom/php-7.2-support#33

Description

Void methods were breaking Interceptor classes due to a return statement.
PHP7.2 forbids the presence of a return statement with parameters in void methods, even if returning null.

Fixed Issues (if relevant)

  1. Mni/source selection #33: Returning void on method makes the whole class unable to use plugins

Contribution checklist

  • Pull request has a meaningful description of its purpose
  • All commits are accompanied by meaningful commit messages
  • All new or changed code is covered with unit/integration tests (if applicable)
  • All automated tests passed successfully (all builds on Travis CI are green)

… unable to use plugins

Void methods were breaking Interceptor classes because of the presence of a return statement.
Return statements with any kind of value (even null) is forbiddent by PHP7.2 .
Interceptor generation clas has been modified to check the void return type and unit tests have been adjusted accordingly.
@maghamed
Copy link
Contributor

Corresponding PR to Core: magento/magento2#14799

Copy link
Member

@larsroettig larsroettig left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please fix the static test currently are red

@larsroettig larsroettig added this to the MSI Part I milestone Apr 22, 2018
@phoenix128
Copy link
Contributor Author

@larsroettig , cannot fix in the MSI branch due to code freeze. Check magento#14799 PR.

@vadimjustus vadimjustus dismissed larsroettig’s stale review April 27, 2018 16:20

The failing static tests are valid! Waiting for magento#14799

@naydav
Copy link

naydav commented May 7, 2018

Merged with
#1044

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

Successfully merging this pull request may close these issues.

4 participants