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

Improve isOffsetAccessible->no #8393

Closed
rajyan opened this issue Nov 19, 2022 · 2 comments · Fixed by phpstan/phpstan-src#3045
Closed

Improve isOffsetAccessible->no #8393

rajyan opened this issue Nov 19, 2022 · 2 comments · Fixed by phpstan/phpstan-src#3045

Comments

@rajyan
Copy link
Contributor

rajyan commented Nov 19, 2022

Feature request

https://3v4l.org/b3Vem
As we can see here, there are two types of isOffsetAccessible->no. Whether it throws a fatal error in isset/coalesce/empty ($scope->isUndefinedExpressionAllowed) or not.
I think we can improve NonexistentOffsetInArrayDimFetchRule by separating these.

Also, a lot of people are not aware why StrictMixedType throws an error with the code below, maybe we can add some information in the document

/** @var mixed $mixed */
isset($mixed['foo']);

Goal

https://phpstan.org/r/1087a148-ae5b-477f-9003-215fefc3437f
No error in line 7, 21
Throw error in 12

Design

  • Add method something like isOffsetAccessThrowsFatalError(): TernaryLogic (want a better naming...) to Types
  • Return TernaryLogic::Yes for isOffsetAccessThrowsFatalError in MixedType/StrictMixedType if object type is subtracted

Related issues

These issues should be closed after documented

The test should be reverted after this is implemented

Array access with json_decode mixed problems

Did PHPStan help you today? Did it make you happy in any way?

PHPStan on PHPStan finds error while developing PHPStan 😄

@cs278
Copy link
Contributor

cs278 commented Dec 7, 2022

Recently stumbled upon the issue described in #8388. Which either needs me to ignore the error or chain a bunch of isset() and is_array() calls together.

My understanding is that objects not implementing ArrayAccess cause fatal errors even inside an isset() (the same also applies to null coalesce): https://3v4l.org/QgpEg

Which is a fun PHP "feature" I'll be honest I'd totally forgotten about, so it's great that PHPStan provides protection from this particular footgun. Are there any other cases I'm not aware of?

However, PHPStan is not consistent in doing this which IMO is worse than the PHP footgun, see these examples: https://phpstan.org/r/14363150-f65d-4748-a9df-1bf733c20cf6

So I'd definitely support a change to add a separate check to determine if a fatal error is possible and making this behaviour consistent, perhaps isOffsetAccessLegal().

Is it possible in the meantime to make PHPStan's behaviour consistent?

@rajyan
Copy link
Contributor Author

rajyan commented Dec 7, 2022

In your example, 'object' is too wide and PHPStan can only say it's isOffsetAccessible->maybe (which wouldn't throw an error in isset nor coalesce).

FYI PHPStan can handle ArrayAccess objects correctly, if the class is "final"
https://phpstan.org/r/cf338470-beef-4d50-b57a-5f44bccd002c

The difficult part with the current implementation is that NonArrayObject and bool are both isOffsetAccessible->no and treated the same way, but in isset or coalesce this behavior is different. The purpose of this feature request is to separate these ->no
the idea is from #8388 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants