Skip to content

Commit ad4b3d7

Browse files
gsherwoodgsherwood
committed
Merge branch 'feature/squiz-forloopdeclaration-add-fixed-file-and-more' of https://github.com/jrfnl/PHP_CodeSniffer 
2 parents 2055248 + db6e834 commit ad4b3d7

File tree

7 files changed

+558
-100
lines changed

7 files changed

+558
-100
lines changed

‎package.xml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1458,7 +1458,9 @@ http://pear.php.net/dtd/package-2.0.xsd">
14581458
<file baseinstalldir="PHP/CodeSniffer" name="ForEachLoopDeclarationUnitTest.inc.fixed" role="test" />
14591459
<file baseinstalldir="PHP/CodeSniffer" name="ForEachLoopDeclarationUnitTest.php" role="test" />
14601460
<file baseinstalldir="PHP/CodeSniffer" name="ForLoopDeclarationUnitTest.inc" role="test" />
1461+
<file baseinstalldir="PHP/CodeSniffer" name="ForLoopDeclarationUnitTest.inc.fixed" role="test" />
14611462
<file baseinstalldir="PHP/CodeSniffer" name="ForLoopDeclarationUnitTest.js" role="test" />
1463+
<file baseinstalldir="PHP/CodeSniffer" name="ForLoopDeclarationUnitTest.js.fixed" role="test" />
14621464
<file baseinstalldir="PHP/CodeSniffer" name="ForLoopDeclarationUnitTest.php" role="test" />
14631465
<file baseinstalldir="PHP/CodeSniffer" name="InlineIfDeclarationUnitTest.inc" role="test" />
14641466
<file baseinstalldir="PHP/CodeSniffer" name="InlineIfDeclarationUnitTest.inc.fixed" role="test" />

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

Lines changed: 149 additions & 77 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111

1212
use PHP_CodeSniffer\Sniffs\Sniff;
1313
use PHP_CodeSniffer\Files\File;
14+
use PHP_CodeSniffer\Util\Tokens;
1415

1516
class ForLoopDeclarationSniff implements Sniff
1617
{
@@ -77,15 +78,27 @@ public function process(File $phpcsFile, $stackPtr)
7778
$closingBracket = $tokens[$openingBracket]['parenthesis_closer'];
7879

7980
if ($this->requiredSpacesAfterOpen === 0 && $tokens[($openingBracket + 1)]['code'] === T_WHITESPACE) {
80-
$error = 'Space found after opening bracket of FOR loop';
81-
$fix = $phpcsFile->addFixableError($error, $stackPtr, 'SpacingAfterOpen');
81+
$error = 'Whitespace found after opening bracket of FOR loop';
82+
$fix = $phpcsFile->addFixableError($error, $openingBracket, 'SpacingAfterOpen');
8283
if ($fix === true) {
83-
$phpcsFile->fixer->replaceToken(($openingBracket + 1), '');
84+
$phpcsFile->fixer->beginChangeset();
85+
for ($i = ($openingBracket + 1); $i < $closingBracket; $i++) {
86+
if ($tokens[$i]['code'] !== T_WHITESPACE) {
87+
break;
88+
}
89+
90+
$phpcsFile->fixer->replaceToken($i, '');
91+
}
92+
93+
$phpcsFile->fixer->endChangeset();
8494
}
8595
} else if ($this->requiredSpacesAfterOpen > 0) {
86-
$spaceAfterOpen = 0;
87-
if ($tokens[($openingBracket + 1)]['code'] === T_WHITESPACE) {
88-
$spaceAfterOpen = strlen($tokens[($openingBracket + 1)]['content']);
96+
$nextNonWhiteSpace = $phpcsFile->findNext(T_WHITESPACE, ($openingBracket + 1), $closingBracket, true);
97+
$spaceAfterOpen = 0;
98+
if ($tokens[$openingBracket]['line'] !== $tokens[$nextNonWhiteSpace]['line']) {
99+
$spaceAfterOpen = 'newline';
100+
} else if ($tokens[($openingBracket + 1)]['code'] === T_WHITESPACE) {
101+
$spaceAfterOpen = $tokens[($openingBracket + 1)]['length'];
89102
}
90103

91104
if ($spaceAfterOpen !== $this->requiredSpacesAfterOpen) {
@@ -94,28 +107,58 @@ public function process(File $phpcsFile, $stackPtr)
94107
$this->requiredSpacesAfterOpen,
95108
$spaceAfterOpen,
96109
];
97-
$fix = $phpcsFile->addFixableError($error, $stackPtr, 'SpacingAfterOpen', $data);
110+
$fix = $phpcsFile->addFixableError($error, $openingBracket, 'SpacingAfterOpen', $data);
98111
if ($fix === true) {
99112
$padding = str_repeat(' ', $this->requiredSpacesAfterOpen);
100113
if ($spaceAfterOpen === 0) {
101114
$phpcsFile->fixer->addContent($openingBracket, $padding);
102115
} else {
116+
$phpcsFile->fixer->beginChangeset();
103117
$phpcsFile->fixer->replaceToken(($openingBracket + 1), $padding);
118+
for ($i = ($openingBracket + 2); $i < $nextNonWhiteSpace; $i++) {
119+
$phpcsFile->fixer->replaceToken($i, '');
120+
}
121+
122+
$phpcsFile->fixer->endChangeset();
104123
}
105124
}
106-
}
125+
}//end if
107126
}//end if
108127

128+
$prevNonWhiteSpace = $phpcsFile->findPrevious(T_WHITESPACE, ($closingBracket - 1), $openingBracket, true);
129+
$beforeClosefixable = true;
130+
if ($tokens[$prevNonWhiteSpace]['line'] !== $tokens[$closingBracket]['line']
131+
&& isset(Tokens::$emptyTokens[$tokens[$prevNonWhiteSpace]['code']]) === true
132+
) {
133+
$beforeClosefixable = false;
134+
}
135+
109136
if ($this->requiredSpacesBeforeClose === 0 && $tokens[($closingBracket - 1)]['code'] === T_WHITESPACE) {
110-
$error = 'Space found before closing bracket of FOR loop';
111-
$fix = $phpcsFile->addFixableError($error, $stackPtr, 'SpacingBeforeClose');
112-
if ($fix === true) {
113-
$phpcsFile->fixer->replaceToken(($closingBracket - 1), '');
137+
$error = 'Whitespace found before closing bracket of FOR loop';
138+
139+
if ($beforeClosefixable === false) {
140+
$phpcsFile->addError($error, $closingBracket, 'SpacingBeforeClose');
141+
} else {
142+
$fix = $phpcsFile->addFixableError($error, $closingBracket, 'SpacingBeforeClose');
143+
if ($fix === true) {
144+
$phpcsFile->fixer->beginChangeset();
145+
for ($i = ($closingBracket - 1); $i > $openingBracket; $i--) {
146+
if ($tokens[$i]['code'] !== T_WHITESPACE) {
147+
break;
148+
}
149+
150+
$phpcsFile->fixer->replaceToken($i, '');
151+
}
152+
153+
$phpcsFile->fixer->endChangeset();
154+
}
114155
}
115156
} else if ($this->requiredSpacesBeforeClose > 0) {
116157
$spaceBeforeClose = 0;
117-
if ($tokens[($closingBracket - 1)]['code'] === T_WHITESPACE) {
118-
$spaceBeforeClose = strlen($tokens[($closingBracket - 1)]['content']);
158+
if ($tokens[$closingBracket]['line'] !== $tokens[$prevNonWhiteSpace]['line']) {
159+
$spaceBeforeClose = 'newline';
160+
} else if ($tokens[($closingBracket - 1)]['code'] === T_WHITESPACE) {
161+
$spaceBeforeClose = $tokens[($closingBracket - 1)]['length'];
119162
}
120163

121164
if ($this->requiredSpacesBeforeClose !== $spaceBeforeClose) {
@@ -124,92 +167,121 @@ public function process(File $phpcsFile, $stackPtr)
124167
$this->requiredSpacesBeforeClose,
125168
$spaceBeforeClose,
126169
];
127-
$fix = $phpcsFile->addFixableError($error, $stackPtr, 'SpacingBeforeClose', $data);
128-
if ($fix === true) {
129-
$padding = str_repeat(' ', $this->requiredSpacesBeforeClose);
130-
if ($spaceBeforeClose === 0) {
131-
$phpcsFile->fixer->addContentBefore($closingBracket, $padding);
132-
} else {
133-
$phpcsFile->fixer->replaceToken(($closingBracket - 1), $padding);
170+
171+
if ($beforeClosefixable === false) {
172+
$phpcsFile->addError($error, $closingBracket, 'SpacingBeforeClose', $data);
173+
} else {
174+
$fix = $phpcsFile->addFixableError($error, $closingBracket, 'SpacingBeforeClose', $data);
175+
if ($fix === true) {
176+
$padding = str_repeat(' ', $this->requiredSpacesBeforeClose);
177+
if ($spaceBeforeClose === 0) {
178+
$phpcsFile->fixer->addContentBefore($closingBracket, $padding);
179+
} else {
180+
$phpcsFile->fixer->beginChangeset();
181+
$phpcsFile->fixer->replaceToken(($closingBracket - 1), $padding);
182+
for ($i = ($closingBracket - 2); $i > $prevNonWhiteSpace; $i--) {
183+
$phpcsFile->fixer->replaceToken($i, '');
184+
}
185+
186+
$phpcsFile->fixer->endChangeset();
187+
}
134188
}
135189
}
136-
}
190+
}//end if
137191
}//end if
138192

139-
$firstSemicolon = $phpcsFile->findNext(T_SEMICOLON, $openingBracket, $closingBracket);
193+
/*
194+
* Check whitespace around each of the semicolon tokens.
195+
*/
140196

141-
// Check whitespace around each of the tokens.
142-
if ($firstSemicolon !== false) {
143-
if ($tokens[($firstSemicolon - 1)]['code'] === T_WHITESPACE) {
144-
$error = 'Space found before first semicolon of FOR loop';
145-
$fix = $phpcsFile->addFixableError($error, $stackPtr, 'SpacingBeforeFirst');
146-
if ($fix === true) {
147-
$phpcsFile->fixer->replaceToken(($firstSemicolon - 1), '');
148-
}
197+
$semicolonCount = 0;
198+
$semicolon = $openingBracket;
199+
$targetNestinglevel = 0;
200+
if (isset($tokens[$openingBracket]['conditions']) === true) {
201+
$targetNestinglevel += count($tokens[$openingBracket]['conditions']);
202+
}
203+
204+
do {
205+
$semicolon = $phpcsFile->findNext(T_SEMICOLON, ($semicolon + 1), $closingBracket);
206+
if ($semicolon === false) {
207+
break;
149208
}
150209

151-
if ($tokens[($firstSemicolon + 1)]['code'] !== T_WHITESPACE
152-
&& $tokens[($firstSemicolon + 1)]['code'] !== T_SEMICOLON
210+
if (isset($tokens[$semicolon]['conditions']) === true
211+
&& count($tokens[$semicolon]['conditions']) > $targetNestinglevel
153212
) {
154-
$error = 'Expected 1 space after first semicolon of FOR loop; 0 found';
155-
$fix = $phpcsFile->addFixableError($error, $stackPtr, 'NoSpaceAfterFirst');
156-
if ($fix === true) {
157-
$phpcsFile->fixer->addContent($firstSemicolon, ' ');
158-
}
159-
} else {
160-
if (strlen($tokens[($firstSemicolon + 1)]['content']) !== 1) {
161-
$spaces = strlen($tokens[($firstSemicolon + 1)]['content']);
162-
$error = 'Expected 1 space after first semicolon of FOR loop; %s found';
163-
$data = [$spaces];
164-
$fix = $phpcsFile->addFixableError($error, $stackPtr, 'SpacingAfterFirst', $data);
165-
if ($fix === true) {
166-
$phpcsFile->fixer->replaceToken(($firstSemicolon + 1), ' ');
167-
}
168-
}
213+
// Semicolon doesn't belong to the for().
214+
continue;
169215
}
170216

171-
$secondSemicolon = $phpcsFile->findNext(T_SEMICOLON, ($firstSemicolon + 1));
217+
++$semicolonCount;
172218

173-
if ($secondSemicolon !== false) {
174-
if ($tokens[($secondSemicolon - 1)]['code'] === T_WHITESPACE
175-
&& $tokens[($firstSemicolon + 1)]['code'] !== T_SEMICOLON
176-
) {
177-
$error = 'Space found before second semicolon of FOR loop';
178-
$fix = $phpcsFile->addFixableError($error, $stackPtr, 'SpacingBeforeSecond');
219+
$humanReadableCount = 'first';
220+
if ($semicolonCount !== 1) {
221+
$humanReadableCount = 'second';
222+
}
223+
224+
$humanReadableCode = ucfirst($humanReadableCount);
225+
$data = [$humanReadableCount];
226+
227+
// Only examine the space before the first semicolon if the first expression is not empty.
228+
// If it *is* empty, leave it up to the `SpacingAfterOpen` logic.
229+
$prevNonWhiteSpace = $phpcsFile->findPrevious(T_WHITESPACE, ($semicolon - 1), $openingBracket, true);
230+
if ($semicolonCount !== 1 || $prevNonWhiteSpace !== $openingBracket) {
231+
if ($tokens[($semicolon - 1)]['code'] === T_WHITESPACE) {
232+
$error = 'Whitespace found before %s semicolon of FOR loop';
233+
$errorCode = 'SpacingBefore'.$humanReadableCode;
234+
$fix = $phpcsFile->addFixableError($error, $semicolon, $errorCode, $data);
179235
if ($fix === true) {
180-
$phpcsFile->fixer->replaceToken(($secondSemicolon - 1), '');
236+
$phpcsFile->fixer->beginChangeset();
237+
for ($i = ($semicolon - 1); $i > $prevNonWhiteSpace; $i--) {
238+
$phpcsFile->fixer->replaceToken($i, '');
239+
}
240+
241+
$phpcsFile->fixer->endChangeset();
181242
}
182243
}
244+
}
183245

184-
if (($secondSemicolon + 1) !== $closingBracket
185-
&& $tokens[($secondSemicolon + 1)]['code'] !== T_WHITESPACE
246+
// Only examine the space after the second semicolon if the last expression is not empty.
247+
// If it *is* empty, leave it up to the `SpacingBeforeClose` logic.
248+
$nextNonWhiteSpace = $phpcsFile->findNext(T_WHITESPACE, ($semicolon + 1), ($closingBracket + 1), true);
249+
if ($semicolonCount !== 2 || $nextNonWhiteSpace !== $closingBracket) {
250+
if ($tokens[($semicolon + 1)]['code'] !== T_WHITESPACE
251+
&& $tokens[($semicolon + 1)]['code'] !== T_SEMICOLON
186252
) {
187-
$error = 'Expected 1 space after second semicolon of FOR loop; 0 found';
188-
$fix = $phpcsFile->addFixableError($error, $stackPtr, 'NoSpaceAfterSecond');
253+
$error = 'Expected 1 space after %s semicolon of FOR loop; 0 found';
254+
$errorCode = 'NoSpaceAfter'.$humanReadableCode;
255+
$fix = $phpcsFile->addFixableError($error, $semicolon, $errorCode, $data);
189256
if ($fix === true) {
190-
$phpcsFile->fixer->addContent($secondSemicolon, ' ');
257+
$phpcsFile->fixer->addContent($semicolon, ' ');
191258
}
192-
} else {
193-
if (strlen($tokens[($secondSemicolon + 1)]['content']) !== 1) {
194-
$spaces = strlen($tokens[($secondSemicolon + 1)]['content']);
195-
$data = [$spaces];
196-
if (($secondSemicolon + 2) === $closingBracket) {
197-
$error = 'Expected no space after second semicolon of FOR loop; %s found';
198-
$fix = $phpcsFile->addFixableError($error, $stackPtr, 'SpacingAfterSecondNoThird', $data);
199-
if ($fix === true) {
200-
$phpcsFile->fixer->replaceToken(($secondSemicolon + 1), '');
201-
}
202-
} else {
203-
$error = 'Expected 1 space after second semicolon of FOR loop; %s found';
204-
$fix = $phpcsFile->addFixableError($error, $stackPtr, 'SpacingAfterSecond', $data);
205-
if ($fix === true) {
206-
$phpcsFile->fixer->replaceToken(($secondSemicolon + 1), ' ');
259+
} else if ($tokens[($semicolon + 1)]['code'] === T_WHITESPACE
260+
&& $tokens[$nextNonWhiteSpace]['code'] !== T_SEMICOLON
261+
) {
262+
$spaces = $tokens[($semicolon + 1)]['length'];
263+
if ($tokens[$semicolon]['line'] !== $tokens[$nextNonWhiteSpace]['line']) {
264+
$spaces = 'newline';
265+
}
266+
267+
if ($spaces !== 1) {
268+
$error = 'Expected 1 space after %s semicolon of FOR loop; %s found';
269+
$errorCode = 'SpacingAfter'.$humanReadableCode;
270+
$data[] = $spaces;
271+
$fix = $phpcsFile->addFixableError($error, $semicolon, $errorCode, $data);
272+
if ($fix === true) {
273+
$phpcsFile->fixer->beginChangeset();
274+
$phpcsFile->fixer->replaceToken(($semicolon + 1), ' ');
275+
for ($i = ($semicolon + 2); $i < $nextNonWhiteSpace; $i++) {
276+
$phpcsFile->fixer->replaceToken($i, '');
207277
}
278+
279+
$phpcsFile->fixer->endChangeset();
208280
}
209281
}
210282
}//end if
211283
}//end if
212-
}//end if
284+
} while ($semicolonCount < 2);
213285

214286
}//end process()
215287

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

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,3 +39,81 @@ for ( $i = 0; $i < 10; $i++ ) {}
3939
for ( $i = 0; $i < 10; $i++ ) {}
4040
// phpcs:set Squiz.ControlStructures.ForLoopDeclaration requiredSpacesAfterOpen 0
4141
// phpcs:set Squiz.ControlStructures.ForLoopDeclaration requiredSpacesBeforeClose 0
42+
43+
for ( ; $i < 10; $i++) {}
44+
for (; $i < 10; $i++) {}
45+
46+
// phpcs:set Squiz.ControlStructures.ForLoopDeclaration requiredSpacesAfterOpen 1
47+
// phpcs:set Squiz.ControlStructures.ForLoopDeclaration requiredSpacesBeforeClose 1
48+
for ( ; $i < 10; $i++ ) {}
49+
for ( ; $i < 10; $i++ ) {}
50+
for (; $i < 10; $i++ ) {}
51+
52+
for ( $i = 0; $i < 10; ) {}
53+
for ( $i = 0; $i < 10;) {}
54+
for ( $i = 0; $i < 10; ) {}
55+
// phpcs:set Squiz.ControlStructures.ForLoopDeclaration requiredSpacesAfterOpen 0
56+
// phpcs:set Squiz.ControlStructures.ForLoopDeclaration requiredSpacesBeforeClose 0
57+
58+
// Test handling of comments and inline annotations.
59+
for ( /*phpcs:enable*/ $i = 0 /*start*/ ; /*end*/$i < 10/*comment*/; $i++ /*comment*/ ) {}
60+
61+
// Test multi-line FOR control structure.
62+
for (
63+
$i = 0;
64+
$i < 10;
65+
$i++
66+
) {}
67+
68+
// Test multi-line FOR control structure with comments and annotations.
69+
for (
70+
$i = 0; /* Start */
71+
$i < 10; /* phpcs:ignore Standard.Category.SniffName -- for reasons. */
72+
$i++ // comment
73+
74+
) {}
75+
76+
// Test fixing each error in one go. Note: lines 78 + 82 contain trailing whitespace on purpose.
77+
for (
78+
79+
80+
$i = 0
81+
82+
;
83+
84+
$i < 10
85+
86+
;
87+
88+
$i++
89+
90+
91+
) {}
92+
93+
// phpcs:set Squiz.ControlStructures.ForLoopDeclaration requiredSpacesAfterOpen 1
94+
// phpcs:set Squiz.ControlStructures.ForLoopDeclaration requiredSpacesBeforeClose 1
95+
for (
96+
97+
98+
99+
$i = 0
100+
101+
;
102+
103+
$i < 10
104+
105+
;
106+
107+
$i++
108+
109+
110+
) {}
111+
// phpcs:set Squiz.ControlStructures.ForLoopDeclaration requiredSpacesAfterOpen 0
112+
// phpcs:set Squiz.ControlStructures.ForLoopDeclaration requiredSpacesBeforeClose 0
113+
114+
// Test with semi-colon not belonging to for.
115+
for ($i = function() { return $this->i ; }; $i < function() { return $this->max; }; $i++) {}
116+
for ($i = function() { return $this->i; }; $i < function() { return $this->max; } ; $i++) {}
117+
118+
// This test has to be the last one in the file! Intentional parse error check.
119+
for

0 commit comments

Comments
 (0)