Introduce allowFloatBoolNullAsArrayKey parameter

[VincentLanglet]

Closes https://github.com/phpstan/phpstan/issues/12589 Closes https://github.com/phpstan/phpstan/issues/7884 Closes https://github.com/phpstan/phpstan/issues/7864

Naming is opened to discussion.

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

[VincentLanglet]

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]

Current status, waiting https://github.com/phpstan/phpstan-src/pull/4166 first

[phpstan-bot]

This pull request has been marked as ready for review.

[VincentLanglet]

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

[ondrejmirtes]

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]

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 https://github.com/phpstan/phpstan/issues/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]

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]

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 ?

[phpstan-bot]

This pull request has been marked as ready for review.

[ondrejmirtes]

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]

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 ?

[VincentLanglet]

Just found the issue https://github.com/phpstan/phpstan/issues/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]

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 ?

[staabm]

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

[VincentLanglet]

Any way to move this forward @ondrejmirtes ?

Do you prefer allowBoolFloatNullAsArrayKey to have the types sorted alphabetically ?