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

PSR12.ControlStructures.ControlStructureSpacing does not ignore comments between conditions #2730

Conversation

jrfnl
Copy link
Contributor

@jrfnl jrfnl commented Dec 1, 2019

PSR12 does not forbid comments interlaced in the conditions of a (multi-line) control structure.

However, the sniff didn't handle those correctly.

With the example code added to the unit test case file, the sniff, as it was, would throw an unsolvable error which would also cause fixer conflicts with the sniff itself:

 82 | ERROR | [x] Each line in a multi-line control structure must be indented at least once; expected at least 8 spaces, but found 0
    |       |     (PSR12.ControlStructures.ControlStructureSpacing.LineIndent)
# phpcbf ./psr12/tests/controlstructures/controlstructurespacingunittest.inc --standard=psr12 --sniffs=psr12.controlstructures.controlstructurespacing

PHP_CodeSniffer version 3.5.3 (stable) by Squiz (http://www.squiz.net)

PHPCBF RESULT SUMMARY
----------------------------------------------------------------------------------------------------------------------------------------
FILE                                                                                                                    FIXED  REMAINING
----------------------------------------------------------------------------------------------------------------------------------------
/src/Standards/PSR12/Tests/ControlStructures/ControlStructureSpacingUnitTest.inc     FAILED TO FIX
----------------------------------------------------------------------------------------------------------------------------------------
A TOTAL OF 17 ERRORS WERE FIXED IN 1 FILE
----------------------------------------------------------------------------------------------------------------------------------------
PHPCBF FAILED TO FIX 1 FILE
----------------------------------------------------------------------------------------------------------------------------------------

By ignoring lines where the first token is a comment, this issue will be fixed.

PSR12 does not forbid comments interlaced in the conditions of a (multi-line) control structure.

However, the sniff didn't handle those correctly.

With the example code added to the unit test case file, the sniff, as it was, would throw an unsolvable error which would also cause fixer conflicts with the sniff itself:
```
 82 | ERROR | [x] Each line in a multi-line control structure must be indented at least once; expected at least 8 spaces, but found 0
    |       |     (PSR12.ControlStructures.ControlStructureSpacing.LineIndent)
```

```
# phpcbf ./psr12/tests/controlstructures/controlstructurespacingunittest.inc --standard=psr12 --sniffs=psr12.controlstructures.controlstructurespacing

PHP_CodeSniffer version 3.5.3 (stable) by Squiz (http://www.squiz.net)

PHPCBF RESULT SUMMARY
----------------------------------------------------------------------------------------------------------------------------------------
FILE                                                                                                                    FIXED  REMAINING
----------------------------------------------------------------------------------------------------------------------------------------
/src/Standards/PSR12/Tests/ControlStructures/ControlStructureSpacingUnitTest.inc     FAILED TO FIX
----------------------------------------------------------------------------------------------------------------------------------------
A TOTAL OF 17 ERRORS WERE FIXED IN 1 FILE
----------------------------------------------------------------------------------------------------------------------------------------
PHPCBF FAILED TO FIX 1 FILE
----------------------------------------------------------------------------------------------------------------------------------------
```

By ignoring lines where the first token is a comment, this issue will be fixed.
@gsherwood gsherwood added this to the 3.5.4 milestone Dec 6, 2019
@gsherwood gsherwood changed the title PSR12/ControlStructureSpacing: allow for comments between conditions PSR12.ControlStructures.ControlStructureSpacing does not ignore comments between conditions Jan 8, 2020
gsherwood added a commit that referenced this pull request Jan 8, 2020
@gsherwood gsherwood merged commit 7aec50f into squizlabs:master Jan 8, 2020
@gsherwood
Copy link
Member

As much as I dont like comments between conditions, you're very right. Thanks for the PR.

@jrfnl jrfnl deleted the feature/psr12-controlstructurespacing-allow-fix-comments branch January 8, 2020 22:11
@jrfnl
Copy link
Contributor Author

jrfnl commented Jan 8, 2020

You're welcome ;-)

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