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 code generation for never return type #735

Open
wants to merge 7 commits into
base: 2.15.x
Choose a base branch
from

Conversation

alekitto
Copy link

Fixes #729

Based upon #731 to execute pipeline on PHP 8.1.
I've fixed the deprecations raised by implementing \Serializable without the new __serialize/__unserialize methods, but I needed to disable deprecation report on lazy-loading-value-holder-internal-php-classes.phpt due to #[\ReturnTypeWillChange]-related errors.

@Ocramius Ocramius self-requested a review February 22, 2022 00:28
@Ocramius Ocramius added this to the 2.14.0 milestone Feb 22, 2022
@Ocramius Ocramius self-assigned this Feb 22, 2022
Comment on lines +1530 to +1532
if (PHP_VERSION_ID < 80100) {
self::markTestSkipped('Needs PHP 8.1');
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Can be replace by annotation:

/** @requires PHP >= 8.1 */

Copy link
Author

Choose a reason for hiding this comment

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

Similar checks have been used in other tests, but yes could be replaced easily if needed.

@Ocramius Ocramius modified the milestones: 2.14.0, 2.15.0 Feb 28, 2022
@Ocramius Ocramius changed the base branch from 2.14.x to 2.15.x February 28, 2022 13:59

if ($originalMethod->returnsReference()) {
if ($originalReturnType instanceof ReflectionNamedType && $originalReturnType->getName() === 'never') {
$method->setBody('throw new \Exception();');
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this really needed? without anything in the body, php already throws:
TypeError: [...]: never-returning function must not implicitly return

Copy link
Author

Choose a reason for hiding this comment

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

Mh, probably is not needed. Don't rembember why I added it honestly, I'll remove that

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.

Verify code generation for the new never type
4 participants