Skip to content

fix: array declaration sniff #1104

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

Draft
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

Jurigag
Copy link

@Jurigag Jurigag commented May 24, 2025

Description

This adds two changes to array declaration sniff:

  • Removes continue when DoubleArrowNotAligned is triggered - currently with continue it simply doesn't fix array values as expected - it keeps whitespaces for it in case double arrow is not aligned as expected
  • Adds alignDoubleArrowToLongestIndex property to ArrayDeclarationSniff, with true value by defualt, when changed to false phpcs will align double arrow to length of index of element

Basically for example now when running phpcs for such array:

$array = [
  'asdasdasdsadsadsadassad'     =>        $this->greenhouseGas->id,
  'xyz'     =>      
            $this->xyz->asd,
  123     =>
    $this->xyz->asd,
  456     =>        $this->xyz->asd,
];

With alignDoubleArrowToLongestIndex value as true will produce output:

$array = [
  'asdasdasdsadsadsadassad' => $this->greenhouseGas->id,
  'xyz'                     => $this->xyz->asd,
  123                       => $this->xyz->asd,
  456                       => $this->xyz->asd,
];

With alignDoubleArrowToLongestIndex value as false will produce output:

$array = [
  'asdasdasdsadsadsadassad' => $this->greenhouseGas->id,
  'xyz' => $this->xyz->asd,
  123 => $this->xyz->asd,
  456 => $this->xyz->asd,
];

Suggested changelog entry

  • Add property alignDoubleArrowToLongestIndex to Squiz.Arrays.ArrayDeclaration to allow double arrow alignment without keeping double arrow on same position of all elements
  • Fixes Squiz.Arrays.ArrayDeclaration.ValueNotAligned not being reported when Squiz.Arrays.ArrayDeclaration.DoubleArrowNotAligned was reported for same array element

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
    • This change is only breaking for integrators, not for external standards or end-users.
  • Documentation improvement

PR checklist

  • I have checked there is no other PR open for the same change.
  • I have read the Contribution Guidelines.
  • I grant the project the right to include and distribute the code under the BSD-3-Clause license (and I have the right to grant these rights).
  • I have added tests to cover my changes.
  • I have verified that the code complies with the projects coding standards.
  • [Required for new sniffs] I have added XML documentation for the sniff.

@jrfnl
Copy link
Member

jrfnl commented May 26, 2025

@Jurigag Thanks for your willingness to contribute to PHPCS.

Just so you know: unless and until the CI builds pass, nobody will look at this PR and if the PR is not fixed up within a few days, it will be closed as inactionable (with an option to re-open when the code in the branch has been fixed up).

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

Successfully merging this pull request may close these issues.

2 participants