Re: [PHP-DEV] [VOTE] Ensure correct signatures of magic methods

2020-06-19 Thread Gabriel Caruso
On Fri, 29 May 2020 at 18:44, Gabriel Caruso 
wrote:

> Hello, internals!
>
> I have opened the voting for
> https://wiki.php.net/rfc/magic-methods-signature.
>
> The voting period ends on 2020-06-19 at 18h (CEST).
>
> Thanks,
>
> -- Gabriel Caruso
>

The RFC has been accepted with 45 votes in favor of it and 2 against.

The Pull Request on GitHub with its implementation will be updated during
the next days. Once it's merged into the main branch, I'll update this
thread.

Thanks everyone for the discussion!


Re: [PHP-DEV] [VOTE] Ensure correct signatures of magic methods

2020-06-17 Thread Nikita Popov
On Wed, Jun 3, 2020 at 11:11 PM Gabriel Caruso 
wrote:

> On Wed, 3 Jun 2020 at 12:32, Nikita Popov  wrote:
>
>> On Sun, May 31, 2020 at 11:20 PM Gabriel Caruso <
>> carusogabrie...@gmail.com> wrote:
>>
>>> On Sun, 31 May 2020 at 15:57, Nikita Popov  wrote:
>>>
 On Fri, May 29, 2020 at 6:45 PM Gabriel Caruso <
 carusogabrie...@gmail.com> wrote:

> Hello, internals!
>
> I have opened the voting for
> https://wiki.php.net/rfc/magic-methods-signature.
>
> The voting period ends on 2020-06-19 at 18h (CEST).
>

 The RFC is a bit unclear on what is actually being proposed. It says

 > This RFC proposes to add parameter and return types checks per the
 following details.

 and goes on to list (reasonable looking) magic method signatures, but
 does not say how exactly those types are going to be checked. Is this going
 to require exactly the same signature, or is this going to be in accordance
 with variance rules? For example, are all of the following signatures valid
 under this RFC? Only the first two? None of them?

 // Narrowed return type from ?array
 public function __debugInfo(): array {}

 // Narrowed return type from mixed
 public function __get(string $name): int {]

 // Widened argument type from string
 public function __get(string|array $name): mixed {}

>>>
>>>
>>> They are going to be checked following the variance rules, not the
>>> *exactly* same as the RFC. I'll mention this, thanks for point it out.
>>>
>>> Assuming this, your examples:
>>>
>>> 1 and 2. Will be valid, following the rules introduced by the `mixed`
>>> RFC.
>>>
>>> 3. Is that allowed in PHP? If so, the RFC will compliance with that.
>>>
>>
>> Yes, it is allowed. It makes little sense in this particular case, but
>> it's allowed.
>>
>
> Ok, so let's allow that as well. I'll cover that with tests.
>
>
>>
>> Also, is omitting the return type still permitted, even though it would
 nominally violate variance?

 public function __debugInfo() {}

>>>
>>> Yes, this hasn't changed. The RFC only affects *typed* methods.
>>>
>>
 Finally, if omitting the return type is permitted, will an implicit
 return type be added, like we do for __toString()? Would the method
 automatically become

 public function __debugInfo(): ?array {}

>>>
>>> An implicit return type won't be added for any of the magic methods. I
>>> believe that's a huge BC, and I don't want to debate that for PHP 8 (maybe
>>> PHP 9, yes).
>>>
>>
>> Why would this be a BC break? To make sure we're on the same page, I'm
>> suggesting to do the same as we do for __toString(), where if you declare
>>
>> public function __toString() {}
>>
>> we automatically convert it into
>>
>> public function __toString(): string {}
>>
>> internally.
>>
>
>> We could do the same for all other magic methods, and I don't think it
>> would introduce a particularly severe BC break.
>>
>> We did this for __toString() to work with the Stringable interface, and
>> we don't have the same requirement for other magic methods, but I still
>> think it's worth considering this for consistency reasons.
>>
>
> Ok, let me see if I understood it: so if someone creates a
>
> public function __set($name, $value) {}
>
> we would automatically convert (as per this RFC) to
>
> public function(string $name, mixed $value): void {}
>
> internally, right? Isn't this a BC if someone is returning something
> inside that method?
>

Yes, that's right, there would be a BC break. I ran some analysis for the
void cases (excluding constructor/destructor) with the following results on
top 2k packages:
https://gist.github.com/nikic/9bc4f025a85c322a14c21128d8202c64 There were
91 issues found. I have no easy way to analyze the non-void cases.

The BC issue is also being discussed in the constructor thread.

So yes, just adding the type automatically may not be possible :(

Or no, are you talking that we only convert that for Reflection purpose?
>

No, that would make Reflection lie. We don't want Reflection to lie :)

Regards,
Nikita


Re: [PHP-DEV] [VOTE] Ensure correct signatures of magic methods

2020-06-07 Thread Gabriel Caruso
On Wed, 3 Jun 2020 at 23:11, Gabriel Caruso 
wrote:

> On Wed, 3 Jun 2020 at 12:32, Nikita Popov  wrote:
>
>> On Sun, May 31, 2020 at 11:20 PM Gabriel Caruso <
>> carusogabrie...@gmail.com> wrote:
>>
>>> On Sun, 31 May 2020 at 15:57, Nikita Popov  wrote:
>>>
 On Fri, May 29, 2020 at 6:45 PM Gabriel Caruso <
 carusogabrie...@gmail.com> wrote:

> Hello, internals!
>
> I have opened the voting for
> https://wiki.php.net/rfc/magic-methods-signature.
>
> The voting period ends on 2020-06-19 at 18h (CEST).
>

 The RFC is a bit unclear on what is actually being proposed. It says

 > This RFC proposes to add parameter and return types checks per the
 following details.

 and goes on to list (reasonable looking) magic method signatures, but
 does not say how exactly those types are going to be checked. Is this going
 to require exactly the same signature, or is this going to be in accordance
 with variance rules? For example, are all of the following signatures valid
 under this RFC? Only the first two? None of them?

 // Narrowed return type from ?array
 public function __debugInfo(): array {}

 // Narrowed return type from mixed
 public function __get(string $name): int {]

 // Widened argument type from string
 public function __get(string|array $name): mixed {}

>>>
>>>
>>> They are going to be checked following the variance rules, not the
>>> *exactly* same as the RFC. I'll mention this, thanks for point it out.
>>>
>>> Assuming this, your examples:
>>>
>>> 1 and 2. Will be valid, following the rules introduced by the `mixed`
>>> RFC.
>>>
>>> 3. Is that allowed in PHP? If so, the RFC will compliance with that.
>>>
>>
>> Yes, it is allowed. It makes little sense in this particular case, but
>> it's allowed.
>>
>
> Ok, so let's allow that as well. I'll cover that with tests.
>
>
>>
>> Also, is omitting the return type still permitted, even though it would
 nominally violate variance?

 public function __debugInfo() {}

>>>
>>> Yes, this hasn't changed. The RFC only affects *typed* methods.
>>>
>>
 Finally, if omitting the return type is permitted, will an implicit
 return type be added, like we do for __toString()? Would the method
 automatically become

 public function __debugInfo(): ?array {}

>>>
>>> An implicit return type won't be added for any of the magic methods. I
>>> believe that's a huge BC, and I don't want to debate that for PHP 8 (maybe
>>> PHP 9, yes).
>>>
>>
>> Why would this be a BC break? To make sure we're on the same page, I'm
>> suggesting to do the same as we do for __toString(), where if you declare
>>
>> public function __toString() {}
>>
>> we automatically convert it into
>>
>> public function __toString(): string {}
>>
>> internally.
>>
>
>> We could do the same for all other magic methods, and I don't think it
>> would introduce a particularly severe BC break.
>>
>> We did this for __toString() to work with the Stringable interface, and
>> we don't have the same requirement for other magic methods, but I still
>> think it's worth considering this for consistency reasons.
>>
>
> Ok, let me see if I understood it: so if someone creates a
>
> public function __set($name, $value) {}
>
> we would automatically convert (as per this RFC) to
>
> public function(string $name, mixed $value): void {}
>
> internally, right? Isn't this a BC if someone is returning something
> inside that method?
>
> Or no, are you talking that we only convert that for Reflection purpose?
>
>>
>> Nikita
>>
>

To exemplify my question: is this change that you are suggesting for us to
add: https://3v4l.org/JKj9f vs. https://3v4l.org/JKj9f/rfc#git-php-master?


Re: [PHP-DEV] [VOTE] Ensure correct signatures of magic methods

2020-06-03 Thread Gabriel Caruso
On Wed, 3 Jun 2020 at 12:32, Nikita Popov  wrote:

> On Sun, May 31, 2020 at 11:20 PM Gabriel Caruso 
> wrote:
>
>> On Sun, 31 May 2020 at 15:57, Nikita Popov  wrote:
>>
>>> On Fri, May 29, 2020 at 6:45 PM Gabriel Caruso <
>>> carusogabrie...@gmail.com> wrote:
>>>
 Hello, internals!

 I have opened the voting for
 https://wiki.php.net/rfc/magic-methods-signature.

 The voting period ends on 2020-06-19 at 18h (CEST).

>>>
>>> The RFC is a bit unclear on what is actually being proposed. It says
>>>
>>> > This RFC proposes to add parameter and return types checks per the
>>> following details.
>>>
>>> and goes on to list (reasonable looking) magic method signatures, but
>>> does not say how exactly those types are going to be checked. Is this going
>>> to require exactly the same signature, or is this going to be in accordance
>>> with variance rules? For example, are all of the following signatures valid
>>> under this RFC? Only the first two? None of them?
>>>
>>> // Narrowed return type from ?array
>>> public function __debugInfo(): array {}
>>>
>>> // Narrowed return type from mixed
>>> public function __get(string $name): int {]
>>>
>>> // Widened argument type from string
>>> public function __get(string|array $name): mixed {}
>>>
>>
>>
>> They are going to be checked following the variance rules, not the
>> *exactly* same as the RFC. I'll mention this, thanks for point it out.
>>
>> Assuming this, your examples:
>>
>> 1 and 2. Will be valid, following the rules introduced by the `mixed` RFC.
>>
>> 3. Is that allowed in PHP? If so, the RFC will compliance with that.
>>
>
> Yes, it is allowed. It makes little sense in this particular case, but
> it's allowed.
>

Ok, so let's allow that as well. I'll cover that with tests.


>
> Also, is omitting the return type still permitted, even though it would
>>> nominally violate variance?
>>>
>>> public function __debugInfo() {}
>>>
>>
>> Yes, this hasn't changed. The RFC only affects *typed* methods.
>>
>
>>> Finally, if omitting the return type is permitted, will an implicit
>>> return type be added, like we do for __toString()? Would the method
>>> automatically become
>>>
>>> public function __debugInfo(): ?array {}
>>>
>>
>> An implicit return type won't be added for any of the magic methods. I
>> believe that's a huge BC, and I don't want to debate that for PHP 8 (maybe
>> PHP 9, yes).
>>
>
> Why would this be a BC break? To make sure we're on the same page, I'm
> suggesting to do the same as we do for __toString(), where if you declare
>
> public function __toString() {}
>
> we automatically convert it into
>
> public function __toString(): string {}
>
> internally.
>

> We could do the same for all other magic methods, and I don't think it
> would introduce a particularly severe BC break.
>
> We did this for __toString() to work with the Stringable interface, and we
> don't have the same requirement for other magic methods, but I still think
> it's worth considering this for consistency reasons.
>

Ok, let me see if I understood it: so if someone creates a

public function __set($name, $value) {}

we would automatically convert (as per this RFC) to

public function(string $name, mixed $value): void {}

internally, right? Isn't this a BC if someone is returning something inside
that method?

Or no, are you talking that we only convert that for Reflection purpose?

>
> Nikita
>


Re: [PHP-DEV] [VOTE] Ensure correct signatures of magic methods

2020-06-03 Thread Gabriel Caruso
>
> On Mon, Jun 1, 2020 at 12:20 AM Gabriel Caruso 
> wrote:
>
>> On Sun, 31 May 2020 at 15:57, Nikita Popov  wrote:
>>
>> > On Fri, May 29, 2020 at 6:45 PM Gabriel Caruso <
>> carusogabrie...@gmail.com>
>> > wrote:
>> >
>> >> Hello, internals!
>> >>
>> >> I have opened the voting for
>> >> https://wiki.php.net/rfc/magic-methods-signature.
>> >>
>> >> The voting period ends on 2020-06-19 at 18h (CEST).
>> >>
>> >
>> > The RFC is a bit unclear on what is actually being proposed. It says
>> >
>> > > This RFC proposes to add parameter and return types checks per the
>> > following details.
>> >
>> > and goes on to list (reasonable looking) magic method signatures, but
>> does
>> > not say how exactly those types are going to be checked. Is this going
>> to
>> > require exactly the same signature, or is this going to be in accordance
>> > with variance rules? For example, are all of the following signatures
>> valid
>> > under this RFC? Only the first two? None of them?
>> >
>> > // Narrowed return type from ?array
>> > public function __debugInfo(): array {}
>> >
>> > // Narrowed return type from mixed
>> > public function __get(string $name): int {]
>> >
>> > // Widened argument type from string
>> > public function __get(string|array $name): mixed {}
>> >
>>
>>
>> They are going to be checked following the variance rules, not the
>> *exactly* same as the RFC. I'll mention this, thanks for point it out.
>>
>> Assuming this, your examples:
>>
>> 1 and 2. Will be valid, following the rules introduced by the `mixed` RFC.
>>
>> 3. Is that allowed in PHP? If so, the RFC will compliance with that.
>>
>>
>> >
>> > Also, is omitting the return type still permitted, even though it would
>> > nominally violate variance?
>> >
>> > public function __debugInfo() {}
>> >
>>
>> Yes, this hasn't changed. The RFC only affects *typed* methods.
>>
>>
>> >
>> > Finally, if omitting the return type is permitted, will an implicit
>> return
>> > type be added, like we do for __toString()? Would the method
>> automatically
>> > become
>> >
>> > public function __debugInfo(): ?array {}
>> >
>>
>> An implicit return type won't be added for any of the magic methods. I
>> believe that's a huge BC, and I don't want to debate that for PHP 8 (maybe
>> PHP 9, yes).
>>
>>
>> >
>> > and report as such from reflection?
>> >
>>
>> I need more clearance on this one: are you asking how magic methods are
>> reported via Reflection and if that will be changed?
>>
>>
>> >
>> > Nikita
>> >
>>
> On Tue, 2 Jun 2020 at 14:26, Dmitry Stogov  wrote:

> Does this RFC support __get() returning by reference?
> See
> https://github.com/php/php-src/blob/master/Zend/tests/overloaded_prop_assign_op_refs.phpt#L11
>
> Thanks. Dmitry.

Hello Dmitry,

Yes, I'm not changing any behavior of magic methods and references.

-- Gabriel Caruso


Re: [PHP-DEV] [VOTE] Ensure correct signatures of magic methods

2020-06-03 Thread Nikita Popov
On Sun, May 31, 2020 at 11:20 PM Gabriel Caruso 
wrote:

> On Sun, 31 May 2020 at 15:57, Nikita Popov  wrote:
>
>> On Fri, May 29, 2020 at 6:45 PM Gabriel Caruso 
>> wrote:
>>
>>> Hello, internals!
>>>
>>> I have opened the voting for
>>> https://wiki.php.net/rfc/magic-methods-signature.
>>>
>>> The voting period ends on 2020-06-19 at 18h (CEST).
>>>
>>
>> The RFC is a bit unclear on what is actually being proposed. It says
>>
>> > This RFC proposes to add parameter and return types checks per the
>> following details.
>>
>> and goes on to list (reasonable looking) magic method signatures, but
>> does not say how exactly those types are going to be checked. Is this going
>> to require exactly the same signature, or is this going to be in accordance
>> with variance rules? For example, are all of the following signatures valid
>> under this RFC? Only the first two? None of them?
>>
>> // Narrowed return type from ?array
>> public function __debugInfo(): array {}
>>
>> // Narrowed return type from mixed
>> public function __get(string $name): int {]
>>
>> // Widened argument type from string
>> public function __get(string|array $name): mixed {}
>>
>
>
> They are going to be checked following the variance rules, not the
> *exactly* same as the RFC. I'll mention this, thanks for point it out.
>
> Assuming this, your examples:
>
> 1 and 2. Will be valid, following the rules introduced by the `mixed` RFC.
>
> 3. Is that allowed in PHP? If so, the RFC will compliance with that.
>

Yes, it is allowed. It makes little sense in this particular case, but it's
allowed.

Also, is omitting the return type still permitted, even though it would
>> nominally violate variance?
>>
>> public function __debugInfo() {}
>>
>
> Yes, this hasn't changed. The RFC only affects *typed* methods.
>

>> Finally, if omitting the return type is permitted, will an implicit
>> return type be added, like we do for __toString()? Would the method
>> automatically become
>>
>> public function __debugInfo(): ?array {}
>>
>
> An implicit return type won't be added for any of the magic methods. I
> believe that's a huge BC, and I don't want to debate that for PHP 8 (maybe
> PHP 9, yes).
>

Why would this be a BC break? To make sure we're on the same page, I'm
suggesting to do the same as we do for __toString(), where if you declare

public function __toString() {}

we automatically convert it into

public function __toString(): string {}

internally.

We could do the same for all other magic methods, and I don't think it
would introduce a particularly severe BC break.

We did this for __toString() to work with the Stringable interface, and we
don't have the same requirement for other magic methods, but I still think
it's worth considering this for consistency reasons.

Nikita


Re: [PHP-DEV] [VOTE] Ensure correct signatures of magic methods

2020-06-02 Thread Dmitry Stogov
Does this RFC support __get() returning by reference?
See
https://github.com/php/php-src/blob/master/Zend/tests/overloaded_prop_assign_op_refs.phpt#L11

Thanks. Dmitry.

On Mon, Jun 1, 2020 at 12:20 AM Gabriel Caruso 
wrote:

> On Sun, 31 May 2020 at 15:57, Nikita Popov  wrote:
>
> > On Fri, May 29, 2020 at 6:45 PM Gabriel Caruso <
> carusogabrie...@gmail.com>
> > wrote:
> >
> >> Hello, internals!
> >>
> >> I have opened the voting for
> >> https://wiki.php.net/rfc/magic-methods-signature.
> >>
> >> The voting period ends on 2020-06-19 at 18h (CEST).
> >>
> >
> > The RFC is a bit unclear on what is actually being proposed. It says
> >
> > > This RFC proposes to add parameter and return types checks per the
> > following details.
> >
> > and goes on to list (reasonable looking) magic method signatures, but
> does
> > not say how exactly those types are going to be checked. Is this going to
> > require exactly the same signature, or is this going to be in accordance
> > with variance rules? For example, are all of the following signatures
> valid
> > under this RFC? Only the first two? None of them?
> >
> > // Narrowed return type from ?array
> > public function __debugInfo(): array {}
> >
> > // Narrowed return type from mixed
> > public function __get(string $name): int {]
> >
> > // Widened argument type from string
> > public function __get(string|array $name): mixed {}
> >
>
>
> They are going to be checked following the variance rules, not the
> *exactly* same as the RFC. I'll mention this, thanks for point it out.
>
> Assuming this, your examples:
>
> 1 and 2. Will be valid, following the rules introduced by the `mixed` RFC.
>
> 3. Is that allowed in PHP? If so, the RFC will compliance with that.
>
>
> >
> > Also, is omitting the return type still permitted, even though it would
> > nominally violate variance?
> >
> > public function __debugInfo() {}
> >
>
> Yes, this hasn't changed. The RFC only affects *typed* methods.
>
>
> >
> > Finally, if omitting the return type is permitted, will an implicit
> return
> > type be added, like we do for __toString()? Would the method
> automatically
> > become
> >
> > public function __debugInfo(): ?array {}
> >
>
> An implicit return type won't be added for any of the magic methods. I
> believe that's a huge BC, and I don't want to debate that for PHP 8 (maybe
> PHP 9, yes).
>
>
> >
> > and report as such from reflection?
> >
>
> I need more clearance on this one: are you asking how magic methods are
> reported via Reflection and if that will be changed?
>
>
> >
> > Nikita
> >
>


Re: [PHP-DEV] [VOTE] Ensure correct signatures of magic methods

2020-05-31 Thread Gabriel Caruso
On Sun, 31 May 2020 at 15:57, Nikita Popov  wrote:

> On Fri, May 29, 2020 at 6:45 PM Gabriel Caruso 
> wrote:
>
>> Hello, internals!
>>
>> I have opened the voting for
>> https://wiki.php.net/rfc/magic-methods-signature.
>>
>> The voting period ends on 2020-06-19 at 18h (CEST).
>>
>
> The RFC is a bit unclear on what is actually being proposed. It says
>
> > This RFC proposes to add parameter and return types checks per the
> following details.
>
> and goes on to list (reasonable looking) magic method signatures, but does
> not say how exactly those types are going to be checked. Is this going to
> require exactly the same signature, or is this going to be in accordance
> with variance rules? For example, are all of the following signatures valid
> under this RFC? Only the first two? None of them?
>
> // Narrowed return type from ?array
> public function __debugInfo(): array {}
>
> // Narrowed return type from mixed
> public function __get(string $name): int {]
>
> // Widened argument type from string
> public function __get(string|array $name): mixed {}
>


They are going to be checked following the variance rules, not the
*exactly* same as the RFC. I'll mention this, thanks for point it out.

Assuming this, your examples:

1 and 2. Will be valid, following the rules introduced by the `mixed` RFC.

3. Is that allowed in PHP? If so, the RFC will compliance with that.


>
> Also, is omitting the return type still permitted, even though it would
> nominally violate variance?
>
> public function __debugInfo() {}
>

Yes, this hasn't changed. The RFC only affects *typed* methods.


>
> Finally, if omitting the return type is permitted, will an implicit return
> type be added, like we do for __toString()? Would the method automatically
> become
>
> public function __debugInfo(): ?array {}
>

An implicit return type won't be added for any of the magic methods. I
believe that's a huge BC, and I don't want to debate that for PHP 8 (maybe
PHP 9, yes).


>
> and report as such from reflection?
>

I need more clearance on this one: are you asking how magic methods are
reported via Reflection and if that will be changed?


>
> Nikita
>


Re: [PHP-DEV] [VOTE] Ensure correct signatures of magic methods

2020-05-31 Thread Nikita Popov
On Fri, May 29, 2020 at 6:45 PM Gabriel Caruso 
wrote:

> Hello, internals!
>
> I have opened the voting for
> https://wiki.php.net/rfc/magic-methods-signature.
>
> The voting period ends on 2020-06-19 at 18h (CEST).
>

The RFC is a bit unclear on what is actually being proposed. It says

> This RFC proposes to add parameter and return types checks per the
following details.

and goes on to list (reasonable looking) magic method signatures, but does
not say how exactly those types are going to be checked. Is this going to
require exactly the same signature, or is this going to be in accordance
with variance rules? For example, are all of the following signatures valid
under this RFC? Only the first two? None of them?

// Narrowed return type from ?array
public function __debugInfo(): array {}

// Narrowed return type from mixed
public function __get(string $name): int {]

// Widened argument type from string
public function __get(string|array $name): mixed {}

Also, is omitting the return type still permitted, even though it would
nominally violate variance?

public function __debugInfo() {}

Finally, if omitting the return type is permitted, will an implicit return
type be added, like we do for __toString()? Would the method automatically
become

public function __debugInfo(): ?array {}

and report as such from reflection?

Nikita


[PHP-DEV] [VOTE] Ensure correct signatures of magic methods

2020-05-29 Thread Gabriel Caruso
Hello, internals!

I have opened the voting for
https://wiki.php.net/rfc/magic-methods-signature.

The voting period ends on 2020-06-19 at 18h (CEST).

Thanks,

-- Gabriel Caruso