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

Generic.Arrays.DisallowShortArraySyntax creates fatal error #3709

Open
kkmuffme opened this issue Nov 7, 2022 · 3 comments
Open

Generic.Arrays.DisallowShortArraySyntax creates fatal error #3709

kkmuffme opened this issue Nov 7, 2022 · 3 comments

Comments

@kkmuffme
Copy link

kkmuffme commented Nov 7, 2022

Describe the bug

Generic.Arrays.DisallowShortArraySyntax.Found creates a fatal when it's an array assignment, where it should replace with list, not array

Code sample

[, $foo] = explode( '=', 'hello=world=bar', 2 );

Will be changed to:

array(, $foo) = explode( '=', 'hello=world=bar', 2 );

which is invalid and a fatal error.
If short array is followed by assignment (=), it should be changed to:

list(, $foo) = explode( '=', 'hello=world=bar', 2 );

Custom ruleset

<?xml version="1.0"?>
<ruleset name="My Custom Standard">
    <rule ref="Generic.Arrays.DisallowShortArraySyntax"/>
</ruleset>

To reproduce
use the above code sample with the custom ruleset and run phpcbf --standard=custom-ruleset.xml code-sample.php on it

Expected behavior

Change it to:

list(, $foo) = explode( '=', 'hello=world=bar', 2 );

Versions (please complete the following information):

  • OS: RHEL 8
  • PHP: 7.4
  • PHPCS: 3.7.1
  • Standard: single rule
@jrfnl
Copy link
Contributor

jrfnl commented Nov 7, 2022

This is a known issue and not easy to solve in PHPCS itself. Loosely related to #1984

There is an alternative for this sniff available in PHPCSExtra which does distinguish between short lists and short arrays correctly: Universal.Arrays.DisallowShortArraySyntax. I'd strongly suggest you switch to using that sniff instead.

As a side-note: the Universal rule collection also contains a Universal.Lists.DisallowShortListSyntax sniff which will correctly detect and fix short lists to long lists.

@kkmuffme
Copy link
Author

kkmuffme commented Nov 8, 2022

Thanks!

Could this issue get fixed in the way PHPCSExtra does?

I inherit this rule from WPCS, should I bring the issue up there too? (asking since you're contributing there too)

@jrfnl
Copy link
Contributor

jrfnl commented Nov 8, 2022

Could this issue get fixed in the way PHPCSExtra does?

The code needed to determine short array vs short list is not to be underestimated. It is in PHPCSUtils and is two classes by themselves just and only for that analysis and has a significant impact on performance (which is why PHPCSUtils has caching build around it as well), so no, this is not something which can be fixed in PHPCS itself "in the same way", at least not unless PHPCS adds a dependency on PHPCSUtils or the issues I raised related to this are fixed (see #1984).

I inherit this rule from WPCS, should I bring the issue up there too? (asking since you're contributing there too)

This has already been fixed in WPCS develop. As of WPCS 3.0.0, the Generic.Arrays.DisallowShortArraySyntax sniff will no longer be used and has been replaced with the Universal.Arrays.DisallowShortArraySyntax sniff.

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

No branches or pull requests

2 participants