Skip to content

Conversation

VincentLanglet
Copy link
Contributor

@VincentLanglet VincentLanglet commented May 24, 2025

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
@VincentLanglet
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.

@VincentLanglet
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
@phpstan-bot
Copy link
Collaborator

This pull request has been marked as ready for review.

@VincentLanglet
Copy link
Contributor Author

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

@ondrejmirtes
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?

@VincentLanglet
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

@ondrejmirtes
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.

@VincentLanglet
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
@phpstan-bot
Copy link
Collaborator

This pull request has been marked as ready for review.

@ondrejmirtes
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.

@VincentLanglet
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 ?

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

Labels

None yet

Projects

None yet

3 participants