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.ArrayIndent has conflicting logic #3100

Closed
morozov opened this issue Sep 5, 2020 · 0 comments · Fixed by #3101
Closed

Generic.Arrays.ArrayIndent has conflicting logic #3100

morozov opened this issue Sep 5, 2020 · 0 comments · Fixed by #3101

Comments

@morozov
Copy link
Contributor

morozov commented Sep 5, 2020

Describe the bug
The fix for #2882 (0425342) introduces conflicting logic in the Generic.Arrays.ArrayIndent sniff.

Code sample

<?php

public function test()
{
    return [
        [
            'foo'
                . 'bar',
            [
                    'baz',
                    'qux',
                ],
        ],
    ];
}

Custom ruleset
None

To reproduce
Steps to reproduce the behavior:

  1. Check the file above by running phpcs -s --standard=Squiz --sniffs=Generic.Arrays.ArrayIndent test.php:
    FILE: /home/morozov/Projects/phpcs-playground/test.php
    ------------------------------------------------------------------------------------------------
    FOUND 1 ERROR AFFECTING 1 LINE
    ------------------------------------------------------------------------------------------------
     9 | ERROR | [x] Array open brace not indented correctly; expected at least 16 spaces but found
       |       |     12 (Generic.Arrays.ArrayIndent.OpenBraceIncorrect)
    ------------------------------------------------------------------------------------------------
    PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
    ------------------------------------------------------------------------------------------------
    
    Time: 46ms; Memory: 8MB
    
  2. Try fixing the violation manually:
    --- a/test.php
    +++ b/test.php
    @@ -6,7 +6,7 @@ public function test()
             [
                 'foo'
                     . 'bar',
    -            [
    +                [
                         'baz',
                         'qux',
                     ],
  3. Check the file again by running phpcs -s --standard=Squiz --sniffs=Generic.Arrays.ArrayIndent test.php:
    FILE: /home/morozov/Projects/phpcs-playground/test.php
    ------------------------------------------------------------------------------------------------
    FOUND 1 ERROR AFFECTING 1 LINE
    ------------------------------------------------------------------------------------------------
     9 | ERROR | [x] Array key not indented correctly; expected 12 spaces but found 16
       |       |     (Generic.Arrays.ArrayIndent.KeyIncorrect)
    ------------------------------------------------------------------------------------------------
    PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
    ------------------------------------------------------------------------------------------------
    
    Time: 48ms; Memory: 8MB
    
  4. Try auto-fixing the file by running phpcbf --standard=Squiz --sniffs=Generic.Arrays.ArrayIndent test.php:
    PHPCBF RESULT SUMMARY
    ----------------------------------------------------------------------
    FILE                                                  FIXED  REMAINING
    ----------------------------------------------------------------------
    /home/morozov/Projects/phpcs-playground/test.php      FAILED TO FIX
    ----------------------------------------------------------------------
    A TOTAL OF 0 ERRORS WERE FIXED IN 1 FILE
    ----------------------------------------------------------------------
    PHPCBF FAILED TO FIX 1 FILE
    ----------------------------------------------------------------------
    
    Time: 116ms; Memory: 8MB
    

Expected behavior
There's no conflict, and the file is auto-fixed.

Versions (please complete the following information):

  • OS: Linux
  • PHP: 7.4.9
  • PHPCS: 0425342
  • Standard: Squiz
rodrigoprimo added a commit to rodrigoprimo/PHP_CodeSniffer that referenced this issue May 9, 2024
This commit removes T_COMMA from a list of tokens to ignore when
searching for the non-empty token before the start of the array. T_COMMA
was added to this list in 1697fac to fix a bug. See
squizlabs/PHP_CodeSniffer#3100 for more
details.

Back when this fix was introduced, it was necessary to ignore commas due
to a bug in findStartOfStatement() that would cause this method to
return the wrong token when passed the last token in a statement. This
bug has been fixed afterwards in ef80e53 and ignoring commas is not
necessary anymore. Now, the call to findStartOfStatement() in
https://github.com/squizlabs/PHP_CodeSniffer/blob/4cf6badaf0c177acaf295e2178f4383e2ea71b20/src/Standards/Generic/Sniffs/Arrays/ArrayIndentSniff.php#L67
correctly finds the start of the statement when passed the comma that
marks the end of the statement.

A test was added 1697fac. It would fail if running an older version of
PHPCS without ignoring commas, but now it passes:

https://github.com/squizlabs/PHP_CodeSniffer/pull/3101/files#diff-f786a9f6c41b82204dc89de2c02437c0e44401d580b02fcbde6278ea03f25693R83-R90
jrfnl pushed a commit to rodrigoprimo/PHP_CodeSniffer that referenced this issue May 14, 2024
This commit removes T_COMMA from a list of tokens to ignore when
searching for the non-empty token before the start of the array. T_COMMA
was added to this list in 1697fac to fix a bug. See
squizlabs/PHP_CodeSniffer#3100 for more
details.

Back when this fix was introduced, it was necessary to ignore commas due
to a bug in findStartOfStatement() that would cause this method to
return the wrong token when passed the last token in a statement. This
bug has been fixed afterwards in ef80e53 and ignoring commas is not
necessary anymore. Now, the call to findStartOfStatement() in
https://github.com/squizlabs/PHP_CodeSniffer/blob/4cf6badaf0c177acaf295e2178f4383e2ea71b20/src/Standards/Generic/Sniffs/Arrays/ArrayIndentSniff.php#L67
correctly finds the start of the statement when passed the comma that
marks the end of the statement.

A test was added 1697fac. It would fail if running an older version of
PHPCS without ignoring commas, but now it passes:

https://github.com/squizlabs/PHP_CodeSniffer/pull/3101/files#diff-f786a9f6c41b82204dc89de2c02437c0e44401d580b02fcbde6278ea03f25693R83-R90
jrfnl pushed a commit to PHPCSStandards/PHP_CodeSniffer that referenced this issue May 14, 2024
This commit removes T_COMMA from a list of tokens to ignore when
searching for the non-empty token before the start of the array. T_COMMA
was added to this list in 1697fac to fix a bug. See
squizlabs/PHP_CodeSniffer#3100 for more
details.

Back when this fix was introduced, it was necessary to ignore commas due
to a bug in findStartOfStatement() that would cause this method to
return the wrong token when passed the last token in a statement. This
bug has been fixed afterwards in ef80e53 and ignoring commas is not
necessary anymore. Now, the call to findStartOfStatement() in
https://github.com/squizlabs/PHP_CodeSniffer/blob/4cf6badaf0c177acaf295e2178f4383e2ea71b20/src/Standards/Generic/Sniffs/Arrays/ArrayIndentSniff.php#L67
correctly finds the start of the statement when passed the comma that
marks the end of the statement.

A test was added 1697fac. It would fail if running an older version of
PHPCS without ignoring commas, but now it passes:

https://github.com/squizlabs/PHP_CodeSniffer/pull/3101/files#diff-f786a9f6c41b82204dc89de2c02437c0e44401d580b02fcbde6278ea03f25693R83-R90
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant