Update ext/standard parameter names

[nikic]

Just putting this up to avoid duplicate work ... will take a while to go through all of it.

[nikic]

I've done a first pass through everything ...

[nikic]

These names are excessively long, but I'm not familiar enough with this API to suggest a good concise name.

[kocsismate]

Other ideas:

• array_push(): I think we could use $array instead of $stack (update: I now see that you did this change in a few places)
• array_walk*(): $array instead of $input, $args instead of $argument
• array_filter(): $flags instead of $use_keys
• move_uploaded_file(): maybe "from" and "to" could be used (either also including path or not: $from/$from_path), but I like the source - destination terms too
• headers_sent(): $filename

[kocsismate]

Slightly unrelated: I saw that the manual has a very detailed description about the two versions of strtr() (https://www.php.net/manual/en/function.strtr.php), and this is the first time when I realized that the default value fix for PHP 8.0 has a side-effect that we can't have an entirely accurate name for the 2nd parameter :/

[nikic]

Something to discuss, but I think what we should do with these overloaded signatures is to keep listing them as multiple signatures in the manual and define a policy that overloaded functions are excluded from our parameter name stability policy -- and that we reserve the right to make invoking them with named params a hard error in the future (once we have "positional only" arguments). We just can't support these kinds of signatures properly.

[kocsismate]

Bob had a very good suggestion IMO to rename these to $from, $to, $subject. Sometimes I'm also wondering if $search is what we search for or where we search, so I'd also prefer these names. :)

[nikic]

strtr() uses $from, $to terminology, so there's some precedent there...

Something we also need to consider is how this will interact with pcre APIs. Here we have $search, $replace, $subject. In preg_replace, we have $pattern, $replace, $subject. In preg_replace_callback, we have $pattern, $callback, $subject.

[kocsismate]

nit: in other places character is not abbreviated. I'm ok with $max_chars_per_line though, it already has very long a name.

[nikic]

* `array_push()`: I think we could use `$array` instead of `$stack` (update: I now see that you did this change in a few places)

Done! This was indeed just an oversight.

* `array_walk*()`: `$array` instead of `$input`, `$args` instead of `$argument`

I've renamed $input to $array and $argument to $arg. However, I think it should stay in the singular, as you can only pass through a single argument.

* `array_filter()`: `$flags` instead of `$use_keys`

Hm... it looks like the values here don't form a bitmaks. I've changed it to $mode, does that seem reasonable?

* `move_uploaded_file()`:  _maybe_ "from" and "to" could be used (either also including `path` or not: `$from`/`$from_path`), but I like the source - destination terms too

I'm not sure on this one. We seem to be using "old"/"new" for renaming operations (see also rename) and "source"/"dest" for copy operations (see also copy, stream_copy_to_stream).

* `headers_sent()`: `$filename`

Done.

[kocsismate]

Oh gosh, I finally reached the bottom of the stub file. :D (although, this time I only vaguely checked the non-changed parts).

• closedir: should $dir_handle become a $stream?
• password_needs_rehash: $hash to $password?

[kocsismate]

I've renamed $input to $array and $argument to $arg. However, I think it should stay in the singular, as you can only pass through a single argument.

the plural was just a typo ^^

[kocsismate]

Hm... it looks like the values here don't form a bitmaks. I've changed it to $mode, does that seem reasonable?

Yes, absolutely!

I'm not sure on this one. We seem to be using "old"/"new" for renaming operations (see also rename) and "source"/"dest" for copy operations (see also copy, stream_copy_to_stream).

I'm on board :)

[nikic]

* `closedir`: should `$dir_handle` become a `$stream`?

Not sure on this one: You are right that closedir() accepts a stream resource, but it's a special kind (PHP_STREAM_FLAG_IS_DIR) created by opendir(). Passing something like fopen() to it will throw. Is it worthwhile to distinguish this?

* `password_needs_rehash`: `$hash` to `$password`?

This function accepts a hash from password_hash(), not a plain password.

[kocsismate]

Not sure on this one: You are right that closedir() accepts a stream resource, but it's a special kind (PHP_STREAM_FLAG_IS_DIR) created by opendir(). Passing something like fopen() to it will throw. Is it worthwhile to distinguish this?

Yes, you're right then, I wasn't really aware of the distinction. Will $dir_handle still work well when we convert this resource to object? Ah, I've just checked it, and since Directory::close() is the implementation alias of closedir, I guess we could convert the resource to Directory, no? Thus, I'd go with $directory if all this is really possible.

This function accepts a hash from password_hash(), not a plain password.

Ah, yes, I didn't realize at first that it's for distinguishing hashed password from plain password. This is something I also support very much, so +1!

[nikic]

Not sure on this one: You are right that closedir() accepts a stream resource, but it's a special kind (PHP_STREAM_FLAG_IS_DIR) created by opendir(). Passing something like fopen() to it will throw. Is it worthwhile to distinguish this?

Yes, you're right then, I wasn't really aware of the distinction. Will $dir_handle still work well when we convert this resource to object? Ah, I've just checked it, and since Directory::close() is the implementation alias of closedir, I guess we could convert the resource to Directory, no? Thus, I'd go with $directory if all this is really possible.

Hard to say. Depends on how the conversion is done. If we use the same underlying implementation, directories might also be an instance of Stream rather than a separate object.

I think $directory is a fine name here -- my only concern is that it does not give an obvious distinction between a directory string and a directory resource.

[kocsismate]

@nikic The PR seems good for me now! :) Somewhere you asked a question about the as_ for bool params, but I can't find it now. I think we could still fine tune these kind of param names a bit later. So all in all, I think this PR is good for landing for RC1, and we could do smaller amendments and "are we sure we're good with it"-kind PRs if necessary next week.

[nikic]

@kocsismate Yeah, I think these are the open questions that are left:

• $as_ prefix.
• $dir_handle
• str_replace naming (let's solve this one while handling ext/pcre).

We'll have to do some fine tuning later.

[kocsismate]

Ok, I'm leaving two more small ideas here:

• parse_url: first param could be $uri because it accepts URIs. It doesn't match with the function name though, so this change is not a must-have. P.S. ext/ldap is about to use $uri.
• is_callable(): $callable_name could be $name or $callback_name I think.

[nikic]

Another thing we'll have to check is that manual references to argument #n ($xxx) in error messages are correct. I'm sure some of them have been broken.