Re: [PHP-DEV] List of attributes
On 28/10/2020 16:58, Nicolas Grekas wrote:
about why we'd need nested attributes, here is a discussion about
nested validation constraints:
https://github.com/symfony/symfony/issues/38503
Thanks, it's useful to see some real-world examples. As I suspected,
nearly all of these explicitly take a *list* of nested attributes, not a
single attribute.
> While most of the constraints receive the nested constraints as
simple array, |Collection| however requires a mapping (field to
constraint) and is usually combined with other composite constraints,
which gives us a second nesting level.
There is much discussion of Collection as an edge case, but although the
nested attributes are indeed inside a mapping, the *value* of that
mapping is again a list of attributes. Single items are allowed, but
this seems to be a special case for convenience.
In the below example from
https://symfony.com/doc/current/reference/constraints/Collection.html
the @Assert\Email could equivalently be written as an array with one item:
/**
* @Assert\Collection(
* fields = {
* "personal_email" = @Assert\Email,
* "short_bio" = {
* @Assert\NotBlank,
* @Assert\Length(
* max = 100,
* maxMessage = "Your short bio is too long!"
* )
* }
* },
* allowMissingFields = true
* )
*/
This reinforces my earlier suggestion
(https://externals.io/message/111936#112109) that #[Foo] in a nested
context can simply imply an array of one attribute, rendering the above as:
#[Assert\Collection(
fields: [
"personal_email" => #[Assert\Email],
"short_bio" => #[
Assert\NotBlank,
Assert\Length(
max: 100,
maxMessage: "Your short bio is too long!"
)
]
],
allowMissingFields: true
]
Regards,
--
Rowan Tommins (né Collins)
[IMSoP]
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: https://www.php.net/unsub.php
Re: [PHP-DEV] Bug #80248 - Swapping parameter names during inheritance does not throw
On 28/10/2020 17:06, Michael Voříšek - ČVUT FEL wrote: For better names, I propose to allow rename "to never used name", thus position is never changed and everything behave consistently across named/unnamed params. I also propose to accept the old names, as currently, if not used with the new names, the parameter names are not resolved, so OOP/LSP is violated. This is precisely the behaviour discussed as an alternative in the RFC: https://wiki.php.net/rfc/named_params#to_parameter_name_changes_during_inheritance The reasons it was not implemented are given at the end of that section. Significant additional code in the engine to perform additional checks and/or name aliasing I know, but we can do very easily one thing - check and throw if overriding method has one or more named parameter on different position. How confident are you that we can "very easily" do this? Have you looked into how it would need to be implemented, and what edge cases we would need to consider? At risk of sounding like a broken record: Nikita did look into it, and documented his conclusions in the RFC. Regards, -- Rowan Tommins (né Collins) [IMSoP] -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: https://www.php.net/unsub.php
Re: [PHP-DEV] Bug #80248 - Swapping parameter names during inheritance does not throw
Significant additional code in the engine to perform additional checks and/or name aliasing I know, but we can do very easily one thing - check and throw if overriding method has one or more named parameter on different position. On class creation time, ie. only once, no overhead per call. Then calling with named/unnamed parameters is **consistent** (or resolves to an "Unknown named parameter" error) and... We are safe! With kind regards / Mit freundlichen Grüßen / S přátelským pozdravem, Michael Voříšek On 28 Oct 2020 17:43, Rowan Tommins wrote: On 28/10/2020 15:14, Michael Voříšek - ČVUT FEL wrote: I agree - "it's harder to imagine a scenario in real life where". [...] If we can agree, that implementation is not guaranteed to be called with named parameters only, what real world usecase to defend this current php behaviour is left? You're thinking about this the wrong way around: the simplest implementation is to detect non-existent named parameters (which happens to include renamed parameters) at run-time; it is additional checks on top of that which need to be justified. Other approaches to the problem require at least one of: * Significant additional code in the engine to perform additional checks and/or name aliasing * Users to change existing code which works correctly, but would theoretically break if used with named parameters The advantages are almost entirely theoretical, with few realistic examples. So the "pragmatic approach" the RFC refers to concludes that the benefit of additional analysis does not outweigh its cost. Regards, -- Rowan Tommins (né Collins) [IMSoP]
Re: [PHP-DEV] Bug #80248 - Swapping parameter names during inheritance does not throw
see https://3v4l.org/X8omS
As long as non-named parameters are supported (they always will be),
calling different implementation can produce different results when
called with named/not-named parametrs.
Let say we have interface/class X with method test(int $offset, int
$limit).
Let's extend it by test(int $limit, int $offset).
When it is called with unnamed arguments, both implementations return
the same result and satisfy the interface. But, if the purpose of the
rename is to allow to swap the parameters, what is the advantage if it
does not work with unnamed arguments?
For better names, I propose to allow rename "to never used name", thus
position is never changed and everything behave consistently across
named/unnamed params. I also propose to accept the old names, as
currently, if not used with the new names, the parameter names are not
resolved, so OOP/LSP is violated.
So my personal summary is, this behaviour is nonsense and should be not
allowed as there is no practical benefit of it.
To your example:
public function foo($str, $needle){
is something I do not propose to forbid, only the names have to be new
or match the original position in iface/parent class
With kind regards / Mit freundlichen Grüßen / S přátelským pozdravem,
Michael Voříšek
On 28 Oct 2020 17:39, Chase Peeler wrote:
On Wed, Oct 28, 2020 at 11:15 AM Michael Voříšek - ČVUT FEL wrote:
I agree - "it's harder to imagine a scenario in real life where".
From php perspective, swapping parameters in inheritance SHOULD NOT be
allowed, as without named parameters the parameters are not swapped.
Also, if the parameters are typed differently, the example is even
impossible (and nowdays, typed parameters are very common, thus
"commonly impossible").
What do you mean they "aren't swapped?"
class A {
public function foo ($str1,$str2){
return strcompare($str1,$str2);
}
}
class B extends A{
public function foo($str2,$str1){
return strcompare($str2,$str);
//or
//return parent::foo($str1,$str2);
}
}
They are swapped in the above example because the code inside B::foo handles the swapping. And, maybe there is a valid reason that the developer wanted to emphasize $str2 over $str1 in the subclass.
If we can agree, that implementation is not guaranteed to be called with
named parameters only, what real world usecase to defend this current
php behaviour is left?
With kind regards / Mit freundlichen Grüßen / S přátelským pozdravem,
Considering that parameter swapping prior to unnamed parameters has always been supported, what unique issue does it represent that requires we solve it now? Anything done to prevent it would almost certainly be a huge BC break. As for real world use-cases, I can think of a few:
1.) Developer the subclass wants to make certain parameters optional which weren't originally at the end of the parameter list
2.) Parent class broke common practices in terms of parameter order ($needle, $haystack vs $haystack, $needle) and they want their subclass to follow the more commonly used pattern.
3.) The developer of the subclass just wants them in a different order for some reason that makes sense to them. Maybe they don't need to support polymorphism. Maybe the idea is that the subclass will be used INSTEAD of the parent class and any knowledge of the parent class by other programmers isn't even necessary.
Michael Voříšek
On 28 Oct 2020 15:57, Rowan Tommins wrote:
On 28/10/2020 10:45, Michael Voříšek - ČVUT FEL wrote:
https://3v4l.org/X8omS parameters renamed, result with named parameters
is different
While it's easy to construct an example where this happens, it's harder to
imagine a scenario in real life where:
* a sub-class overloads the function with different parameter names...
* ...that overlap with the original parameter names... (i.e. the call will
succeed)
* ... but not in the same order...
* ...where calling with ordered parameters results in the expected behaviour
(i.e. it's not already incorrect code)
It seems more likely in practice that a polymorphic call assuming the
parameters are in the same order would fail where one assuming they have the
same names will succeed, e.g.:
class A {
public function search(string $needle, string $haystack) { ... }
}
class B extends A {
public function search(string $haystack, string $needle) { ... }
}
$aOrB->search("foo", "foobar"); // incorrect call on instances of B, but
allowed in every version of PHP
$aOrB->search(needle: "foo", haystack: "foobar"); // correct behaviour whether
instance of A or B :)
I agree that this scenario is very contrived. You are looking at a scenario where multiple examples of poor programming are layered on top of each other. Trying to prevent such scenarios risks going down a rabbit hole that removes a lot of freedom from the language.
https://3v4l.org/kgHWf renamed parameter, call with named parameters
does not succeed at all (which violated basic principe of OOP
inherit
Re: [PHP-DEV] List of attributes
Hi all! about why we'd need nested attributes, here is a discussion about nested validation constraints: https://github.com/symfony/symfony/issues/38503 You'll see that we're looking hard into ways around using nested attributes, but they all fall short. We actually concluded that we should not create a hacky syntax before php-internals settled on the topic. I invite you to review the ideas and arguments posted there to see a real world need on the topic (please don't mind sharing your thoughts about the topic in the PR of course if you think you can help us move forward.) See more comments below: Le ven. 23 oct. 2020 à 17:52, Theodore Brown a écrit : > On Tue, Oct 20, 2020 at 10:13 AM Rowan Tommins > wrote: > > > On Mon, 19 Oct 2020 at 16:17, Theodore Brown > wrote: > > > > > > > > In theory nested attributes could be supported in the same way with > > > the `#[]` syntax, but it's more verbose and I think less intuitive > > > (e.g. people may try to use the grouped syntax in this context, but > > > it wouldn't work). Also the combination of brackets delineating both > > > arrays and attributes reduces readability: > > > > > > #[Assert\All([ > > > #[Assert\Email], > > > #[Assert\NotBlank], > > > #[Assert\Length(max: 100)] > > > ])] > > > > > > > I think you're presupposing how a feature would work that hasn't > > even been specced out yet. On the face of it, it would seem logical > > and achievable for the above to be equivalent to this: > > > > #[Assert\All( > >#[ > > Assert\Email, > > Assert\NotBlank, > > Assert\Length(max: 100) > >] > > )] > > > > i.e. for a list of grouped attributes in nested context to be > > equivalent to an array of nested attributes. > > Hi Rowan, > > The problem with this is that it results in inconsistent semantics, > where the number of items in an attribute group results in a > different type of value being passed. I.e. if you remove two of the > three attributes from the list, suddenly the code will break since > the `Assert\All` attribute is no longer being passed an array. > Yes. This would be solved by NOT accepting #[] inside attribute declarations. Since we borrowed from Rust, let's borrow this again: http://web.mit.edu/rust-lang_v1.25/arch/amd64_ubuntu1404/share/doc/rust/html/reference/attributes.html#attributes The Rust syntax accepts the #[] only on the outside of the declaration. The example above should be written as: #[Assert\All([ Assert\Email(), Assert\NotBlank(), Assert\Length(max: 100) ])] The closing brackets () would be required to remove any ambiguity with constants. Since constant expressions won't ever allow functions in them, this isn't ambiguous with function calls. > // error when Assert\All is instantiated since it's no longer being > // passed an array of attributes: > > #[Assert\All( > #[ > Assert\Email, > ] > )] > > So when removing nested attributes from a list such that there's only > one left in the list, you'd have to remember to additionally wrap the > attribute group in array brackets: > > #[Assert\All( > [#[ > Assert\Email, > ]] > )] > > And of course when you want to add additional attributes to a group > that originally had only one attribute, you'd have to remember to > remove the extra array brackets. > > Generally speaking, having the number of items in a list change the > type of the list is a recipe for confusion and unexpected errors. So > I think the only sane approach is to ban using the grouped syntax in > combination with nested attributes. > > Unfortunately, no one thought through these issues when proposing to > change the shorter attribute syntax away from @@ and add the grouped > syntax, and now it seems like we're stuck with a syntax that is > unnecessarily complex and confusing to use for nested attributes. > > > Unless nested attributes were limited to being direct arguments to > > another attribute, the *semantics* of nested attributes inside > > arrays would have to be defined anyway (e.g. how they would look in > > reflection, whether they would be recursively instantiated by > > newInstance(), etc). > > Yes, the exact semantics of nested attributes need to be defined, but > this is a completely separate topic from the grouped syntax issue > described above. > > As a user I would expect nested attributes to be reflected the same > as any other attribute (i.e. as a `ReflectionAttribute` instance). > Calling `getArguments` on a parent attribute would return an array > with `ReflectionAttribute` objects for any nested attribute passed > as a direct argument or array value. > I 100% agree with this. On first thought I think it makes sense to recursively instantiate > nested attributes when calling `newInstance` on a parent attribute. > This would allow attribute class constructors to specify the typ
Re: [PHP-DEV] Bug #80248 - Swapping parameter names during inheritance does not throw
On 28/10/2020 15:14, Michael Voříšek - ČVUT FEL wrote: I agree - "it's harder to imagine a scenario in real life where". [...] If we can agree, that implementation is not guaranteed to be called with named parameters only, what real world usecase to defend this current php behaviour is left? You're thinking about this the wrong way around: the simplest implementation is to detect non-existent named parameters (which happens to include renamed parameters) at run-time; it is additional checks on top of that which need to be justified. Other approaches to the problem require at least one of: * Significant additional code in the engine to perform additional checks and/or name aliasing * Users to change existing code which works correctly, but would theoretically break if used with named parameters The advantages are almost entirely theoretical, with few realistic examples. So the "pragmatic approach" the RFC refers to concludes that the benefit of additional analysis does not outweigh its cost. Regards, -- Rowan Tommins (né Collins) [IMSoP] -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: https://www.php.net/unsub.php
Re: [PHP-DEV] Bug #80248 - Swapping parameter names during inheritance does not throw
On Wed, Oct 28, 2020 at 11:15 AM Michael Voříšek - ČVUT FEL <
voris...@fel.cvut.cz> wrote:
> I agree - "it's harder to imagine a scenario in real life where".
>
> From php perspective, swapping parameters in inheritance SHOULD NOT be
> allowed, as without named parameters the parameters are not swapped.
> Also, if the parameters are typed differently, the example is even
> impossible (and nowdays, typed parameters are very common, thus
> "commonly impossible").
>
>
What do you mean they "aren't swapped?"
class A {
public function foo ($str1,$str2){
return strcompare($str1,$str2);
}
}
class B extends A{
public function foo($str2,$str1){
return strcompare($str2,$str);
//or
//return parent::foo($str1,$str2);
}
}
They are swapped in the above example because the code inside B::foo
handles the swapping. And, maybe there is a valid reason that the developer
wanted to emphasize $str2 over $str1 in the subclass.
If we can agree, that implementation is not guaranteed to be called with
> named parameters only, what real world usecase to defend this current
> php behaviour is left?
With kind regards / Mit freundlichen Grüßen / S přátelským pozdravem,
>
>
Considering that parameter swapping prior to unnamed parameters has always
been supported, what unique issue does it represent that requires we solve
it now? Anything done to prevent it would almost certainly be a huge BC
break. As for real world use-cases, I can think of a few:
1.) Developer the subclass wants to make certain parameters optional which
weren't originally at the end of the parameter list
2.) Parent class broke common practices in terms of parameter order
($needle, $haystack vs $haystack, $needle) and they want their subclass to
follow the more commonly used pattern.
3.) The developer of the subclass just wants them in a different order for
some reason that makes sense to them. Maybe they don't need to support
polymorphism. Maybe the idea is that the subclass will be used INSTEAD of
the parent class and any knowledge of the parent class by other programmers
isn't even necessary.
> Michael Voříšek
>
> On 28 Oct 2020 15:57, Rowan Tommins wrote:
>
> > On 28/10/2020 10:45, Michael Voříšek - ČVUT FEL wrote:
> >
> >> https://3v4l.org/X8omS parameters renamed, result with named parameters
> >> is different
> >
> > While it's easy to construct an example where this happens, it's harder
> to imagine a scenario in real life where:
> >
> > * a sub-class overloads the function with different parameter names...
> > * ...that overlap with the original parameter names... (i.e. the call
> will succeed)
> > * ... but not in the same order...
> > * ...where calling with ordered parameters results in the expected
> behaviour (i.e. it's not already incorrect code)
> >
> > It seems more likely in practice that a polymorphic call assuming the
> parameters are in the same order would fail where one assuming they have
> the same names will succeed, e.g.:
> >
> > class A {
> > public function search(string $needle, string $haystack) { ... }
> > }
> > class B extends A {
> > public function search(string $haystack, string $needle) { ... }
> > }
> >
> > $aOrB->search("foo", "foobar"); // incorrect call on instances of B, but
> allowed in every version of PHP
> >
> > $aOrB->search(needle: "foo", haystack: "foobar"); // correct behaviour
> whether instance of A or B :)
> >
>
I agree that this scenario is very contrived. You are looking at a scenario
where multiple examples of poor programming are layered on top of each
other. Trying to prevent such scenarios risks going down a rabbit hole that
removes a lot of freedom from the language.
> >> https://3v4l.org/kgHWf renamed parameter, call with named parameters
> >> does not succeed at all (which violated basic principe of OOP
> >> inheritance at least)
> >
> > This is the case that is explicitly discussed in the RFC:
> https://wiki.php.net/rfc/named_params#parameter_name_changes_during_inheritance
> >
> > I'm not sure what more can be said than appears in that summary, and in
> the linked discussion of rejected alternatives. As the RFC says, the
> pragmatic decision was taken to defer these errors to runtime.
> >
> > It's worth noting that since PHP doesn't have checked exceptions, a
> child method throwing an error that it's parent wouldn't is already
> possible and not considered a violation: https://3v4l.org/3m7eo
> >
> > Regards,
> >
> > --
> > Rowan Tommins (né Collins)
> > [IMSoP]
--
Chase Peeler
chasepee...@gmail.com
Re: [PHP-DEV] Bug #80248 - Swapping parameter names during inheritance does not throw
I agree - "it's harder to imagine a scenario in real life where".
From php perspective, swapping parameters in inheritance SHOULD NOT be
allowed, as without named parameters the parameters are not swapped.
Also, if the parameters are typed differently, the example is even
impossible (and nowdays, typed parameters are very common, thus
"commonly impossible").
If we can agree, that implementation is not guaranteed to be called with
named parameters only, what real world usecase to defend this current
php behaviour is left?
With kind regards / Mit freundlichen Grüßen / S přátelským pozdravem,
Michael Voříšek
On 28 Oct 2020 15:57, Rowan Tommins wrote:
On 28/10/2020 10:45, Michael Voříšek - ČVUT FEL wrote:
https://3v4l.org/X8omS parameters renamed, result with named parameters
is different
While it's easy to construct an example where this happens, it's harder to
imagine a scenario in real life where:
* a sub-class overloads the function with different parameter names...
* ...that overlap with the original parameter names... (i.e. the call will
succeed)
* ... but not in the same order...
* ...where calling with ordered parameters results in the expected behaviour
(i.e. it's not already incorrect code)
It seems more likely in practice that a polymorphic call assuming the
parameters are in the same order would fail where one assuming they have the
same names will succeed, e.g.:
class A {
public function search(string $needle, string $haystack) { ... }
}
class B extends A {
public function search(string $haystack, string $needle) { ... }
}
$aOrB->search("foo", "foobar"); // incorrect call on instances of B, but
allowed in every version of PHP
$aOrB->search(needle: "foo", haystack: "foobar"); // correct behaviour whether
instance of A or B :)
https://3v4l.org/kgHWf renamed parameter, call with named parameters
does not succeed at all (which violated basic principe of OOP
inheritance at least)
This is the case that is explicitly discussed in the RFC:
https://wiki.php.net/rfc/named_params#parameter_name_changes_during_inheritance
I'm not sure what more can be said than appears in that summary, and in the
linked discussion of rejected alternatives. As the RFC says, the pragmatic
decision was taken to defer these errors to runtime.
It's worth noting that since PHP doesn't have checked exceptions, a child
method throwing an error that it's parent wouldn't is already possible and not
considered a violation: https://3v4l.org/3m7eo
Regards,
--
Rowan Tommins (né Collins)
[IMSoP]
Re: [PHP-DEV] Bug #80248 - Swapping parameter names during inheritance does not throw
On 28/10/2020 10:45, Michael Voříšek - ČVUT FEL wrote:
https://3v4l.org/X8omS parameters renamed, result with named parameters
is different
While it's easy to construct an example where this happens, it's harder
to imagine a scenario in real life where:
* a sub-class overloads the function with different parameter names...
* ...that overlap with the original parameter names... (i.e. the call
will succeed)
* ... but not in the same order...
* ...where calling with ordered parameters results in the expected
behaviour (i.e. it's not already incorrect code)
It seems more likely in practice that a polymorphic call assuming the
parameters are in the same order would fail where one assuming they have
the same names will succeed, e.g.:
class A {
public function search(string $needle, string $haystack) { ... }
}
class B extends A {
public function search(string $haystack, string $needle) { ... }
}
$aOrB->search("foo", "foobar"); // incorrect call on instances of B, but
allowed in every version of PHP
$aOrB->search(needle: "foo", haystack: "foobar"); // correct behaviour
whether instance of A or B :)
https://3v4l.org/kgHWf renamed parameter, call with named parameters
does not succeed at all (which violated basic principe of OOP
inheritance at least)
This is the case that is explicitly discussed in the RFC:
https://wiki.php.net/rfc/named_params#parameter_name_changes_during_inheritance
I'm not sure what more can be said than appears in that summary, and in
the linked discussion of rejected alternatives. As the RFC says, the
pragmatic decision was taken to defer these errors to runtime.
It's worth noting that since PHP doesn't have checked exceptions, a
child method throwing an error that it's parent wouldn't is already
possible and not considered a violation: https://3v4l.org/3m7eo
Regards,
--
Rowan Tommins (né Collins)
[IMSoP]
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: https://www.php.net/unsub.php
Re: [PHP-DEV] Bug #80248 - Swapping parameter names during inheritance does not throw
I see, guys, what is the single reason to allowing this dangerous inheritance? https://3v4l.org/X8omS parameters renamed, result with named parameters is different https://3v4l.org/kgHWf renamed parameter, call with named parameters does not succeed at all (which violated basic principe of OOP inheritance at least) With kind regards / Mit freundlichen Grüßen / S přátelským pozdravem, Michael Voříšek On 26 Oct 2020 15:36, Rowan Tommins wrote: On 26/10/2020 13:12, Michael Voříšek - ČVUT FEL wrote: I am writing regarding bug 80248. Currently, PHP 8 allows parameter reuse at different position, which I belive is very dangerous, as passed parameters may be passed in a different order with different object impl. Hi Michael, Yes, this was one of the most discussed aspects of named parameters. There were a few proposals for how it should work, but in the end none of them gained consensus, and Nikita decided to keep things simple. I suggest having a look at the RFC [https://wiki.php.net/rfc/named_params] and its main discussion thread [https://externals.io/message/110004] to see the pros and cons of various alternatives. Since none of these were adopted in the proposal, it's not clear that this could be considered a bug, rather than a feature change, so there'd need to be a very strong justification for re-visiting it this close to 8.0 availability. Regards, -- Rowan Tommins (né Collins) [IMSoP]
