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

Terminal comma to indicate discarded items in array destructuring fails Squiz.Arrays.ArrayDeclaration.CommaAfterLast #528

Open
3 of 4 tasks
joachim-n opened this issue Jun 24, 2024 · 5 comments

Comments

@joachim-n
Copy link
Contributor

joachim-n commented Jun 24, 2024

Describe the bug

Array destructuring is a syntax in PHP where items of an array are assigned to single variables, using an array construct on the left of the assignment, e.g.

[$foo, $bar, $baz] = $source_array;

It's a common pattern to include a terminal comma to indicate that we know not all of the array's elements are being assigned.

E.g. in Composer:

[$response, ] = $spec;
                    [$defaultUsername, ] = explode(':', $authParts, 2);

However, this fails the Squiz.Arrays.ArrayDeclaration.CommaAfterLast sniff:

124 | ERROR | [x] Comma not allowed after last value in single-line array
     |       |     declaration (Squiz.Arrays.ArrayDeclaration.CommaAfterLast)

Code sample

<?php

[$alpha, ] = explode('/', 'alpha/beta');

To reproduce

Steps to reproduce the behavior:

  1. Create a file called test.php with the code sample above...
  2. Run phpcs --standard=Squiz --sniffs=Squiz.Arrays.ArrayDeclaration test.php
  3. See error message displayed
 3 | ERROR | [x] Comma not allowed after last value in single-line array declaration

Expected behavior

A clear and concise description of what you expected to happen.

Versions (please complete the following information)

Operating System MacOS 10.15
PHP version 8.3
PHP_CodeSniffer version 3.10.1
Standard Squiz
Install type Composer

Additional context

Add any other context about the problem here.

Please confirm

  • I have searched the issue list and am not opening a duplicate issue.
  • I have read the Contribution Guidelines and this is not a support question.
  • I confirm that this bug is a bug in PHP_CodeSniffer and not in one of the external standards.
  • I have verified the issue still exists in the master branch of PHP_CodeSniffer.
@jrfnl
Copy link
Member

jrfnl commented Jun 24, 2024

@joachim-n Thank you for reporting this. This is due to the sniff incorrectly acting on short lists.

The Squiz.Arrays.ArrayDeclaration sniff is known to be problematic and has been for years.

I've been building up a (highly configurable) NormalizedArrays standard in PHPCSExtra to replace it, but that's not complete yet. Would be lovely if I could find some time to finish that at some point.

Having said that, you could consider excluding the Squiz.Arrays.ArrayDeclaration.CommaAfterLast error code and using the PHPCSExtra NormalizedArrays.Arrays.CommaAfterLast sniff to replace it. That sniff does ignore short lists correctly.

@jrfnl
Copy link
Member

jrfnl commented Jun 24, 2024

Loosely related to #527

@joachim-n
Copy link
Contributor Author

Thanks for the super quick response!

I've passed on the advice about using a different sniff to the issue in Drupal that brought me here - https://www.drupal.org/project/coder/issues/3456683

@jrfnl
Copy link
Member

jrfnl commented Sep 14, 2024

@joachim-n Can this issue be closed ?

@joachim-n
Copy link
Contributor Author

Probably, though if the replacement isn't ready yet, it might be an idea to add some docs to Squiz.Arrays.ArrayDeclaration to explain its limitations.

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