squizlabs / PHP_CodeSniffer

PHP_CodeSniffer tokenizes PHP files and detects violations of a defined set of coding standards.
BSD 3-Clause "New" or "Revised" License
10.66k stars 1.48k forks source link

Fixer conflict between Generic.WhiteSpace.ScopeIndent and Squiz.WhiteSpace.ScopeClosingBrace when class indented 1 space #2464

Closed mirzazeyrek closed 5 years ago

mirzazeyrek commented 5 years ago

PHPCS version:

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

Command:

phpcbf --standard=PSR2 calculator.php -p

File:

 <?php

 class Test {

 }

image

jrfnl commented 5 years ago

@mirzazeyrek Thanks for reporting this, though I think the topic needs to be changed.

There is no rule in PSR-2 about space before the PHP open tag not being allowed, so PHPCS is not trying to fix that at all, nor will it report on it.

The fixer conflict you are seeing is a conflict between the Generic.WhiteSpace.ScopeIndent and the Squiz.WhiteSpace.ScopeClosingBrace sniffs.

I've ran into that one a number of times before, but haven't been able to find the time to debug it properly .

gsherwood commented 5 years ago

A possible solution for this is to just let scope closers (like the class closing brace) be indented to a column that isn't divisible by the current tab width.

This change stops the conflict from occurring and it doesn't make any of the scope indent unit tests fail, but need to add some more tests just to be sure.

gsherwood commented 5 years ago

I checked the history of this code and it's been around forever, but an exception was made for closures about 5 years ago. None of the tests are failing, so I've made the change, which allows the closing brace to sit wherever the opener does and not have to be a perfect index. This lets other parts of a standard enforce closing brace rules.