Resurrecting named parameters

LSP checks for parameter names

Parameter names currently have no particular significance in PHP, only
their position in the signature is important. If named parameters are
introduced (in a way that does not require opt-in at the declaration-site),
then parameter names become significant. This means that changing a
parameter name during inheritance constitutes an LSP violation (for
non-constructor methods).

I would prefer to emit a low severity diagnostic if the parameter
names do not match during inheritance, specifically noting that named
parameter calls may not work as expected on such methods, rather than
emitting a diagnostic when named parameters get used.

If we decide to do it at named parameter usage as proposed, I would
want a stronger error than simply a diagnostic: the user is directly
hitting a case where we have very reasonable doubt that the code is
doing what was intended.

Internal functions

The larger problem is that internal functions don't have a well-defined
concept of parameter default value. Parameter defaults are implicit in C
code, and sometimes based on argument count checks. When named parameters
are involved, one generally cannot assume that if a later (optional)
parameter is passed, all earlier (optional) parameters are passed as well.

I know that if we can make parameter defaults in Reflection that some
people would consider that reason enough to vote yes on this proposal.
It's always annoying with Internal and User functions have
differences.

I am okay with a heavier cost to named parameters in general. It's a
new feature, and generally a convenience one.

I am a bit worried about variadics having named parameters in them. I
remember encountering code that assumes that ...$a is a packed
array. Is that assumption always correct today?

Make named-parameter opt-in in some fashion, so that parameter names
only need to be preserved for methods that have the opt-in marker. I'm not
a fan of this, as it greatly diminishes the practical usefulness of named
parameters.

I generally prefer the idea of named parameters requiring some sort of
opt-in at the definition site.

I feel like this would solve a whole bunch of problems:

• LSP violations could be stricter errors, because they would show up
  once the parent class opted in, not as soon as you upgraded your PHP
  version.

• Similarly, library authors would have a chance to tidy up their
  parameter naming, knowing that it was going to be a part of the
  compatibility contract for future versions. Without an opt-in, they
  might find themselves unable to fix poor or inconsistent names without
  declaring a BC break.

• Internal functions could also be "opted in" gradually, giving core
  devs and extension authors time to sort out any that do odd things with
  ZPP or ZEND_NUM_ARGS.

• Variadic functions could be automatically "opted out" without needing
  a particular special case (they could just give a generic "function
  doesn't support named params" error).

The biggest downside I can see is that there's no obvious way of library
code opting in to named parameters when run under PHP 8 (or whenever we
add them) but also compiling successfully under earlier versions. That
would certainly slow the spread of the feature rather drastically, which
would be a shame. I wonder if there's a clever syntax we could adopt
that would avoid this.

In some ways, it would be nice to go one step further, and separate the
names presented to callers from the names used inside the function body.
Since parameters become ordinary mutable variables for the rest of the
function, it can be tempting to think of them as part of the
implementation, not part of the signature. This can lead to them
naturally changing both over time, and in inheritance hierarchies.

For instance, an interface might define a parameter as "$id", but a
concrete class want to label it as "$requestedFoobarId" to make the
implementation clearer. It would be nice to be able to (optionally)
define both. Straw man example syntax, using ":$foo" as opt-in, and
"foo: $bar" as aliasing:

function get( int id: $requestedFoobarId, boolean :$deep )

That would essentially be sugar for:

<<AllowCallingWithNamedParams>>
function get( int $id, boolean $deep ) {
    $requestedFoobarId = $id;
    unset($id);
    // ...
}

In the same way, normal positional parameters could be (and in some
languages are) seen as sugar for this:

function get( int $1, boolean $2 ) {
    $requestedFoobarId = $1;
    $deep = $2;
    // ...
}

It would also give another option for the LSP problem - if the parent
class or interface opts into named parameters, the child class gets
implicit aliases if it doesn't specify any:

interface Foo { function thing( :$id ); } // named parameter "id"
class Bar implements Foo { function thing( id: $barId ) {} } // explicit
alias matches interface, $bar->thing(id: 42) will set $barId=42
class Baz implements Foo { function thing( $bazId ) {} } // implicitly
aliased, $baz->thing(id: 42) will set $bazId=42 based on the name in the
interface
class Wrong implements Foo { function thing( :$wrongName ) {} } //
Error: tries to accept named parameter "wrongName", but interface
requires it to accept named parameter "id" instead

Regards,

--
Rowan Tommins (né Collins)
[IMSoP]

Le mardi 7 avril 2020, 14:44:38 CEST Nikita Popov a écrit :

The larger problem is that internal functions don't have a well-defined
concept of parameter default value. Parameter defaults are implicit in C
code, and sometimes based on argument count checks. When named parameters
are involved, one generally cannot assume that if a later (optional)
parameter is passed, all earlier (optional) parameters are passed as well.

While it is possible (but rather challenging) to make things work mostly
transparently with current ZPP, some functions do need to adjusted to
support named parameters, and may cause misbehavior/crashes if they are
not. As a rule of thumb, functions that make non-trivial use of
ZEND_NUM_ARGS() are affected. This is something we can address for
functions in bundled extensions, but may also affect 3rd-party extensions.

I think that ideally, we'd deal with internal functions the same way we do
for userland functions, by relying on default values that are part of the
function declaration. As part of the PHP stub work, we now have information
on default values in stub files, but this information is currently not
available to the engine.

This is a more general problem with internal functions which bugs me a lot.
They can behave in a different way when not providing an attribute and when providing the default value.

We should aim to make them behave exactly as a userland function with the same stub, no?

--
Côme Chilliet
FusionDirectory - https://www.fusiondirectory.org

Hi!

2. Throw a notice if a parameter name is changed. While LSP violations are
   normally fatal errors (in PHP 8), we could use a lower-severity diagnostic
   for this case, that allows code to still run, but makes developers aware of
   the problem. (It should be noted that automatic fixup of parameter names
   using tooling should be fairly easy to implement.)

I think we should have a notice for now, and eventually when people are
used to it upgrade it to an error.

The larger problem is that internal functions don't have a well-defined
concept of parameter default value. Parameter defaults are implicit in C
code, and sometimes based on argument count checks. When named parameters
are involved, one generally cannot assume that if a later (optional)
parameter is passed, all earlier (optional) parameters are passed as well.

IIRC I did some work on that when making the skipped parameter RFC, but
I think we have no choice but to make all internal functions we control
have proper default implementation. Older modules may be lagging though,
so another option would be to add some kind of flag to internal
functions that would block named parameters if not set, and make
extension writers fix their functions and then set this flag explicitly.
Of course we'll set this flag for all functions in core, after fixing
those of them that need fixing. Maybe not a flag but using different
macro/API function, etc. - the idea is that some explicit change would
be needed. Yes, that means additional work...

More complicated but milder option would be to only check the flag if
there's a "gap" in parameters - but this may have higher learning curve
as extension writers wouldn't even know their extension needs work until
someone skips a parameter.

Stas Malyshev
smalyshev@gmail.com

Hi internals,

It's been a few years since I originally brought up the named parameters
RFC: https://wiki.php.net/rfc/named_params

This topic has recently come up as part of
https://externals.io/message/109220 again, in particular with the
observation that the combination of constructor parameter promotion and
named parameters will give us, as a side-effect, an ergonomic object
initialization syntax that is well-integrated with the remainder of the
language.

I've reimplemented basic named param support for master in
https://github.com/php/php-src/pull/5357, but before going any further
here, I want to restart discussion on the various open issues that the
named parameter concept in PHP has. I think there's two primary issues:

LSP checks for parameter names

Parameter names currently have no particular significance in PHP, only
their position in the signature is important. If named parameters are
introduced (in a way that does not require opt-in at the declaration-site),
then parameter names become significant. This means that changing a
parameter name during inheritance constitutes an LSP violation (for
non-constructor methods). There are a number of ways in which this issue
may be approached:

1. Silently ignore the problem. No diagnostic is issued when a parameter
   name is changed during inheritance. An error exception will be thrown when
   trying to use the (now) unknown parameter name.

2. Throw a notice if a parameter name is changed. While LSP violations are
   normally fatal errors (in PHP 8), we could use a lower-severity diagnostic
   for this case, that allows code to still run, but makes developers aware of
   the problem. (It should be noted that automatic fixup of parameter names
   using tooling should be fairly easy to implement.)

3. Allow using parameter names from the parent method, even if they have
   been renamed. This makes things "just work", but seems fairly magic, and
   has edge cases like a signature foo($a, $b) being changed to foo($b, $a),
   where it's not possible to implicitly support both at the same time.

4. Make named-parameter opt-in in some fashion, so that parameter names
   only need to be preserved for methods that have the opt-in marker. I'm not
   a fan of this, as it greatly diminishes the practical usefulness of named
   parameters.

I've done some due diligence on this issue, and checked how other languages
with named parameters handle this, and also did some quick checks on what
the fallout looks like if we add a notice.

C#

C# does something fairly peculiar. Changing the parameter name introduces a
new Schrödinger's overload: We use "new" here to indicate that the parent
method is hidden, and it would be if we invoked positionally, but we can
actually still access the parent method by performing the call with the old
parameter name. This seems pretty weird to me.

class Test1
{
    public void test(int x) {
        Console.WriteLine("X: {0}", x);
    }
}

class Test2 : Test1
{
     public new void test(int y) {
         Console.WriteLine("Y: {0}", y);
     }
}

public class Program
{
     public static void Main()
     {
         Test2 test2 = new Test2();
         test2.test(y: 42); // Y: 42
         test2.test(x: 42); // X: 42
     }
}

Python

Unsurprisingly, Python just ignores the problem. If you use the name from
the parent, you get an error.

class Test1:
  def test(self, x):
    print('X:', x);

class Test2(Test1):
  def test(self, y):
    print('Y:', y);

test2 = Test2();
test2.test(y=42); // Y: 42
test2.test(x=42); // TypeError: test() got an unexpected keyword argument
'x'

Ruby

Similarly, Ruby also ignores the issue. It should be noted though that Ruby
distinguishes keyword arguments from positional arguments, so you need to
opt-in to their use (and if you do, they must be used at the call-site).

class Test1
  def test(x:)
    print('X: ', x)
  end
end

class Test2 < Test1
  def test(y:)
    print('Y: ', y)
  end
end

test2 = Test2.new;
test2.test(y: 42) // Y: 42
test2.test(x: 42) // missing keyword: y

Kotlin

Kotlin does what is reasonable: There is a warning about the parameter name
change at the declaration-site, and a compile-error at the call-site.

open class Test1 {
    open fun test(x: Int) {
        println("X: $x")
    }
}

class Test2 : Test1() {
    // Warning: the corresponding parameter in the supertype 'Test1' is
named 'x'.
    //          This may cause problems when calling this function with
named arguments.
    override fun test(y: Int) {
        println("Y: $y")
    }
}

fun main() {
    val test2 = Test2()
    test2.test(y=42); // Y: 42
    // Error: Cannot find a parameter with this name: x
    test2.test(x=42);
}

Swift

Swift behaves somewhat similarly to C#, in that by default changing the
parameter name introduces a new overload. In this case it's a proper
overload though, as named parameters must be used at the call-site, so
there is no method hiding going on.

class Test1 {
    func test(x: Int) {
        print("X: ", x)
    }
}

class Test2 : Test1 {
    func test(y: Int) {
        print("Y: ", y)
    }
}

let test2 = Test2()
test2.test(x: 42)
test2.test(y: 42)

When specifying that an override is desired instead, an error is generated:

class Test2 : Test1 {
    // error: argument labels for method 'test(y:)'
    //        do not match those of overridden method 'test(x:)'
    override func test(y: Int) {
        print("Y: ", y)
    }
}

Swift also distinguishes between the internal and the exernal parameter
names, so it's possible to rename the internal name, while keeping the
external one:

class Test2 : Test1 {
    override func test(x y: Int) {
        print("Y: ", y)
    }
}

Internal methods

When adding a notice for changed parameter names, methods defined by
bundled extensions are mostly clean, with one notable exception:

PHP Notice: Parameter $offset of ArrayAccess::offsetExists() has been
renamed to $object in WeakMap::offsetExists(), which may break named
parameters in Unknown on line 0
PHP Notice: Parameter $offset of ArrayAccess::offsetGet() has been
renamed to $object in WeakMap::offsetGet(), which may break named
parameters in Unknown on line 0
PHP Notice: Parameter $offset of ArrayAccess::offsetSet() has been
renamed to $object in WeakMap::offsetSet(), which may break named
parameters in Unknown on line 0
PHP Notice: Parameter $offset of ArrayAccess::offsetUnset() has been
renamed to $object in WeakMap::offsetUnset(), which may break named
parameters in Unknown on line 0

We get similar warnings for CachingIterator, ArrayObject, ArrayIterator,
SplDoublyLinkedList, SplFixedArray, SplObjectStorage, Phar and PharData.
The commonality is always that ArrayAccess parameters are renamed.

PHP Notice: Parameter $position of SeekableIterator::seek() has been
renamed to $line_pos in SplFileObject::seek(), which may break named
parameters in Unknown on line 0

This is the only occurrence that is not related to ArrayAccess.

Userland methods

When testing PHP-Parser, there are only two warnings. One comes from
generated PhpUnit mocks, where codegen would just need to be fixed:

Parameter $invocationRule of
PHPUnit\Framework\MockObject\MockObject::expects() has been renamed to
$matcher in Mock_Parser_eaee5d81::expects(), which may break named
parameters

The other one is a renamed parameter in my own code:

Parameter $node of PhpParser\NodeVisitorAbstract::enterNode() has been
renamed to $origNode in PhpParser\NodeVisitor\CloningVisitor::enterNode(),
which may break named parameters

To test a larger project, here are warnings during Laravel startup:
https://gist.github.com/nikic/c624399659ef1052e8751d8bb3024bc4

There quite a few more of these. The majority of the warnings occurs
because an internal method parameter was renamed, not a userland parameter.
I haven't checked whether the parameters match docs or not here.

From the non-internal cases, one that stood out is:

Notice: Parameter $files of
Orchestra\Testbench\Contracts\TestCase::call() has been renamed to $cookies
in Orchestra\Testbench\TestCase::call(), which may break named parameters
in
/home/nikic/repos/framework/vendor/orchestra/testbench-core/src/TestCase.php
on line 15
Notice: Parameter $server of
Orchestra\Testbench\Contracts\TestCase::call() has been renamed to $files
in Orchestra\Testbench\TestCase::call(), which may break named parameters
in
/home/nikic/repos/framework/vendor/orchestra/testbench-core/src/TestCase.php
on line 15
Notice: Parameter $content of
Orchestra\Testbench\Contracts\TestCase::call() has been renamed to $server
in Orchestra\Testbench\TestCase::call(), which may break named parameters
in
/home/nikic/repos/framework/vendor/orchestra/testbench-core/src/TestCase.php
on line 15
Notice: Parameter $changeHistory of
Orchestra\Testbench\Contracts\TestCase::call() has been renamed to $content
in Orchestra\Testbench\TestCase::call(), which may break named parameters
in
/home/nikic/repos/framework/vendor/orchestra/testbench-core/src/TestCase.php
on line 15

That looks like an actual bug where the signature went out of sync.
Illuminate defines:

public function call($method, $uri, $parameters = [], $cookies = [],
$files = [], $server = [], $content = null)

while Orchestra defines:

public function call($method, $uri, $parameters = [], $files = [],
$server = [], $content = null, $changeHistory = true);

Note that the $cookies parameter went missing.

Conclusion

From other languages, we have:

• Python, Ruby: Parameter name changes ignored, throw error on call.
• C#, Swift: Parameter name changes introduce new overload. (Unless
  "override" is used, in which case it is an error.)
• Kotlin: Warns on parameter name change, error on call.

From the warnings observed in practice, I think the ArrayAccess case is an
interesting one, in that it's pretty unlikely that those methods will ever
be called with named parameters (usually they are not explicitly called at
all!)

I think we might ultimately be best off with going for the Python/Ruby
approach, at least initially, and encourage IDEs / static analyzers / style
analyzers to offer inspections that check whether parameter names match. As
we are retrofitting named parameters to a language where parameter names
held no significance historically, being overly pedantic about this might
not do us any favors, especially if we consider that languages like Python
do get away with not enforcing anything in this regard.

Internal functions

There are two problems when it comes to named parameters and internal
functions. The first is that the internal parameter names do not always
match the parameter names in the documentation. This is nothing we can't
solve (in a hopefully mostly automated way), but is work that needs to be
done.

The larger problem is that internal functions don't have a well-defined
concept of parameter default value. Parameter defaults are implicit in C
code, and sometimes based on argument count checks. When named parameters
are involved, one generally cannot assume that if a later (optional)
parameter is passed, all earlier (optional) parameters are passed as well.

While it is possible (but rather challenging) to make things work mostly
transparently with current ZPP, some functions do need to adjusted to
support named parameters, and may cause misbehavior/crashes if they are
not. As a rule of thumb, functions that make non-trivial use of
ZEND_NUM_ARGS() are affected. This is something we can address for
functions in bundled extensions, but may also affect 3rd-party extensions.

I think that ideally, we'd deal with internal functions the same way we do
for userland functions, by relying on default values that are part of the
function declaration. As part of the PHP stub work, we now have information
on default values in stub files, but this information is currently not
available to the engine.

Máté Kocsis has been working on getting this information exposed in
Reflection, see https://github.com/php/php-src/pull/5353. In the current
form, accessing these defaults is fairly inefficient, but maybe with some
caching, we could base named parameters on this information:

If there are no "gaps" in the passed parameters, then named parameters
will always work. If an optional parameter is not specified, but a later
parameter is, then we will fetch the default value for that parameter, and
pass it instead. If the parameter is optional, but has no well-defined
default (UNKNOWN in stubs), then the call is not permitted (Error: "Cannot
skip parameter $foo with unknown default value", or similar.)

For the record, internal function default values are now available via
Reflection, so the technical groundwork to go down this path is present now.

Regards,
Nikita

LSP checks for parameter names

Parameter names currently have no particular significance in PHP, only
their position in the signature is important. If named parameters are
introduced (in a way that does not require opt-in at the declaration-site),
then parameter names become significant. This means that changing a
parameter name during inheritance constitutes an LSP violation (for
non-constructor methods). There are a number of ways in which this issue
may be approached:

1. Silently ignore the problem. No diagnostic is issued when a parameter
   name is changed during inheritance. An error exception will be thrown when
   trying to use the (now) unknown parameter name.

I prefer this approach. It might seem inelegant at first but I believe
none of the other options will do us any real favors, especially
stronger LSP enforcement on the language level. And opt-in seems like
a non-starter for reasons Rowan Tommins lay out (though he's in favor
:-)
Dev side tools will be fully able to "deal" with the downfall.

Hi internals,

It's been a few years since I originally brought up the named parameters
RFC: https://wiki.php.net/rfc/named_params

This topic has recently come up as part of
https://externals.io/message/109220 again, in particular with the
observation that the combination of constructor parameter promotion and
named parameters will give us, as a side-effect, an ergonomic object
initialization syntax that is well-integrated with the remainder of the
language.

I've reimplemented basic named param support for master in
https://github.com/php/php-src/pull/5357, but before going any further
here, I want to restart discussion on the various open issues that the
named parameter concept in PHP has. I think there's two primary issues:

LSP checks for parameter names

Parameter names currently have no particular significance in PHP, only
their position in the signature is important. If named parameters are
introduced (in a way that does not require opt-in at the declaration-site),
then parameter names become significant. This means that changing a
parameter name during inheritance constitutes an LSP violation (for
non-constructor methods). There are a number of ways in which this issue
may be approached:

1. Silently ignore the problem. No diagnostic is issued when a parameter
   name is changed during inheritance. An error exception will be thrown when
   trying to use the (now) unknown parameter name.

2. Throw a notice if a parameter name is changed. While LSP violations are
   normally fatal errors (in PHP 8), we could use a lower-severity diagnostic
   for this case, that allows code to still run, but makes developers aware of
   the problem. (It should be noted that automatic fixup of parameter names
   using tooling should be fairly easy to implement.)

I think option 2 here makes the most sense. People haven't taken parameter
names into account for now, so warning them in 8.x should help avoid subtle
bugs when starting to use named parameters. and we could make it an LSP
failure in 9.

3. Allow using parameter names from the parent method, even if they have
   been renamed. This makes things "just work", but seems fairly magic, and
   has edge cases like a signature foo($a, $b) being changed to foo($b, $a),
   where it's not possible to implicitly support both at the same time.

4. Make named-parameter opt-in in some fashion, so that parameter names
   only need to be preserved for methods that have the opt-in marker. I'm not
   a fan of this, as it greatly diminishes the practical usefulness of named
   parameters.

Internal functions

There are two problems when it comes to named parameters and internal
functions. The first is that the internal parameter names do not always
match the parameter names in the documentation. This is nothing we can't
solve (in a hopefully mostly automated way), but is work that needs to be
done.

The larger problem is that internal functions don't have a well-defined
concept of parameter default value. Parameter defaults are implicit in C
code, and sometimes based on argument count checks. When named parameters
are involved, one generally cannot assume that if a later (optional)
parameter is passed, all earlier (optional) parameters are passed as well.

I still can see us getting 95-99% of php-src migrated to use parameters
form the documentation in a short amount of time before 8, as the more
detailed and complex work on the stubs has shown.

While it is possible (but rather challenging) to make things work mostly
transparently with current ZPP, some functions do need to adjusted to
support named parameters, and may cause misbehavior/crashes if they are
not. As a rule of thumb, functions that make non-trivial use of
ZEND_NUM_ARGS() are affected. This is something we can address for
functions in bundled extensions, but may also affect 3rd-party extensions.

I think that ideally, we'd deal with internal functions the same way we do
for userland functions, by relying on default values that are part of the
function declaration. As part of the PHP stub work, we now have information
on default values in stub files, but this information is currently not
available to the engine.

Máté Kocsis has been working on getting this information exposed in
Reflection, see https://github.com/php/php-src/pull/5353. In the current
form, accessing these defaults is fairly inefficient, but maybe with some
caching, we could base named parameters on this information:

If there are no "gaps" in the passed parameters, then named parameters will
always work. If an optional parameter is not specified, but a later
parameter is, then we will fetch the default value for that parameter, and
pass it instead. If the parameter is optional, but has no well-defined
default (UNKNOWN in stubs), then the call is not permitted (Error: "Cannot
skip parameter $foo with unknown default value", or similar.)

I think this is a good approach. Even when the function has more complex
"default" logic than the stub can represent, the default value in the stub
(and more importantly in the docs) is what counts for users, not what the
function body does when the argument is not present.

Any thoughts on these issues, or named parameters in general would be
welcome.

Regards,
Nikita