Re: [PHP-DEV] [RFC] Interface Default Methods
> On Thursday, Jun 15, 2023 at 12:01 PM, Deleu (mailto:deleu...@gmail.com)> wrote: > > One can argue that this change might make it so that users start > considering adding methods with default implementation as > not-so-much-a-bc-break and do so without bumping a major version, in which > case this RFC could be said to "open the door for some users to start > introducing BC-breaks without bumping major version" because they consider > it a much smaller BC break, but it can't be said to open the possibility. At this point, though, how different is the impact from this type of B/C break from the B/C break that already occurs when new methods are added to non-final classes where a subclass used a different signature and isn’t compatible with the new addition? - Michael
Re: [PHP-DEV] NULL Coercion Consistency
On Fri, May 27, 2022 at 9:36 PM Craig Francis
wrote:
> I know I keep going on about this very simply example, but it represents a
> fairly typical style of programming PHP, and I just do not understand what
> the problem with it is:
>
> ```
> $search = $request->input('q'); // Laravel, returns NULL when 'q' is not
> defined.
>
> echo 'Results for ' . htmlspecialchars($search);
> ```
If you want to go with this specific example, exactly as written, here is
the exact issue I have with it.
As already mentioned, `htmlspecialchars()` is documented and the
implementing code requires a string argument. Passing a null value to it
requires explicitly writing non-typesafe code, and requires understanding
of how the PHP language handles type coercion, and how those rules are
different based on whether the file calling that function has the
`strict_types` declaration. So on a code review, I have to understand the
context of the value passed through and know what context the language is
operating in to determine if non-string values are allowed to avoid errors,
and understand the semantics of PHP’s type coercion system to make sure the
output is something expected based on the input. Compare that to knowing
the function explicitly only accepts a string value and you know you are
working with a string through appropriate validation; for me, even in a
loosely typed environment, I’d rather follow the documented parameter types
instead of having to worry about all the other magic involved to make null
work.
Though, that input variable would never reach output in any of my projects
to begin with if it were null or an empty string; both would land on a code
path treated as no search being performed. So that example for me is also
way oversimplified because it doesn’t match a real world workflow IMO.
On Fri, May 27, 2022 at 9:36 PM Craig Francis
wrote:
> Sorry, but I'm not following... if there is a benefit/reason for PHP to
> reject NULL for `htmlspecialchars()`, and I'm just too stupid to see what
> it is, I would have assumed that benefit/reason would also apply to the
> HTML encoding function `e()` in Laravel.
No, that would not automatically apply to Laravel’s helper function, or any
escaping functions in Twig, or escaping functions used in platforms without
templating frameworks like WordPress. If it’s not OK for functions that
can call `htmlspecialchars()` to gracefully handle null to make sure those
trigger the same type error as the core language, then along the same
lines, I would argue that patches which add null coalescing or explicit
typecasting to that parameter in libraries or applications should also be
rejected because then they’re masking an error the language purposefully
elected to emit.
>
--
- Michael Please pardon any errors, this message was sent from my iPhone.
Re: [PHP-DEV] NULL Coercion Consistency
On Thursday, May 26, 2022 at 11:41 AM, Craig Francis mailto:cr...@craigfrancis.co.uk)> wrote: > On 26 May 2022, at 15:01, Michael Babker wrote: > > On Thu, May 26, 2022 at 7:21 AM Craig Francis > > wrote: > > > That said, I would still like to know about the benefits of rejecting > > > NULL for `htmlspecialchars()`, in the context of that Laravel Blade > > > patch; because, if it is beneficial (vs the BC break), they should revert > > > that patch... shouldn't they? > > > > No, they should not. Laravel made an explicit choice to handle null values > > in its escaping function for its templating framework. That is their choice > > to make. They should not be forced to implicitly handle null values because > > the language decides to add implicit type coercion to user defined > > functions (because consistency seems to be so important to some), and they > > should not be forced to reject null values because underlying language > > functions reject them as well. > > > Erm, I'm simply asking - If there is a good reason for throwing an exception > when NULL is passed to `htmlspecialchars()`, then that reason would also > apply to Laravel Blade for it's HTML encoding. Laravel’s `e()` helper function is more than a wrapper around `htmlspecialchars()` which supports values beyond a string, and has elected to support a null value being passed into that function with explicit handling for it. I’ve seen similar patches land in other packages as well. Whether it is to prevent that package from triggering the deprecation regarding null values itself or the package owners choosing to explicitly handle the null case so users don’t need to deal with it doesn’t really matter, the point is folks have made that decision and it is not problematic in any way IMO. You’re basically saying that a framework choosing to apply the same `htmlspecialchars($string ?? ‘’)` patch that applications are suggested to use (for those who don’t care to differentiate between null and empty string before something reaches an escaping function) is in the wrong by arguing for Laravel to revert that patch. On Thursday, May 26, 2022 at 11:41 AM, Craig Francis mailto:cr...@craigfrancis.co.uk)> wrote: > > If a function, by explicit design, can safely work with null values then > > that parameter should be properly declared nullable. > > > `htmlspecialchars()` can and always has safely worked with NULL, the same > with the other 335 parameters I've listed. But when I suggested changing > these parameters to be nullable, it was not well received. The arginfo for the `htmlspecialchars()` function, and if I’m reading correctly (I’m no C developer so I could very well be misinterpreting things) the internal `php_html_entities()` function which `htmlspecialchars()` calls, does not declare null as a supported value for the `$string` parameter. Meaning that depending on whether your code uses strict_types or not, either calling the function with a null value triggers a type error or requires implicit type coercision to convert the value to a type that the function does support. This means to me that the `htmlspecialchars()` function does NOT support null values and that passing null through only works because of the reliance on implicit type coercision. That, to me, is a valid reason for a type error to be raised; the code very explicitly requires a string and is not written to process a null value. A blanket suggestion to make every string parameter which implicitly supports a null value nullable because they previously supported null through type coercion IMO should be shot down. Instead, parameters which are emitting a deprecation notice because they are receiving an unsupported null value should be reviewed, and if that function can by design support null values, those parameters should be updated to reflect nullability. IMO, `htmlspecialchars()` is not a function which should expliciitly support a null value and the present deprecation is correct. On Thursday, May 26, 2022 at 11:41 AM, Craig Francis mailto:cr...@craigfrancis.co.uk)> wrote: > On 26 May 2022, at 15:01, Michael Babker wrote: > > Yes, that means that there is a lot of work involved for folks between now > > and the release of PHP 9.0 to either refactor code to avoid type errors or > > to get the language to expand the supported types in appropriate functions, > > but IMO that effort from all parties is worth it in the long run to ensure > > language consistency (I really don’t think userland PHP and > > internal/extension PHP should have vastly different behaviors, and type > > coercion support based on where a function is designed is one of those > > holes that needs filled) and to ensure all APIs explicitly declare what
Re: [PHP-DEV] NULL Coercion Consistency
On Thu, May 26, 2022 at 7:21 AM Craig Francis wrote: > That said, I would still like to know about the benefits of rejecting NULL > for `htmlspecialchars()`, in the context of that Laravel Blade patch; > because, if it is beneficial (vs the BC break), they should revert that > patch... shouldn't they? > No, they should not. Laravel made an explicit choice to handle null values in its escaping function for its templating framework. That is their choice to make. They should not be forced to implicitly handle null values because the language decides to add implicit type coercion to user defined functions (because consistency seems to be so important to some), and they should not be forced to reject null values because underlying language functions reject them as well. Every implicit type coercion is a bug in hiding IMO. If something doesn’t explicitly support null, you shouldn’t have been passing null to it in the first place. Relying on magic language behaviors to prevent type errors is not a long term sustainable code pattern. If a function, by explicit design, can safely work with null values then that parameter should be properly declared nullable. If a function, by explicit design, does not support null values then a type error is a hugely acceptable response. Yes, that means that there is a lot of work involved for folks between now and the release of PHP 9.0 to either refactor code to avoid type errors or to get the language to expand the supported types in appropriate functions, but IMO that effort from all parties is worth it in the long run to ensure language consistency (I really don’t think userland PHP and internal/extension PHP should have vastly different behaviors, and type coercion support based on where a function is designed is one of those holes that needs filled) and to ensure all APIs explicitly declare what types of data they support. -- - Michael Please pardon any errors, this message was sent from my iPhone.
Re: [PHP-DEV] Changing fundamental language behaviors
On Thu, Sep 12, 2019 at 2:17 PM Olumide Samson wrote: > Most of these changes wouldn't have been problematic to you if the > language has prevented you from writing what we can now consider bad code, > so please allow the new PHP developer that newly start using PHP to not > follow that your path that will/might hunt him later in the future... > > There a notices, warning and errors to inform you that this shouldn't have > been the use case of this feature and you chose to ignore it and now, we > are simplifying things and making those your errors teach you how to write > proper codes in the future, you're objecting.. Why not just stay in PHP 7.x? > > Or were you implying you want hitch-free, no-modification upgrade to PHP 8 > from PHP 7.0? > If yes, follow the best practices and not suppress error notices. > We're clearly talking past one another so I will be going back to work after this response. I am not saying anything about whether the warnings RFC should pass or fail, or if it makes my code good or bad. I responded explicitly to one idea about creating a LTS version that might somehow make it easier for RFCs like this one to be accepted because users could basically be encouraged to stay on the LTS version if the new major version introduces too many breaking changes, which I think is bad justification for creating a LTS version. I'm not sure how that equates to my code being good or bad or whether I am following someone else's recommended best practices, but that was never the point of discussion.
Re: [PHP-DEV] Changing fundamental language behaviors
On Thu, Sep 12, 2019 at 2:06 PM Olumide Samson wrote: > On Thu, Sep 12, 2019 at 8:00 PM Michael Babker > wrote: > >> On Thu, Sep 12, 2019 at 1:51 PM Peter Kokot wrote: >> >> > Just a dumb idea, since there clearly is a majority in favor of the >> > change with these warnings and strictness and all that now... Why not >> > making something like an LTS PHP 7.x where all the legacy code would >> > work OK as long as practically possible and 8.x+ would be the future >> > of what the developers want and not what business wants? One who won't >> > upgrade due to the BC breaks also won't need the new features anyway >> > very realistically. >> > >> >> Please don't tie the notion of LTS with the idea that a new major can >> break >> BC at will or create larger scale breaks because the previous major has >> extended support. Sooner or later that will end up back at the ++ idea >> and >> fragmentation encouraged by the language is a bad idea. >> > > Not sure you are really seeing the goal... > > Why is LTS not a good idea? > I'm not saying LTS is a bad idea. I'm saying using LTS to justify shipping larger scale BC breaks, such as the changes suggested in the last couple of "contentious" RFCs in a major version because "hey, we have a LTS version you can use that until you're ready to deal with the backlog of BC breaks created" is a bad idea. For the record, I happen to agree with as these RFCs would have minimal impact on my day-to-day work, but having also been in the role of a maintainer of open source libraries and applications I also grasp why these types of changes can be problematic to the ecosystem (both end users of those libraries and applications and the maintainers of them) and wouldn't jump the gun to ship them without careful consideration. >
Re: [PHP-DEV] Changing fundamental language behaviors
On Thu, Sep 12, 2019 at 1:51 PM Peter Kokot wrote: > Just a dumb idea, since there clearly is a majority in favor of the > change with these warnings and strictness and all that now... Why not > making something like an LTS PHP 7.x where all the legacy code would > work OK as long as practically possible and 8.x+ would be the future > of what the developers want and not what business wants? One who won't > upgrade due to the BC breaks also won't need the new features anyway > very realistically. > Please don't tie the notion of LTS with the idea that a new major can break BC at will or create larger scale breaks because the previous major has extended support. Sooner or later that will end up back at the ++ idea and fragmentation encouraged by the language is a bad idea.
