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

Array unions are improperly collapsed #3801

Closed
gnarlyquack opened this issue Aug 31, 2020 · 15 comments
Closed

Array unions are improperly collapsed #3801

gnarlyquack opened this issue Aug 31, 2020 · 15 comments

Comments

@gnarlyquack
Copy link

Bug report

array{a, b}|array{c, d} is improperly collapsed as array{a|c, b|d}

Code snippet that reproduces the problem

https://phpstan.org/r/2fe252ba-d874-4f42-9164-d5672993d555

Expected output

Line 10: No error
Line 30: Function do_foo() should return array(true, null)|array(null, true) but returns ....
Actually, something (else) weird seems to be going on in line 30 since the reported "actual" type doesn't seem to make any sense.

@gnarlyquack
Copy link
Author

Actually, this seems to be an issue with array unions in general:
https://phpstan.org/r/794c728c-a1a8-4dee-8c80-42fcd9907ac8
I would expect an error on line 8 but, as can be seen on line 17, PHPStan is ok with a mixed array being returned.

I think my original message may have misemphasized the issue: the concern isn't just that the returned array type isn't deduced from the typecheck, but that an error isn't raised when an array of incorrect type is returned:
https://phpstan.org/r/9dbdde14-c59b-41db-a54b-7fdee611037a
I would expect errors on lines 15 and 19.

@gnarlyquack gnarlyquack changed the title Unions of array shapes are improperly collapsed Array unions are improperly collapsed Sep 1, 2020
@ondrejmirtes
Copy link
Member

Hi, just a heads-up - I consider this a feature request, with such complex needs you'd be much better served modeling your types as objects.

@gnarlyquack
Copy link
Author

@phpstan-bot
Copy link
Contributor

@gnarlyquack PHPStan now reports different result with your code snippet:

@@ @@
 45: Function foo\bar() never returns foo\Error<array<string>> so it can be removed from the return typehint.
-65: Function foo\bar() should return foo\Error<array<string>>|foo\Success<bool> but returns foo\Error<array<int, string>>.
+65: Function foo\bar() should return foo\Error<array<string>>|foo\Success<bool> but returns foo\Error<array(string)>.
Full report
Line Error
45 Function foo\bar() never returns foo\Error<array<string>> so it can be removed from the return typehint.
65 `Function foo\bar() should return foo\Error<array>

@phpstan-bot
Copy link
Contributor

@gnarlyquack PHPStan now reports different result with your code snippet:

@@ @@
 45: Function foo\bar() never returns foo\Error<array<int, string>> so it can be removed from the return typehint.
-65: Function foo\bar() should return foo\Error<array<int, string>>|foo\Success<bool> but returns foo\Error<array<int, string>>.
+65: Function foo\bar() should return foo\Error<array<int, string>>|foo\Success<bool> but returns foo\Error<array(string)>.
Full report
Line Error
45 Function foo\bar() never returns foo\Error<array<int, string>> so it can be removed from the return typehint.
65 `Function foo\bar() should return foo\Error<array<int, string>>

@Seldaek
Copy link
Contributor

Seldaek commented May 28, 2021

Another example for this feature request https://phpstan.org/r/14283564-cafe-450f-a734-ab6188da40d8 - bit me while trying to type the write() function here https://github.com/Seldaek/monolog/blob/main/src/Monolog/Handler/AbstractProcessingHandler.php#L40 which accepts a record array with the addition of a formatted key.

Not a huge deal, I duplicated the record array in this one special instance, but would have been neat if it just worked :) And yes I can see the argument that records should be objects perhaps, but they've been arrays for 11+ years and there's lots of code working with that assumption.

@phpstan-bot
Copy link
Contributor

@gnarlyquack After the latest commit in dev-master, PHPStan now reports different result with your code snippet:

@@ @@
-45: Function foo\bar() never returns foo\Error<array<string>> so it can be removed from the return typehint.
-65: Function foo\bar() should return foo\Error<array<string>>|foo\Success<bool> but returns foo\Error<array<int, string>>.
+45: Function foo\bar() never returns foo\Error<array<string>> so it can be removed from the return type.
+65: Function foo\bar() should return foo\Error<array<string>>|foo\Success<bool> but returns foo\Error<array(string)>.
Full report
Line Error
45 Function foo\bar() never returns foo\Error<array<string>> so it can be removed from the return type.
65 `Function foo\bar() should return foo\Error<array>

@phpstan-bot
Copy link
Contributor

@gnarlyquack After the latest commit in dev-master, PHPStan now reports different result with your code snippet:

@@ @@
-45: Function foo\bar() never returns foo\Error<array<int, string>> so it can be removed from the return typehint.
-65: Function foo\bar() should return foo\Error<array<int, string>>|foo\Success<bool> but returns foo\Error<array<int, string>>.
+45: Function foo\bar() never returns foo\Error<array<int, string>> so it can be removed from the return type.
+65: Function foo\bar() should return foo\Error<array<int, string>>|foo\Success<bool> but returns foo\Error<array(string)>.
Full report
Line Error
45 Function foo\bar() never returns foo\Error<array<int, string>> so it can be removed from the return type.
65 `Function foo\bar() should return foo\Error<array<int, string>>

@phpstan-bot
Copy link
Contributor

@gnarlyquack After the latest commit in dev-master, PHPStan now reports different result with your code snippet:

@@ @@
 10: Parameter #1 $flag of function do_bar expects bool, true|null given.
-30: Function do_foo() should return array(true|null, true|null) but returns array(null, int<1, max>).
+30: Function do_foo() should return array{true|null, true|null} but returns array{null, int<1, max>}.
Full report
Line Error
10 `Parameter #1 $flag of function do_bar expects bool, true
30 `Function do_foo() should return array{true

@phpstan-bot
Copy link
Contributor

@gnarlyquack After the latest commit in dev-master, PHPStan now reports different result with your code snippet:

@@ @@
-45: Function foo\bar() never returns foo\Error<array<string>> so it can be removed from the return typehint.
-65: Function foo\bar() should return foo\Error<array<string>>|foo\Success<bool> but returns foo\Error<array<int, string>>.
+45: Function foo\bar() never returns foo\Error<array<string>> so it can be removed from the return type.
+65: Function foo\bar() should return foo\Error<array<string>>|foo\Success<bool> but returns foo\Error<array{string}>.
Full report
Line Error
45 Function foo\bar() never returns foo\Error<array<string>> so it can be removed from the return type.
65 `Function foo\bar() should return foo\Error<array>

@phpstan-bot
Copy link
Contributor

@gnarlyquack After the latest commit in dev-master, PHPStan now reports different result with your code snippet:

@@ @@
-45: Function foo\bar() never returns foo\Error<array<int, string>> so it can be removed from the return typehint.
-65: Function foo\bar() should return foo\Error<array<int, string>>|foo\Success<bool> but returns foo\Error<array<int, string>>.
+45: Function foo\bar() never returns foo\Error<array<int, string>> so it can be removed from the return type.
+65: Function foo\bar() should return foo\Error<array<int, string>>|foo\Success<bool> but returns foo\Error<array{string}>.
Full report
Line Error
45 Function foo\bar() never returns foo\Error<array<int, string>> so it can be removed from the return type.
65 `Function foo\bar() should return foo\Error<array<int, string>>

@phpstan-bot
Copy link
Contributor

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

@@ @@
 10: Parameter #1 $flag of function do_bar expects bool, true|null given.
-30: Function do_foo() should return array(true|null, true|null) but returns array(null, int<1, max>).
+30: Function do_foo() should return array{null, true}|array{true, null} but returns array{null, int<1, max>}.
Full report
Line Error
10 `Parameter #1 $flag of function do_bar expects bool, true
30 `Function do_foo() should return array{null, true}

@phpstan-bot
Copy link
Contributor

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

@@ @@
-No errors
+ 7: Function do_foo() never returns array{null, bool} so it can be removed from the return type.
+15: Function do_foo() should return array{bool, null}|array{null, bool} but returns array{false, true}.
+19: Function do_foo() should return array{bool, null}|array{null, bool} but returns array{false, false}.
Full report
Line Error
7 Function do_foo() never returns array{null, bool} so it can be removed from the return type.
15 `Function do_foo() should return array{bool, null}
19 `Function do_foo() should return array{bool, null}

@gnarlyquack
Copy link
Author

Hi,

Thanks for working on this.

This appears to fix the specific error on line 30 in the original code snippet, but in general this still doesn't seem to be working correctly:

https://phpstan.org/r/794c728c-a1a8-4dee-8c80-42fcd9907ac8
Line 17 is now being properly flagged, but line 8 isn't. It's allowing a mixed array of ints and strings to be returned, when it should either be an array of ints or an array of strings.

Same thing here:
https://phpstan.org/r/9dbdde14-c59b-41db-a54b-7fdee611037a
Lines 15 and 19 should both be errors.

I also see that in line 10 of the original snippet, the type of $result is still not being correctly determined. If $error isn't set (i.e., null), then $result must be a bool, so there shouldn't be an error.
https://phpstan.org/r/9ebf73e8-cf62-4339-8779-710e2ba048eb

@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 Oct 29, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants