Welcome! Log In Create A New Profile

Advanced

[PHP-DEV] [RFC][Discussion] Make compact function reports undefined passed variables

Posted by Gabriel Caruso 
Hello dear internals, how are you?

I'd like to propose a new RFC to PHP's core, but as this one contains a BC
Break, let's discussed it before making anything official.

A couple of days ago, while discussing some [Coding Standards rules for
Doctrine, forbidden the `compact` function](
https://github.com/doctrine/coding-standard/pull/49), an argument caught my
attention:

> The `compact` function var does not report undefined variables.

Looking in [the `compact` documentation](https://secure.php.net/compact),
this is even emphasizes:

> Any strings that are not set will simply be skipped.

I couldn't figure out why this is done this way, but, here's what I'd like
to propose: make the `compact` function starts to report undefined passed
variables for it.

With [only 2 lines of code](
https://github.com/php/php-src/compare/master...carusogabriel:warning-unknow-compact-variable),
this is possible, but, of course, this is a BC Break.

Let me know your opinion on that, and perhaps, make it happen!

Regards,
--
Gabriel Caruso
----- Original Message -----
> From: "Gabriel Caruso" <[email protected]>
> To: "internals" <[email protected]>
> Sent: Monday, April 2, 2018 11:17:43 AM
> Subject: [PHP-DEV] [RFC][Discussion] Make compact function reports undefined passed variables

> Hello dear internals, how are you?
>
> I'd like to propose a new RFC to PHP's core, but as this one contains a BC
> Break, let's discussed it before making anything official.
>
> A couple of days ago, while discussing some [Coding Standards rules for
> Doctrine, forbidden the `compact` function](
> https://github.com/doctrine/coding-standard/pull/49), an argument caught my
> attention:
>
>> The `compact` function var does not report undefined variables.
>
> Looking in [the `compact` documentation](https://secure.php.net/compact),
> this is even emphasizes:
>
>> Any strings that are not set will simply be skipped.
>
> I couldn't figure out why this is done this way, but, here's what I'd like
> to propose: make the `compact` function starts to report undefined passed
> variables for it.
>
> With [only 2 lines of code](
> https://github.com/php/php-src/compare/master...carusogabriel:warning-unknow-compact-variable),
> this is possible, but, of course, this is a BC Break.
>
> Let me know your opinion on that, and perhaps, make it happen!
>
> Regards,
> --
> Gabriel Caruso

Hey Gabriel,

What I am missing here is the reason why we should break well defined behavior of existing functions.

How is the BC break for all existing code justified?

Pieter

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
>
> Hey Gabriel,
>
> What I am missing here is the reason why we should break well defined
> behavior of existing functions.
>
> How is the BC break for all existing code justified?
>
> Pieter
>


Hi Pieter.

> What I am missing here is the reason why we should break well defined
behavior of existing functions.

This is something I'd like to discuss about `compact`. I'd like to know why
we skip undefined variables instead of report them. `compact` is use in a
large scale by the MVC community, by passing variables in the controller
and using them in the view.

> How is the BC break for all existing code justified?

I'd like to start report this undefined variables. Would be one more way to
prevent code-smell. IMHO this belongs to the core, not to the userland.


Thanks,

--
Gabriel Caruso
> Hi Pieter.

>> What I am missing here is the reason why we should break well defined behavior
> > of existing functions.


> Hi Pieter.

>> What I am missing here is the reason why we should break well defined behavior
> > of existing functions.

> This is something I'd like to discuss about `compact`. I'd like to know why we
> skip undefined variables instead of report them. `compact` is use in a large
> scale by the MVC community, by passing variables in the controller and using
> them in the view.

> > How is the BC break for all existing code justified?

> I'd like to start report this undefined variables. Would be one more way to
> prevent code-smell. IMHO this belongs to the core, not to the userland.

> Thanks,

> --
> Gabriel Caruso

Regardless of the why it is like that the fact of the matter is that it is and imho
you would need a really good reason to break existing code.

I still don't don't see your reason for it. Nor do I see how it is a code smell atm.

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
Hello,

Yes, a thousand yes. Silently ignoring undefined variables is a source
of multiple bugs. Emiting a warning is a small BC break comparing to the
benefits.

Regards.

On 02.04.18 11:17, Gabriel Caruso wrote:
> Hello dear internals, how are you?
>
> I'd like to propose a new RFC to PHP's core, but as this one contains a BC
> Break, let's discussed it before making anything official.
>
> A couple of days ago, while discussing some [Coding Standards rules for
> Doctrine, forbidden the `compact` function](
> https://github.com/doctrine/coding-standard/pull/49), an argument caught my
> attention:
>
>> The `compact` function var does not report undefined variables.
> Looking in [the `compact` documentation](https://secure.php.net/compact),
> this is even emphasizes:
>
>> Any strings that are not set will simply be skipped.
> I couldn't figure out why this is done this way, but, here's what I'd like
> to propose: make the `compact` function starts to report undefined passed
> variables for it.
>
> With [only 2 lines of code](
> https://github.com/php/php-src/compare/master...carusogabriel:warning-unknow-compact-variable),
> this is possible, but, of course, this is a BC Break.
>
> Let me know your opinion on that, and perhaps, make it happen!
>
> Regards,

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
And the determined can prefix with an @ if they want similar to old
behaviour :)

Ivan Enderlin wrote:
> Hello,
>
> Yes, a thousand yes. Silently ignoring undefined variables is a source
> of multiple bugs. Emiting a warning is a small BC break comparing to the
> benefits.
>
> Regards.
>

--
Andrea Faulds
https://ajf.me/

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
And by that one could argue that the BC break is negligible :)
Anyway, I think this is a clear improvement!

Regards
--
//Björn Larsson


Den 2018-04-09 kl. 23:54, skrev Andrea Faulds:

> And the determined can prefix with an @ if they want similar to old
> behaviour :)
>
> Ivan Enderlin wrote:
>> Hello,
>>
>> Yes, a thousand yes. Silently ignoring undefined variables is a source
>> of multiple bugs. Emiting a warning is a small BC break comparing to the
>> benefits.
>>
>> Regards.
>>
>


--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
Sorry, only registered users may post in this forum.

Click here to login