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.