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

UnsafeForeachArrayOfStringRule doesn't report BenevolentUnionType of (int|string) (the default for unspecified array keys) #6534

Closed
dktapps opened this issue Nov 25, 2024 · 0 comments
Labels
Category: Core Related to internal functionality Category: Tests Pertaining to unit / integration tests Resolution: Fixed

Comments

@dktapps
Copy link
Member

dktapps commented Nov 25, 2024

Issue description

BenevolentUnionType is a very annoying PHPStan feature that suppresses errors when the union is passed to a type that only accepts a subset of its subtypes.

e.g.:

  • int|string is accepted by int|string but not by int and not by string
  • (int|string) is accepted by int|string, int, and string

This is a problem because (int|string) is the default array key type when doing @var SomeType[], meaning that dozens of potential crash points haven't been reported.

This is because UnsafeForeachArrayOfStringRule doesn't distinguish between explicit types and implicit ones.

Annoyingly, the places where BenevolentUnionType has suppressed these errors are the exact places we'd be most concerned about crashes.

OS and versions

  • PocketMine-MP: 5.21.1

Crashdump, backtrace or other files

The following is a list of errors that pop up when UnsafeForeachArrayOfStringRule is modified to account for BenevolentUnionType. Unfortunately, many of these are quite annoying to fix, because Utils::stringifyKeys() can't infer types from BenevolentUnionType. @var is also difficult to use in many such cases as is_array() may overwrite info from @var.

output.txt

@dktapps dktapps added Category: Core Related to internal functionality Status: Debugged Cause of the bug has been found, but not fixed Category: Tests Pertaining to unit / integration tests labels Nov 25, 2024
@dktapps dktapps added Resolution: Fixed and removed Status: Debugged Cause of the bug has been found, but not fixed labels Nov 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Category: Core Related to internal functionality Category: Tests Pertaining to unit / integration tests Resolution: Fixed
Projects
None yet
Development

No branches or pull requests

1 participant