Bug #80248 - Swapping parameter names during inheritance does not throw View as Markdown

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 Wed, Oct 28, 2020 at 11:15 AM Michael Voříšek - ČVUT FEL vorismi3@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/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
chasepeeler@gmail.com

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/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]