Re: [PHP-DEV] Warn when declaring required parameter after optional one
On Wed, Feb 5, 2020 at 1:15 PM Guilliam Xavier
wrote:
> Hi,
>
> On Wednesday, February 5, 2020, Nikita Popov wrote:
>>
>>
>> I like this idea and have updated the pull request to ignore the "Type
>> $param = null" case, so the deprecation should now just catch the
>> "definitely" incorrect signatures.
>>
>> So to summarize the current state:
>>
>> function test($foo = "bar", $baz) {}
>> // Deprecated: Required parameter $baz follows optional parameter $foo
>>
>> function test(Abc $foo = null, $baz) {}
>> // No warnings, works fine!
>>
>> With that adjustment made, are there any further concerns about this
>> change?
>
>
> Can you please just clarify which of the following will emit a deprecation?
>
> function f1($a = null, $b) {}
> function f2(A $a = null, $b) {}
> function f3(?A $a = null, $b) {}
>
> (I think f1 will, f2 won't, but f3?
>
f1 will, f2 won't, f3 also won't. f3 could technically emit one, but I'm
not sure it's worth having a special case of a special case for this.
Nikita
Re: [PHP-DEV] Warn when declaring required parameter after optional one
Hi,
On Wednesday, February 5, 2020, Nikita Popov wrote:
>
>
> I like this idea and have updated the pull request to ignore the "Type
> $param = null" case, so the deprecation should now just catch the
> "definitely" incorrect signatures.
>
> So to summarize the current state:
>
> function test($foo = "bar", $baz) {}
> // Deprecated: Required parameter $baz follows optional parameter $foo
>
> function test(Abc $foo = null, $baz) {}
> // No warnings, works fine!
>
> With that adjustment made, are there any further concerns about this
> change?
Can you please just clarify which of the following will emit a deprecation?
function f1($a = null, $b) {}
function f2(A $a = null, $b) {}
function f3(?A $a = null, $b) {}
(I think f1 will, f2 won't, but f3?)
Thanks
>
> Regards,
> Nikita
>
--
Guilliam Xavier
Re: [PHP-DEV] Warn when declaring required parameter after optional one
On Fri, Jan 17, 2020 at 11:59 AM Nikita Popov wrote:
>> I've created https://github.com/php/php-src/pull/5067 to make code like
>>function test($foo = null, $bar) {}
>> throw a warning
>
> I was interested in seeing how prevalent this pattern, is, so I ran
> some analysis on the top 2k composer packages. I found 527 signatures
> that would throw a deprecation warning with this change. Of these 187
> are potentially used as "poor man's nullable types" (the optional
> argument has both a type and a null default), while the other 340 are
> definite bugs.
Given that most of these usages are definite bugs, I'm in favor of
deprecating this in PHP 8 and making it a compile error in PHP 9. This
should provide plenty of time for codebases to migrate to the simpler
nullable types syntax for the minority of usages that aren't bugs.
Theodore
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Warn when declaring required parameter after optional one
Hi all, Whilst I totally see the benefits of this, I'm not sure if this is a job for PHP itself, rather a coding standard enforced optionally by something like PHPCS. It's certainly a bad practice, but I'm not sure the benefit will be worth the refactors required. I feel we might be an interesting example, however, it'd cause us a lot of pain for the following reason: We have a portion of our codebase (shared over hundreds of websites) which is symlinked into each of the websites so the core functionality can be easily updated when required on all sites. These sites are staggered at present across multiple PHP versions whilst being migrated (and due to the number of sites to migrate, they will be for some time). If this change were to come into effect, the same code we've safely had symlinked running across multiple PHP versions would suddenly become incompatible on the latest version, with no easy way to be suppressed which would mean we have to fork things and maintain two codebases. I appreciate this isn't likely to be a normal workflow for people, but it is an example of how it could cause unintended pain. Just my thoughts anyway. Cheers, Aran On Fri, 17 Jan 2020 at 18:00, Nikita Popov wrote: > On Sat, Jan 11, 2020 at 2:35 PM Niklas Keller wrote: > > > Hi Nikita, > > > > while this is a rather small change, it has quite some BC impact, as > > not all old code has been adjusted to run on PHP 7.1+ only using > > nullable types. > > > > I'd like to see an RFC with a vote for this. > > > > Regards, > > Niklas > > > > I was interested in seeing how prevalent this pattern, is, so I ran some > analysis on the top 2k composer packages. I found 527 signatures that would > throw a deprecation warning with this change. Of these 187 are potentially > used as "poor man's nullable types" (the optional argument has both a type > and a null default), while the other 340 are definite bugs. > > Regards, > Nikita >
Re: [PHP-DEV] Warn when declaring required parameter after optional one
On Sat, Jan 11, 2020 at 2:35 PM Niklas Keller wrote: > Hi Nikita, > > while this is a rather small change, it has quite some BC impact, as > not all old code has been adjusted to run on PHP 7.1+ only using > nullable types. > > I'd like to see an RFC with a vote for this. > > Regards, > Niklas > I was interested in seeing how prevalent this pattern, is, so I ran some analysis on the top 2k composer packages. I found 527 signatures that would throw a deprecation warning with this change. Of these 187 are potentially used as "poor man's nullable types" (the optional argument has both a type and a null default), while the other 340 are definite bugs. Regards, Nikita
Re: [PHP-DEV] Warn when declaring required parameter after optional one
Hi Nikita, while this is a rather small change, it has quite some BC impact, as not all old code has been adjusted to run on PHP 7.1+ only using nullable types. I'd like to see an RFC with a vote for this. Regards, Niklas -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Warn when declaring required parameter after optional one
On Thu, Jan 9, 2020 at 2:41 PM Bob Weinand wrote:
> Hey,
>
> While I generally agree that it likely is a bug in new code, I would
> rather deprecate it than warn or even error: the change would make it
> impossible to retain a type without warning while preserving compatibility
> with an old PHP version and making incremental migrations harder (would
> then not be possible to write warning-free code running on 7.0 and 8.0 at
> the same time while retaining type information).
>
> I would like to see it deprecated and then removed in 4+ years.
> Bob
>
As deprecation and later removal seem to be preferred by multiple people,
I've changed the PR to implementation that instead.
Nikita
> > Am 09.01.2020 um 13:27 schrieb Nikita Popov :
> >
> > Hi internals,
> >
> > I've created https://github.com/php/php-src/pull/5067 to make code like
> >
> >function test($foo = null, $bar) {}
> >
> > throw a warning:
> >
> >Warning: Required parameter $bar follows optional parameter
> >
> > Historically, having an "optional" parameter before a required one was
> > useful for poor man's nullable types. That is, one could write
> >
> >function test(FooBar $param = null, $param2)
> >
> > to get an effective
> >
> >function test(?FooBar $param, $param2)
> >
> > signature on old PHP versions that did not have native support for
> nullable
> > types.
> >
> > Since nullable types have been available since PHP 7.1, having a required
> > parameter after an optional one is increasingly likely a bug rather than
> an
> > intentional workaround, so I think it would be good to throw a warning
> for
> > this case.
> >
> > What do you think?
> >
> > Nikita
>
Re: [PHP-DEV] Warn when declaring required parameter after optional one
Am 09.01.2020 um 13:31 schrieb Sebastian Bergmann: I would prefer erroring out over just emitting a warning. What I meant, of course, was deprecation (or warning) first before erroring out. Sorry. -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Warn when declaring required parameter after optional one
Hey,
While I generally agree that it likely is a bug in new code, I would rather
deprecate it than warn or even error: the change would make it impossible to
retain a type without warning while preserving compatibility with an old PHP
version and making incremental migrations harder (would then not be possible to
write warning-free code running on 7.0 and 8.0 at the same time while retaining
type information).
I would like to see it deprecated and then removed in 4+ years.
Bob
> Am 09.01.2020 um 13:27 schrieb Nikita Popov :
>
> Hi internals,
>
> I've created https://github.com/php/php-src/pull/5067 to make code like
>
>function test($foo = null, $bar) {}
>
> throw a warning:
>
>Warning: Required parameter $bar follows optional parameter
>
> Historically, having an "optional" parameter before a required one was
> useful for poor man's nullable types. That is, one could write
>
>function test(FooBar $param = null, $param2)
>
> to get an effective
>
>function test(?FooBar $param, $param2)
>
> signature on old PHP versions that did not have native support for nullable
> types.
>
> Since nullable types have been available since PHP 7.1, having a required
> parameter after an optional one is increasingly likely a bug rather than an
> intentional workaround, so I think it would be good to throw a warning for
> this case.
>
> What do you think?
>
> Nikita
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Warn when declaring required parameter after optional one
Am 09.01.2020 um 13:26 schrieb Nikita Popov: What do you think? I would prefer erroring out over just emitting a warning. -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
[PHP-DEV] Warn when declaring required parameter after optional one
Hi internals,
I've created https://github.com/php/php-src/pull/5067 to make code like
function test($foo = null, $bar) {}
throw a warning:
Warning: Required parameter $bar follows optional parameter
Historically, having an "optional" parameter before a required one was
useful for poor man's nullable types. That is, one could write
function test(FooBar $param = null, $param2)
to get an effective
function test(?FooBar $param, $param2)
signature on old PHP versions that did not have native support for nullable
types.
Since nullable types have been available since PHP 7.1, having a required
parameter after an optional one is increasingly likely a bug rather than an
intentional workaround, so I think it would be good to throw a warning for
this case.
What do you think?
Nikita
