webimpress/coding-standard

Sniffs\Commenting\PlacementSniff has possible conflict with another sniff

Xerkus opened this issue · 4 comments

This file causes maximum number of loops reached error with zend-coding-standard 2.0.0-alpha.3:
https://github.com/zendframework/zend-mvc/blob/d83d57e2e64e240e94efcc43d5b1483919cb65a0/src/View/Http/InjectTemplateListener.php

Related dependencies:

dealerdirect/phpcodesniffer-composer-installer v0.5.0
slevomat/coding-standard                       4.8.7
squizlabs/php_codesniffer                      3.4.0
webimpress/coding-standard                     dev-master dffb652
zendframework/zend-coding-standard             2.0.0-alpha.3

Offending part by the looks of it:
https://github.com/zendframework/zend-mvc/blob/d83d57e2e64e240e94efcc43d5b1483919cb65a0/src/View/Http/InjectTemplateListener.php#L108-L115

            if (// Allow disabling rule by setting value to false since config
                // merging have no feature to remove entries
                false == $replacement
                // Match full class or full namespace
                || ! ($controller === $namespace || strpos($controller, $namespace . '\\') === 0)
            ) {
                continue;
            }
 => Fixing file: 0/38 violations remaining [made 45 passes]...   * fixed 0 violations, starting loop 46 *
WebimpressCodingStandard\Sniffs\Commenting\PlacementSniff (line 41) replaced token 838 (T_COMMENT) "//·Allow·disabling·rule·by·setting·value·to·false·since·config\n" => "·//·Allow·disabling·rule·by·setting·value·to·false·since·config\n"
=> Fixing file: 1/38 violations remaining [made 46 passes]...   * fixed 1 violations, starting loop 47 *
**** PHP_CodeSniffer\Standards\PSR2\Sniffs\ControlStructures\ControlStructureSpacingSniff (line 105) has possible conflict with another sniff on loop 45; caused by the following change ****
**** replaced token 838 (T_WHITESPACE) "·//·Allow·disabling·rule·by·setting·value·to·false·since·config\n" => "//·Allow·disabling·rule·by·setting·value·to·false·since·config\n" ****
**** ignoring all changes until next loop ****
=> Fixing file: 0/38 violations remaining [made 47 passes]...   * fixed 0 violations, starting loop 48 *
PHP_CodeSniffer\Standards\PSR2\Sniffs\ControlStructures\ControlStructureSpacingSniff (line 105) replaced token 838 (T_WHITESPACE) "·//·Allow·disabling·rule·by·setting·value·to·false·since·config\n" => "//·Allow·disabling·rule·by·setting·value·to·false·since·config\n"
=> Fixing file: 1/38 violations remaining [made 48 passes]...   * fixed 1 violations, starting loop 49 *
**** WebimpressCodingStandard\Sniffs\Commenting\PlacementSniff (line 41) has possible conflict with another sniff on loop 47; caused by the following change ****
**** replaced token 838 (T_COMMENT) "//·Allow·disabling·rule·by·setting·value·to·false·since·config\n" => "·//·Allow·disabling·rule·by·setting·value·to·false·since·config\n" ****
**** ignoring all changes until next loop ****

Crosslink: zendframework/zend-coding-standard#14

Thanks for reporting that issue.

I was checking this and not sure how to resolve it. There is a conflict with PSR2.ControlStructures.ControlStructureSpacing.SpacingAfterOpenBrace as this disallow spaces after opening bracket in if statement. But we do not have any sniff to disallow spaces after opening bracket in multiline function definition, for example:

...
public function method( //comment
    int $param = 1
) {
}

is a valid code, but:

if ( // comment
    $var === false
) {
}

is not valid - using PSR-2 standard from PHP_CodeSniffer.

I would argue if this if statement should be with comments inside. What I would do is define descriptive variables and use them in the if statement, so then we do not need comments (or if still we need them - add before variable definition). Comments in if statement are quite confusing.

I need to think what solution we can apply. Maybe PSR-2 from PHP_CodeSniffer should allow space after if ( in case the next token is a comment? Thoughts?

I've opened PR in PHP_CodeSniffer to solve that issue:
squizlabs/PHP_CodeSniffer#2441

Just to update: my PR is merged into PHP_CodeSniffer for 3.5.0 release.

Closed by #37.