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

Conflict between Squiz.Arrays.ArrayDeclaration and ScopeIndent sniffs when heredoc used in array #2165

Closed
morozov opened this issue Sep 24, 2018 · 2 comments
Milestone

Comments

@morozov
Copy link
Contributor

morozov commented Sep 24, 2018

The issue is reproducible with the newest PHP_CodeSniffer 3.3.2 and older versions. I stumbled upon it using doctrine/coding-standard 4.0.0 but it looks localized in the ArrayDeclaration sniff and could be reproduced with Squiz as well.

This file could be used as an example:

<?php

namespace Test;

function test()
{
    return [
        <<<'SQLDATA'
DATA
SQLDATA
        ,
    ];
}

Checking it produces (the problem is in line 11):

$ phpcs --standard=Squiz test.php 

FILE: /home/morozov/Projects/dbal/test.php
------------------------------------------------------------------------------------------------
FOUND 7 ERRORS AND 1 WARNING AFFECTING 5 LINES
------------------------------------------------------------------------------------------------
  1 | ERROR   | [ ] Missing file doc comment
  5 | WARNING | [ ] Consider putting global function "test" in a static class
  5 | ERROR   | [ ] Missing doc comment for function test()
  5 | ERROR   | [x] Expected 2 blank lines before function; 1 found
  8 | ERROR   | [ ] Use of heredoc and nowdoc syntax ("<<<") is not allowed; use standard
    |         |     strings or inline HTML instead
 11 | ERROR   | [x] Expected 0 spaces before comma; 8 found
 13 | ERROR   | [x] Expected //end test()
 13 | ERROR   | [x] Expected 1 blank line before closing function brace; 0 found
------------------------------------------------------------------------------------------------
PHPCBF CAN FIX THE 4 MARKED SNIFF VIOLATIONS AUTOMATICALLY
------------------------------------------------------------------------------------------------

If I remove the spaces before the comma as suggested, the error message still stays but turns into:

$ phpcs --standard=Squiz test.php 

FILE: /home/morozov/Projects/dbal/test.php
------------------------------------------------------------------------------------------------
FOUND 7 ERRORS AND 1 WARNING AFFECTING 5 LINES
------------------------------------------------------------------------------------------------
  1 | ERROR   | [ ] Missing file doc comment
  5 | WARNING | [ ] Consider putting global function "test" in a static class
  5 | ERROR   | [ ] Missing doc comment for function test()
  5 | ERROR   | [x] Expected 2 blank lines before function; 1 found
  8 | ERROR   | [ ] Use of heredoc and nowdoc syntax ("<<<") is not allowed; use standard
    |         |     strings or inline HTML instead
 11 | ERROR   | [x] Line indented incorrectly; expected at least 4 spaces, found 0
 13 | ERROR   | [x] Expected //end test()
 13 | ERROR   | [x] Expected 1 blank line before closing function brace; 0 found
------------------------------------------------------------------------------------------------
PHPCBF CAN FIX THE 4 MARKED SNIFF VIOLATIONS AUTOMATICALLY
------------------------------------------------------------------------------------------------

Auto-fixing the issue doesn't work either:

phpcbf --standard=Squiz test.php 

PHPCBF RESULT SUMMARY
----------------------------------------------------------------------
FILE                                                  FIXED  REMAINING
----------------------------------------------------------------------
/home/morozov/Projects/dbal/test.php                  FAILED TO FIX
----------------------------------------------------------------------
A TOTAL OF 3 ERRORS WERE FIXED IN 1 FILE
----------------------------------------------------------------------
PHPCBF FAILED TO FIX 1 FILE
----------------------------------------------------------------------

The fixing process internally looks like:

$ phpcs --standard=Squiz --report=diff -vv test.php
--------------------8<-----------------------------
	*** START FILE FIXING ***
	E: [Line 12] Line indented incorrectly; expected at least 4 spaces, found 0 (Generic.WhiteSpace.ScopeIndent.Incorrect)
	PHP_CodeSniffer\Standards\Generic\Sniffs\WhiteSpace\ScopeIndentSniff (line 1342) replaced token 27 (T_COMMA) "," => "····,"
	* fixed 1 violations, starting loop 2 *
	E: [Line 12] Expected 0 spaces before comma; 4 found (Squiz.Arrays.ArrayDeclaration.SpaceBeforeComma)
	PHP_CodeSniffer\Standards\Squiz\Sniffs\Arrays\ArrayDeclarationSniff (line 451) replaced token 27 (T_WHITESPACE) "····," => ","
	* fixed 1 violations, starting loop 3 *
	E: [Line 12] Line indented incorrectly; expected at least 4 spaces, found 0 (Generic.WhiteSpace.ScopeIndent.Incorrect)
	**** PHP_CodeSniffer\Standards\Generic\Sniffs\WhiteSpace\ScopeIndentSniff (line 1342) has possible conflict with another sniff on loop 1; caused by the following change ****
	**** replaced token 27 (T_COMMA) "," => "····," ****
--------------------8<-----------------------------
	* fixed 0 violations, starting loop 50 *
	E: [Line 12] Expected 0 spaces before comma; 4 found (Squiz.Arrays.ArrayDeclaration.SpaceBeforeComma)
	PHP_CodeSniffer\Standards\Squiz\Sniffs\Arrays\ArrayDeclarationSniff (line 451) replaced token 27 (T_WHITESPACE) "····," => ","
	* fixed 1 violations, starting loop 51 *
	*** Reached maximum number of loops with 1 violations left unfixed ***

Looks like there's some internal conflict which cannot be resolved, and the fixer gives up.

morozov added a commit to morozov/dbal that referenced this issue Sep 28, 2018
@gsherwood gsherwood added this to the 3.4.0 milestone Oct 8, 2018
@gsherwood gsherwood changed the title Incorrect handling of a HEREDOC value in array Conflict between Squiz.Arrays.ArrayDeclaration and ScopeIndent sniffs when heredoc used in array Oct 8, 2018
gsherwood added a commit that referenced this issue Oct 8, 2018
…ScopeIndent sniffs when heredoc used in array
@gsherwood
Copy link
Member

The ScopeIndent sniffs was an fault here. It was trying to skip here/nowdocs but not taking into account the character that followed the closer. It's normally a semi-colon on the same line and so causes no problems, but the comma used after the array value was not ignored.

Thanks for reporting this.

@morozov
Copy link
Contributor Author

morozov commented Oct 8, 2018

Thank you, @gsherwood, for taking care.

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