Skip to content

PHP 8.3 | Incorrect error when using static on parameter in extended class #12069

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

Closed
jrfnl opened this issue Aug 28, 2023 · 6 comments
Closed

Comments

@jrfnl
Copy link
Contributor

jrfnl commented Aug 28, 2023

Description

The following code:

<?php

class Foo {
    protected function doSomething(Reflection $a) {
        return true;
    }
}

class Bar extends Foo {
    protected function doSomething(static $b) {
        return true;
    }
}

$obj = new Bar;

PHP 8.0 - 8.2: https://3v4l.org/c5r5q
PHP 8.3: https://3v4l.org/c5r5q/rfc#vgit.master

Resulted in this output:

Fatal error: Cannot use the static modifier on a promoted property in /in/c5r5q on line 10

But I expected this output instead:

Parse error: syntax error, unexpected token "static", expecting variable in /in/c5r5q on line 10

As this method is not a class constructor, the reference to promoted properties is confusing.

Discovered by accident while creating a code sample to test something else completely.

PHP Version

PHP 8.3.0-dev

Operating System

No response

@bwoebi
Copy link
Member

bwoebi commented Aug 28, 2023

This is a side-effect of #9926.

Anyhow, the definition is simple: any keyword which may be used before a variable to declare it within class scope, is considered an attempt to declare a promoted property. And it's not possible to create a static promoted property, hence the error.

It might be possible to craft a special case error message here, but it won't be a syntax error.

@jrfnl
Copy link
Contributor Author

jrfnl commented Aug 28, 2023

@bwoebi I'd be fine with it not being a parse error, as long as the reference to promoted properties does not show for non-__construct() methods (as property promotion does not apply).

Something along the lines of The static type cannot be used with parameters maybe ?

@TimWolla
Copy link
Member

When replacing the static with public PHP 8.3 also reports:

Fatal error: Cannot declare promoted property outside a constructor in /in/AM49EF on line 10

Perhaps the order of the checks just needs to be reversed: https://3v4l.org/AM49EF/rfc#vgit.master?

@Girgias
Copy link
Member

Girgias commented Sep 6, 2023

When replacing the static with public PHP 8.3 also reports:

Fatal error: Cannot declare promoted property outside a constructor in /in/AM49EF on line 10

Perhaps the order of the checks just needs to be reversed: https://3v4l.org/AM49EF/rfc#vgit.master?

I don't think this is possible as it looks like the new error happens during lexing, while the

Cannot declare promoted property outside a constructor

Error happens while compiling a list of parameters together.

@iluuu1994
Copy link
Member

iluuu1994 commented Sep 11, 2023

We could just change the error message from "Cannot use the static modifier on a promoted property" to "Cannot use the static modifier on a parameter" in all cases. That should make it less confusing.

iluuu1994 added a commit to iluuu1994/php-src that referenced this issue Sep 11, 2023
The ZEND_MODIFIER_TARGET_CPP should really have been called _PARAM, but we
shouldn't break API at this point.

Closes phpGH-12069
@jrfnl
Copy link
Contributor Author

jrfnl commented Sep 11, 2023

@iluuu1994 Makes sense to me. Thanks for getting this sorted.

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

5 participants