Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Introduce allowFloatBoolNullAsArrayKey parameter #4012

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

Open
VincentLanglet wants to merge 3 commits into phpstan:2.1.x
base: 2.1.x
Choose a base branch
Loading
from VincentLanglet:reportCastedArrayKey

Conversation

@VincentLanglet
Copy link
Contributor

@VincentLanglet VincentLanglet commented May 24, 2025
edited
Loading

Closes phpstan/phpstan#12589
Closes phpstan/phpstan#7884
Closes phpstan/phpstan#7864

Naming is opened to discussion.

I think we could enable this in phpstan-strict-rules.

Copy link
Member

@ondrejmirtes ondrejmirtes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also would be nice if it observed rule levels. Always wrong type like null to be always reported, but a partially wrong type like string|stdClass only on level 7+...

This is achieved with RuleLevelHelper::findTypeToCheck.

@VincentLanglet VincentLanglet force-pushed the reportCastedArrayKey branch 2 times, most recently from 692ddc0 to a5aea3c Compare July 17, 2025 20:08
Copy link
Contributor Author

Seems like the RuleLevelHelper::findTypeToCheck was not used at all (or wrongly used) on theses rules.

So I fixed it and added tests with and without the reportArrayKeyCast option.

Copy link
Contributor Author

Current status, waiting #4166 first

@VincentLanglet VincentLanglet changed the title (削除) Introduce reportCastedArrayKey parameter (削除ここまで) (追記) Introduce reportArrayKeyCast parameter (追記ここまで) Sep 10, 2025
@VincentLanglet VincentLanglet marked this pull request as ready for review September 10, 2025 18:00
Copy link
Collaborator

This pull request has been marked as ready for review.

Copy link
Contributor Author

Friendly ping @ondrejmirtes ; I would be interested enabling this on phpstan-strict-rules :)

Copy link
Member

So now this is only about the boolean. I don't think it's worth it adding a new config parameter just for that. Maybe it'd be better to advocate for deprecating that in PHP 8.6?

Copy link
Contributor Author

So now this is only about the boolean. I don't think it's worth it adding a new config parameter just for that.

It's for boolean or people which are using PHP < 8.5 (8.1) and still want the array key casted deprecated.
Since PHP supports 7.4 it might have some benefit.

Also I feel like waiting for 8.6 might be a little bit long for an important bug like the example of phpstan/phpstan#12589 ...

Maybe it'd be better to advocate for deprecating that in PHP 8.6?

It's would still be a good idea but I have no knowledge in C to implement this in PHP codebase

Copy link
Member

So deprecating bools was considered but rejected in the RFC process. Here's the comment and some discussion above it https://externals.io/message/127849#128184.

Copy link
Contributor Author

So deprecating bools was considered but rejected in the RFC process. Here's the comment and some discussion above it externals.io/message/127849#128184.

Then based on the fact it will be unlikely deprecated soon on PHP side, do you confirm that such options still make sens on PHPStan side ?

@VincentLanglet VincentLanglet marked this pull request as ready for review October 8, 2025 17:01
Copy link
Collaborator

This pull request has been marked as ready for review.

Copy link
Member

I don't like the name of the config parameter. You're not reporting array key casts. For example if someone uses '1', you're not reporting that although it's still cast to 1.

This is more like allowFloatBoolNullAsArrayKey with default set to true.

Copy link
Contributor Author

This is more like allowFloatBoolNullAsArrayKey with default set to true.

I renamed with allowFloatBoolNullAsArrayKey, but have some suggestions:

  • allowBoolFloatNullAsArrayKey => Type are alphabetically ordered
  • allowLooseArrayKeys
  • strictArrayKeyTypes (default false)

Any preferences ?

Copy link
Contributor Author

Just found the issue phpstan/phpstan#7564 and I wonder if it should be included in this option or in a separate one.

If so, rather than introducing allowFloatBoolNull param in AllowedArrayKeysTypes it could be something like checking if

$type->toArrayKeyType()->isSuperTypeOf($type)->yes();

(to avoid reporting numeric-string array keys)

@VincentLanglet VincentLanglet changed the title (削除) Introduce reportArrayKeyCast parameter (削除ここまで) (追記) Introduce allowFloatBoolNullAsArrayKey parameter (追記ここまで) Oct 19, 2025
Copy link
Contributor Author

This is more like allowFloatBoolNullAsArrayKey with default set to true.

I renamed with allowFloatBoolNullAsArrayKey, but have some suggestions:

  • allowBoolFloatNullAsArrayKey => Type are alphabetically ordered
  • allowLooseArrayKeys
  • strictArrayKeyTypes (default false)

Any preferences ?

Maybe you'll have some suggestion about the naming @staabm ?

Copy link
Contributor

staabm commented Oct 22, 2025
edited
Loading

I would go with one of the allowFloatBoolNullAsArrayKey variants, as otherwise we need to document somewhere what "loose" or "strict" means in this context

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

Reviewers

@ondrejmirtes ondrejmirtes Awaiting requested review from ondrejmirtes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

AltStyle によって変換されたページ (->オリジナル) /