Skip to content

Commit db6e834

Browse files
committed
Squiz/ForLoopDeclaration: prevent creating parse error with SpacingBeforeClose fixer
When a `//`-style comment would be at the end of a multi-line `for` declaration, the `SpacingBeforeClose` fixers could inadvertently create a parse error, by moving the `for` close parenthesis into the comment. Fixed by checking whether the previous non-whitespace token is on another line and a comment and making the errors non-fixable in that case.
1 parent c882c2b commit db6e834

File tree

1 file changed

+41
-23
lines changed

1 file changed

+41
-23
lines changed

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

Lines changed: 41 additions & 23 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
{
@@ -124,24 +125,36 @@ public function process(File $phpcsFile, $stackPtr)
124125
}//end if
125126
}//end if
126127

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+
127136
if ($this->requiredSpacesBeforeClose === 0 && $tokens[($closingBracket - 1)]['code'] === T_WHITESPACE) {
128137
$error = 'Whitespace found before closing bracket of FOR loop';
129-
$fix = $phpcsFile->addFixableError($error, $closingBracket, 'SpacingBeforeClose');
130-
if ($fix === true) {
131-
$phpcsFile->fixer->beginChangeset();
132-
for ($i = ($closingBracket - 1); $i > $openingBracket; $i--) {
133-
if ($tokens[$i]['code'] !== T_WHITESPACE) {
134-
break;
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, '');
135151
}
136152

137-
$phpcsFile->fixer->replaceToken($i, '');
153+
$phpcsFile->fixer->endChangeset();
138154
}
139-
140-
$phpcsFile->fixer->endChangeset();
141155
}
142156
} else if ($this->requiredSpacesBeforeClose > 0) {
143-
$prevNonWhiteSpace = $phpcsFile->findPrevious(T_WHITESPACE, ($closingBracket - 1), $openingBracket, true);
144-
$spaceBeforeClose = 0;
157+
$spaceBeforeClose = 0;
145158
if ($tokens[$closingBracket]['line'] !== $tokens[$prevNonWhiteSpace]['line']) {
146159
$spaceBeforeClose = 'newline';
147160
} else if ($tokens[($closingBracket - 1)]['code'] === T_WHITESPACE) {
@@ -154,19 +167,24 @@ public function process(File $phpcsFile, $stackPtr)
154167
$this->requiredSpacesBeforeClose,
155168
$spaceBeforeClose,
156169
];
157-
$fix = $phpcsFile->addFixableError($error, $closingBracket, 'SpacingBeforeClose', $data);
158-
if ($fix === true) {
159-
$padding = str_repeat(' ', $this->requiredSpacesBeforeClose);
160-
if ($spaceBeforeClose === 0) {
161-
$phpcsFile->fixer->addContentBefore($closingBracket, $padding);
162-
} else {
163-
$phpcsFile->fixer->beginChangeset();
164-
$phpcsFile->fixer->replaceToken(($closingBracket - 1), $padding);
165-
for ($i = ($closingBracket - 2); $i > $prevNonWhiteSpace; $i--) {
166-
$phpcsFile->fixer->replaceToken($i, '');
167-
}
168170

169-
$phpcsFile->fixer->endChangeset();
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+
}
170188
}
171189
}
172190
}//end if

0 commit comments

Comments
 (0)