-
Notifications
You must be signed in to change notification settings - Fork 529
Fix hasOffsetValueType for arrays #4277
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
Fix hasOffsetValueType for arrays #4277
Conversation
@@ -1801,7 +1801,7 @@ public static function dataProperties(): array | |||
'$this->resource', | |||
], | |||
[ | |||
'*ERROR*', | |||
'mixed', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change revert the one in #4272
@@ -193,10 +193,6 @@ public function testStrings(): void | |||
'Offset 12.34 might not exist on array|string.', | |||
28, | |||
], | |||
[ | |||
'Offset int|object might not exist on array|string.', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This error was added by #4272
It make sens to not have it since
- int is a valid offset and produce no error
- object is not a valid offset (and produce a php crash) and is already reported by the InvalidArrayOffsetRule.
@@ -932,10 +928,6 @@ public function testBugObject(): void | |||
'Offset int|object does not exist on array{baz: 21}|array{foo: 17, bar: 19}.', | |||
12, | |||
], | |||
[ | |||
'Offset object does not exist on array<string, int>.', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same, already reported by InvalidArrayOffsetRule and this was added by #4272
c66a340
to
8ff0adb
Compare
The 7.4 failure already exists on 2.1.x https://github.com/phpstan/phpstan-src/actions/runs/17398668843/job/49386473327 |
This pull request has been marked as ready for review. |
@@ -18,10 +18,5 @@ | |||
"message": "Invalid array key type stdClass.", | |||
"line": 59, | |||
"ignorable": true | |||
}, | |||
{ | |||
"message": "Offset stdClass does not exist on array{baz: 21}|array{foo: 17, bar: 19}.", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We're losing these useful errors. What are they replaced with?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is already (above)
Invalid array key type stdClass.
Thank you! |
I feel like we should only check for AllowedArrayKeysTypes since others types are checked by InvalidArrayKey rule.