Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

max()/min() should expect non-empty-array when called with a single arg #7239

Closed
staabm opened this issue May 14, 2022 · 9 comments
Closed

Comments

@staabm
Copy link
Contributor

staabm commented May 14, 2022

Feature request

https://phpstan.org/r/c16b4e78-df26-405b-b0cb-3fe4043c477c

it should report a error when max() min() is invoked with a empty array

https://3v4l.org/OE4CD
https://3v4l.org/vhUXS

@ondrejmirtes
Copy link
Member

Might be possible with a conditional parameter type 😂

@staabm
Copy link
Contributor Author

staabm commented May 18, 2022

if I understand ondrey correctly, we would aim for something like
@param (func_num_args() is 1 ? non-empty-array : ...mixed) $values
which requires support for func_num_args() in conditional parameters.

psalm example

@rvanvelzen is this something on your roadmap?

@ondrejmirtes
Copy link
Member

I don't think we need that. We can continue using extensions for situations that are more complicated.

@staabm
Copy link
Contributor Author

staabm commented May 18, 2022

Just to get you right: do you still think this is possible with a conditional parameter?

Or should I just implement a extension

@ondrejmirtes
Copy link
Member

Oh sorry, I thought that was about another issue. You can't influence parameter types with an extension, so the conditional parameter type is an only option. But I don't know to solve this scenario right now, better to focus on something else probably :)

@hemberger
Copy link

hemberger commented Jan 1, 2023

Is this considered the same issue as min([]) and max([]) having an inferred type of false for PHP 8+, even though this is no longer true as of PHP 8.0.0?

max() throws a ValueError on failure now; previously, false was returned and an E_WARNING error was emitted.

It's a bit misleading, since it suggests that the error might be mitigated by special handling of the return value, rather than the input. If there's simply no better solution at the moment, I can certainly understand that - I just wanted to make sure I shouldn't be reporting this as a separate issue.

https://phpstan.org/r/9990fa9a-455b-412b-9995-9e5f5ec6fe77

Thanks for your time!

@phpstan-bot
Copy link
Contributor

@hemberger After the latest push in 1.9.x, PHPStan now reports different result with your code snippet:

@@ @@
+PHP 8.0 – 8.2 (3 errors)
+==========
+
+4: Dumped type: int
+7: Dumped type: int
+9: Dumped type: *ERROR*
+
+PHP 7.1 – 7.4 (3 errors)
+==========
+
 4: Dumped type: int
 7: Dumped type: int|false
 9: Dumped type: false
Full report

PHP 8.0 – 8.2 (3 errors)

Line Error
4 Dumped type: int
7 Dumped type: int
9 Dumped type: *ERROR*

PHP 7.1 – 7.4 (3 errors)

Line Error
4 Dumped type: int
7 `Dumped type: int
9 Dumped type: false

@ondrejmirtes
Copy link
Member

Done with bleedingEdge already.

@github-actions
Copy link

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 14, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants