Hacker News Reader

Comment by p4bl0 | Hacker News Reader

Something I'd really like is for PHP to somehow be stricter on the number of arguments passed to a function.

As of now, PHP emits an error if arguments are missing but not if there are too many.

A way to bake that in without breaking old code would be to allow function definition to put an explicit stop to the argument list, for example using the void type keyword:

    function foo (int $a, string $b, void) : bool
    { ... }

A few month ago I discussed this on the development mailing list and people seemed to agree and even suggested that this would be a good idea by default without the keyword thing I suggested. But I never got the time to properly write an RFC. There is already an old one from years ago that was voted against but In was told it was from before anything strict and typing related was considered important in PHP. If anyone's up to it, please write this RFC :) !

View on HN

FWIW, core PHP functions do throw an ArgumentCountError when passing fewer OR more parameters than the signature allows.

Yep! And currently the behavior is not consistent with user defined functions.

I'm not a fan of the void argument syntax. Wouldn't something like the code below work? We already do it with `strict_types=1`.

<?php declare(strict_args=1);

Mark the function with an attribute to allow the old behavior. See link for similar use case.

https://www.php.net/manual/en/class.allowdynamicproperties.p...

Yup, that's even better.

That's equivalent to my initial proposal, except that mine adds the information in the type signature of the function rather than in a decorator.

If I understood your proposal correctly, to get the new behavior add an explicit stop to the function, my proposal add attribute to keep old behavior.

Thus if understanding this right that would require to update every function signature to new behavior rather than marking a few functions to get the old behavior and automatically get the new better behavior of every other function for free.

Oh okay, you would do it in reverse. I strongly agree with that. But that means the new feature is opt-out rather than opt-in, and it may break some old code. Maybe it should be done in two-steps (opt-in + deprecation, then opt-out).

Typically you emit E_DEPRECATED for a full major version, then in the next major version you throw an error, e.g if it would land in PHP 9.0 then E_DEPRECATED for non-compliant functions and in PHP 10.0 start throwing errors.

One of the main advantages of actually allowing more arguments is forward compatibility:

You can, within a library, provide an additional argument to a callback without actually introducing a BC break for all users.

My favorite approach would be allowing too many args on dynamic calls (closures, and function calls with dynamic name, not method calls in general) and otherwise rejecting it.

There's no need to have this in the language just to solve the case you describe.

"function current_thing(id: Int, callback: Fn(Int)) {}" and, when you decide you need more you have a myriad of options to add these. From the despised "function current_real_thing(id: Ind, success_callback: Fn(Int), error_callback: Fn(Error)) {}" to "some_namespace:current_thing(...)" via "OtherClass::current_thing(...)" to "load current_thing from NewImplementationOfThing" and so on.

Being strict and explicit isn't opposed to being flexible. And strictness and explicitness is most often a predicament to allow for future change, rather than hampering that future change.

It's far easier to refactor, maintain, test and reason about strict and limited implementations than to do so with dynamic, runtime-magically-changing implementations.

I found this approach works best with languages having method overloading. For PHP it felt quite limiting, and it also requires you to have more complexity and overhead with wrapping.

But I have no hard evidence at hand, only how I experienced that in PHP.

I'd be curious to read about what percentage of active PHP devs use the recent features. The last time I worked in a PHP codebase (2020?) was half PHP 5 (bad) and half PHP 7 (much nicer). Curious if there's any real info out there on this

php 7 has been released 9 years ago.

Yeah, and I just finished porting an enormous amount of production code from PHP 5 to 7.x before fully moving it to 8. There are so many breaking changes in each major version, when you have a lot of live projects and clients don't have the budget to pay you to upgrade them, they can lay stagnant for years until way past EOL. It would have been nice to know, for instance, that future versions of PHP would throw warnings about undeclared variables or unaccessible named properties of "arrays" - which could previously be relied upon to be false-ish. That's a major pain point in code bases that treated arrays as simply dynamic objects that could be checked or defined at will. Lots of isset() and !empty() and other BS. Fine, but it takes time to sit down and check it all. I really preferred it when it let you just screw up or try to access a null property or define a variable inside a block and access it later without throwing any errors at all. Nothing about its actual functionality has changed in that regard; it's just errors you have to suppress or be more verbose to get around. In PHP 8 you can still do this:

`if ($a) { $previouslyUndefined = 2; } if ($previouslyUndefined) { echo "yeah"; }`

PHP still knows what $previouslyUndefined is or isn't at the second if statement, but it'll throw an error now in the first statement if you hadn't declared it outside the block. Why? Who cares? Scope in PHP is still understood to be inline, not in block; there is no equivalent to let vs var in JS. Stop telling me where I can check a variable if you're not enforcing various kinds of scope.

Your $previouslyUndefined thing as something that's changed, as far as I know, isn't true? Unless I've missed some very recent change.

If $a is true, that snippet will just execute with no errors. If $a is false you'll get a warning trying to check $previouslyUndefined in the second if. That behavior's been the same for a very long time. The blocks don't matter for scope but the fact that you never executed the line that would have defined the variable does.

Similarly, warnings on accessing array keys that don't exist, that's been a thing forever too. Pretty sure both go back with the same behavior to PHP 4, and probably earlier.

>Why? Who cares?

I do, if I typoed previouslyUndefined the first time. I get it adds boilerplate, but it also catches stupid bugs

The Laravel ecosystem folks seem to be always up to date in recent PHP developments. At least, that's my impression.

Symfony also does a great job adding polyfills way ahead of a PHP release , eg https://github.com/symfony/polyfill-php84

Yes, and this is incredibly annoying. Many packages add them as a dependency, and then you get subtle bugs because of it. Or worse, they add a dependency for the polyfill that is related to an extension and suffer performance issues when the extensions are not installed; yet no warning is output.

Drupal is very hot on attributes, has fiber support, uses readonly (a PHP 8.2 feature) extensively, enums are used -- overall it's fairly up to date.

PHP 5 is as close to phased out as it gets at this point. No doubt it's still in a lot of legacy enterprise codebases (lots of breaking changes going from 5 to 7 or 8), but outside of that no one is using it.

I work on multiple projects from PHP 5.2 to PHP 8.3 and everything in between.

Statistics based on packagist(composer)

https://stitcher.io/blog/php-version-stats-january-2024

Statistics based on web servers in the wild

https://w3techs.com/technologies/details/pl-php

https://w3techs.com/technologies/history_details/pl-php

Somehow people seem to be missing the fact that this would've been an opt-in feature.

> A way to bake that in without breaking old code would be to allow function definition to put an explicit stop to the argument list, ...

It might be too big a change on a language level given this has been in since forever, but it might be picked up by static analysis / a linter. I'd argue it's always better to have additional protections like this in a linter as the process of adding linter rules is easier and less impactful than making a language change.

It's also always preferred to not add anything to the language imo; in this case, I'd opt to have the interpreter emit a warning or info message. It's not broken, it's a developer error.

Indeed on the PHP internals mailing list some people were saying that it would be better to entirely deprecate passing extra arguments to a non-variadic function, without adding syntax/keyword to the language.

I think this would be nice ergonomically, from a coding perspective, but I'm curious as to how it would be a security threat to pass too many arguments. What's the potential exploit here?

Exploit I don't know, but as any stricter type verification, it would catch some bugs for sure. Note that builtin functions already throw an ArgumentCountError when passing fewer OR more parameters than the signature allows. My proposal consists in (optionally in a first place) make this behavior consistent for user-defined functions.

The trouble for me, where the rubber meets the road, is external API calls that spread their arguments into a PHP function that takes a bunch of args. So I would love a way to detect if they're sending too many, which I don't think currently exists (?) but not at the expense of breaking the API if they actually do send too many.

I don’t really understand the issue. Already if you have a mismatch, the only way you’d ever know is through static analysis. It will run and maybe crash during run time. I always joke that changing a function signature is the single most risky thing you can do in php (especially if you have any dynamic dispatch). Making it even more risky isn’t the right answer, IMHO.

Oh, and doing this would literally break class autoloading in symfony, and even the engine itself, which relies on this feature.

> the only way you’d ever know is through static analysis

Not for builtin PHP functions which already throw errors on arity mismatch.

> this would literally break class autoloading in symfony, and even the engine itself, which relies on this feature

I don't understand. Could you point to where in the Symfony code it relies on being able to wrongly call a function with more arguments than it expects and will use?

For variadic functions there is the ... operator already in the language since version 5.6, and my proposal wouldn't break that. Also note that builtin functions already emit deprecated warning in PHP 8 when called with too many arguments.

Here is the full thread discussing it on the PHP internal mailing list: https://news-web.php.net/php.internals/122928

You should look at `func_get_args()` usage in the wild. This is sometimes used for (mostly outdated) good-enough reasons and doing this might break it?

I know of func_get_args, but proper variadic functions have been a thing since PHP 5.6 (released more than 10 years ago) using the ... operator. Also, my initial proposal doesn't break existing code :).

Variadic functions serve a purpose but also change how the engine parses arguments. func_get_args is faster and more efficient.

IIRC, that's how symphony and other php frameworks do dependency injection.

> Not for builtin PHP functions which already throw errors on arity mismatch.

It still only happens during run time. Having a never-called function with an incorrect number of arguments is not an error.

Of course! And I strongly agree that static analysis is a good thing. But PHP still is a dynamically typed language and most of its development and usage is done with this dynamic approach in mind. It's not a XOR either, we can have better dynamic error reporting AND develop better static analysis tools at the same time.

Also, due to the nature and usage of PHP, some things cannot be statically analyzed because they're inherently dynamic. A simple example would be MVC frameworks where the routing is done like so: /controller/action/param1/param2/param3 where "controller" references a class and "action" references a method, which will take the "paramN" as arguments through the splicing of an array: `$ctrl->$actn(...$args);`. In such situations it would be nice to have errors/exceptions raised automatically if the URL is wrong (not enough OR too much arguments) without having to manually assess everything inside each method. Since PHP 7 and 8 over we're moving away from long lines of isset() and !empty() (and verifications such as is_numeric() etc. thanks to argument typing).

To be specific about static analysis: Lots of tools catch this. Sure, making some checks native would be nice, but for instance PHPStan always catches this, and more.

Regardless of the ‘improve the language angle’: Is somebody isn’t running PHPStan (or Psalm, Sonar, etc), then they’re missing out.

PHPStan is currently so good that using it should be non-negiotable. So the question would then even be: “I’d like rule 123 of the tool to be native, we helps with the RFC?”

I find these tools to not be too useful. The things they catch is like trying to force it to be an entirely different language, and don’t actually help with maintaining code. Then again, I’m usually writing low-level libraries that take advantage of quirks in the language for performance:

    $a = &$arr[]

Gives you a reference to null and appends it to the array. Then you can pass $a to something to mutate the tail “from a distance”. Most people never need this, nor should they use it. But when you are writing a streaming parser, it is quite handy as a one-liner instead of writing

    $a = null
    $arr[] = &$a

or keeping track of the current index and dealing with off-by-one errors.

For applications, these static analysis tools are great. For libraries, not so much.

Reply on news.ycombinator.com