Skip to content

Commit 692237d

Browse files
gsherwoodgsherwood
committed
Fixed bug #2763 : PSR12 standard reports errors for multi-line FOR definitions
This was done by adding a new ignoreNewlines property to Squiz.ControlStructures.ForLoopDeclaration so that the PSR12 standard could use it. In the case of newlines, PSR12 should ignore the statement and let other sniffs enforce the multi-line rules. But when there is no newline, it needs this sniff to enforce a single space.
1 parent f0ec2ab commit 692237d

File tree

6 files changed

+72
-17
lines changed

6 files changed

+72
-17
lines changed

‎package.xml‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,9 @@ http://pear.php.net/dtd/package-2.0.xsd">
4040
-- Thanks to Vincent Langlet for the patch
4141
- PSR12.Files.ImportStatement now auto-fixes import statements by removing the leading slash
4242
-- Thanks to Michał Bundyra for the patch
43+
- Squiz.ControlStructures.ForLoopDeclaration now has a setting to ignore newline characters
44+
-- Default remains FALSE, so newlines are not allowed within FOR definitions
45+
-- Override the "ignoreNewlines" setting in a ruleset.xml file to change
4346
- Squiz.PHP.InnerFunctions now handles multiple nested anon classes correctly
4447
- Fixed bug #2688 : Case statements not tokenized correctly when switch is contained within ternary
4548
- Fixed bug #2698 : PHPCS throws errors determining auto report width when shell_exec is disabled
@@ -48,6 +51,7 @@ http://pear.php.net/dtd/package-2.0.xsd">
4851
-- Thanks to Michał Bundyra for the patch
4952
- Fixed bug #2751 : Autoload relative paths first to avoid confusion with files from the global include path
5053
-- Thanks to Klaus Purer for the patch
54+
- Fixed bug #2763 : PSR12 standard reports errors for multi-line FOR definitions
5155
- Fixed bug #2768 : Generic.Files.LineLength false positive for non-breakable strings at exactly the soft limit
5256
-- Thanks to Alex Miles for the patch
5357
- Fixed bug #2791 : PSR12.Functions.NullableTypeDeclaration false positive when ternary operator used with instanceof

‎src/Standards/PSR12/ruleset.xml‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -228,7 +228,11 @@
228228
<rule ref="Squiz.WhiteSpace.ControlStructureSpacing.SpacingBeforeClose"/>
229229
<rule ref="Squiz.WhiteSpace.ScopeClosingBrace"/>
230230
<rule ref="Squiz.ControlStructures.ForEachLoopDeclaration"/>
231-
<rule ref="Squiz.ControlStructures.ForLoopDeclaration"/>
231+
<rule ref="Squiz.ControlStructures.ForLoopDeclaration">
232+
<properties>
233+
<property name="ignoreNewlines" value="true"/>
234+
</properties>
235+
</rule>
232236
<rule ref="Squiz.ControlStructures.ForLoopDeclaration.SpacingAfterOpen">
233237
<severity>0</severity>
234238
</rule>

‎src/Standards/Squiz/Sniffs/ControlStructures/ForLoopDeclarationSniff.php‎

Lines changed: 42 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,13 @@ class ForLoopDeclarationSniff implements Sniff
3030
*/
3131
public $requiredSpacesBeforeClose = 0;
3232

33+
/**
34+
* Allow newlines instead of spaces.
35+
*
36+
* @var boolean
37+
*/
38+
public $ignoreNewlines = false;
39+
3340
/**
3441
* A list of tokenizers this sniff supports.
3542
*
@@ -77,20 +84,27 @@ public function process(File $phpcsFile, $stackPtr)
7784

7885
$closingBracket = $tokens[$openingBracket]['parenthesis_closer'];
7986

80-
if ($this->requiredSpacesAfterOpen === 0 && $tokens[($openingBracket + 1)]['code'] === T_WHITESPACE) {
81-
$error = 'Whitespace found after opening bracket of FOR loop';
82-
$fix = $phpcsFile->addFixableError($error, $openingBracket, 'SpacingAfterOpen');
83-
if ($fix === true) {
84-
$phpcsFile->fixer->beginChangeset();
85-
for ($i = ($openingBracket + 1); $i < $closingBracket; $i++) {
86-
if ($tokens[$i]['code'] !== T_WHITESPACE) {
87-
break;
87+
if ($this->requiredSpacesAfterOpen === 0
88+
&& $tokens[($openingBracket + 1)]['code'] === T_WHITESPACE
89+
) {
90+
$nextNonWhiteSpace = $phpcsFile->findNext(T_WHITESPACE, ($openingBracket + 1), $closingBracket, true);
91+
if ($this->ignoreNewlines === false
92+
|| $tokens[$nextNonWhiteSpace]['line'] === $tokens[$openingBracket]['line']
93+
) {
94+
$error = 'Whitespace found after opening bracket of FOR loop';
95+
$fix = $phpcsFile->addFixableError($error, $openingBracket, 'SpacingAfterOpen');
96+
if ($fix === true) {
97+
$phpcsFile->fixer->beginChangeset();
98+
for ($i = ($openingBracket + 1); $i < $closingBracket; $i++) {
99+
if ($tokens[$i]['code'] !== T_WHITESPACE) {
100+
break;
101+
}
102+
103+
$phpcsFile->fixer->replaceToken($i, '');
88104
}
89105

90-
$phpcsFile->fixer->replaceToken($i, '');
106+
$phpcsFile->fixer->endChangeset();
91107
}
92-
93-
$phpcsFile->fixer->endChangeset();
94108
}
95109
} else if ($this->requiredSpacesAfterOpen > 0) {
96110
$nextNonWhiteSpace = $phpcsFile->findNext(T_WHITESPACE, ($openingBracket + 1), $closingBracket, true);
@@ -101,7 +115,10 @@ public function process(File $phpcsFile, $stackPtr)
101115
$spaceAfterOpen = $tokens[($openingBracket + 1)]['length'];
102116
}
103117

104-
if ($spaceAfterOpen !== $this->requiredSpacesAfterOpen) {
118+
if ($spaceAfterOpen !== $this->requiredSpacesAfterOpen
119+
&& ($this->ignoreNewlines === false
120+
|| $spaceAfterOpen !== 'newline')
121+
) {
105122
$error = 'Expected %s spaces after opening bracket; %s found';
106123
$data = [
107124
$this->requiredSpacesAfterOpen,
@@ -133,7 +150,11 @@ public function process(File $phpcsFile, $stackPtr)
133150
$beforeClosefixable = false;
134151
}
135152

136-
if ($this->requiredSpacesBeforeClose === 0 && $tokens[($closingBracket - 1)]['code'] === T_WHITESPACE) {
153+
if ($this->requiredSpacesBeforeClose === 0
154+
&& $tokens[($closingBracket - 1)]['code'] === T_WHITESPACE
155+
&& ($this->ignoreNewlines === false
156+
|| $tokens[$prevNonWhiteSpace]['line'] === $tokens[$closingBracket]['line'])
157+
) {
137158
$error = 'Whitespace found before closing bracket of FOR loop';
138159

139160
if ($beforeClosefixable === false) {
@@ -161,7 +182,10 @@ public function process(File $phpcsFile, $stackPtr)
161182
$spaceBeforeClose = $tokens[($closingBracket - 1)]['length'];
162183
}
163184

164-
if ($this->requiredSpacesBeforeClose !== $spaceBeforeClose) {
185+
if ($this->requiredSpacesBeforeClose !== $spaceBeforeClose
186+
&& ($this->ignoreNewlines === false
187+
|| $spaceBeforeClose !== 'newline')
188+
) {
165189
$error = 'Expected %s spaces before closing bracket; %s found';
166190
$data = [
167191
$this->requiredSpacesBeforeClose,
@@ -264,7 +288,10 @@ public function process(File $phpcsFile, $stackPtr)
264288
$spaces = 'newline';
265289
}
266290

267-
if ($spaces !== 1) {
291+
if ($spaces !== 1
292+
&& ($this->ignoreNewlines === false
293+
|| $spaces !== 'newline')
294+
) {
268295
$error = 'Expected 1 space after %s semicolon of FOR loop; %s found';
269296
$errorCode = 'SpacingAfter'.$humanReadableCode;
270297
$data[] = $spaces;

‎src/Standards/Squiz/Tests/ControlStructures/ForLoopDeclarationUnitTest.inc‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -115,5 +115,15 @@ for (
115115
for ($i = function() { return $this->i ; }; $i < function() { return $this->max; }; $i++) {}
116116
for ($i = function() { return $this->i; }; $i < function() { return $this->max; } ; $i++) {}
117117

118+
// phpcs:set Squiz.ControlStructures.ForLoopDeclaration ignoreNewlines true
119+
for (
120+
$i = 0;
121+
$i < 5;
122+
$i++
123+
) {
124+
// body here
125+
}
126+
// phpcs:set Squiz.ControlStructures.ForLoopDeclaration ignoreNewlines false
127+
118128
// This test has to be the last one in the file! Intentional parse error check.
119129
for

‎src/Standards/Squiz/Tests/ControlStructures/ForLoopDeclarationUnitTest.inc.fixed‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,5 +81,15 @@ for ( $i = 0; $i < 10; $i++ ) {}
8181
for ($i = function() { return $this->i ; }; $i < function() { return $this->max; }; $i++) {}
8282
for ($i = function() { return $this->i; }; $i < function() { return $this->max; }; $i++) {}
8383

84+
// phpcs:set Squiz.ControlStructures.ForLoopDeclaration ignoreNewlines true
85+
for (
86+
$i = 0;
87+
$i < 5;
88+
$i++
89+
) {
90+
// body here
91+
}
92+
// phpcs:set Squiz.ControlStructures.ForLoopDeclaration ignoreNewlines false
93+
8494
// This test has to be the last one in the file! Intentional parse error check.
8595
for

‎src/Standards/Squiz/Tests/ControlStructures/ForLoopDeclarationUnitTest.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -117,7 +117,7 @@ public function getWarningList($testFile='ForLoopDeclarationUnitTest.inc')
117117
{
118118
switch ($testFile) {
119119
case 'ForLoopDeclarationUnitTest.inc':
120-
return [119 => 1];
120+
return [129 => 1];
121121

122122
case 'ForLoopDeclarationUnitTest.js':
123123
return [125 => 1];

0 commit comments

Comments
 (0)