Skip to content

Commit f668a5b

Browse files
jrfnljrfnl
authored
Merge pull request #833 from PHPCSStandards/feature/squiz-embeddedphp-fixer-conflict
Squiz/EmbeddedPhp: bug fix - fixer conflict with itself
2 parents 2f7ee70 + e9ef421 commit f668a5b

6 files changed

+160
-58
lines changed

‎src/Standards/Squiz/Sniffs/PHP/EmbeddedPhpSniff.php

Lines changed: 62 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -102,8 +102,7 @@ private function validateMultilineEmbeddedPhp($phpcsFile, $stackPtr, $closingTag
102102
$error = 'Opening PHP tag must be on a line by itself';
103103
$fix = $phpcsFile->addFixableError($error, $stackPtr, 'ContentAfterOpen');
104104
if ($fix === true) {
105-
$first = $phpcsFile->findFirstOnLine(T_WHITESPACE, $stackPtr, true);
106-
$padding = (strlen($tokens[$first]['content']) - strlen(ltrim($tokens[$first]['content'])));
105+
$padding = $this->calculateLineIndent($phpcsFile, $stackPtr);
107106

108107
$phpcsFile->fixer->beginChangeset();
109108
$phpcsFile->fixer->replaceToken($stackPtr, rtrim($tokens[$stackPtr]['content']));
@@ -147,17 +146,7 @@ private function validateMultilineEmbeddedPhp($phpcsFile, $stackPtr, $closingTag
147146
}
148147
}//end if
149148

150-
$indent = 0;
151-
$first = $phpcsFile->findFirstOnLine(T_WHITESPACE, $stackPtr);
152-
if ($first === false) {
153-
$first = $phpcsFile->findFirstOnLine(T_INLINE_HTML, $stackPtr);
154-
if ($first !== false) {
155-
$indent = (strlen($tokens[$first]['content']) - strlen(ltrim($tokens[$first]['content'])));
156-
}
157-
} else {
158-
$indent = ($tokens[($first + 1)]['column'] - 1);
159-
}
160-
149+
$indent = $this->calculateLineIndent($phpcsFile, $stackPtr);
161150
$contentColumn = ($tokens[$firstContent]['column'] - 1);
162151
if ($contentColumn !== $indent) {
163152
$error = 'First line of embedded PHP code must be indented %s spaces; %s found';
@@ -180,52 +169,40 @@ private function validateMultilineEmbeddedPhp($phpcsFile, $stackPtr, $closingTag
180169

181170
$lastContentBeforeBlock = $phpcsFile->findPrevious(T_WHITESPACE, ($stackPtr - 1), null, true);
182171
if ($tokens[$lastContentBeforeBlock]['line'] === $tokens[$stackPtr]['line']
183-
&& trim($tokens[$lastContentBeforeBlock]['content']) !== ''
172+
&& (($tokens[$lastContentBeforeBlock]['code'] === T_INLINE_HTML
173+
&& trim($tokens[$lastContentBeforeBlock]['content']) !== '')
174+
|| ($tokens[($lastContentBeforeBlock - 1)]['code'] !== T_INLINE_HTML
175+
&& $tokens[($lastContentBeforeBlock - 1)]['line'] === $tokens[$stackPtr]['line']))
184176
) {
185177
$error = 'Opening PHP tag must be on a line by itself';
186178
$fix = $phpcsFile->addFixableError($error, $stackPtr, 'ContentBeforeOpen');
187179
if ($fix === true) {
188-
$padding = 0;
189-
$first = $phpcsFile->findFirstOnLine(T_WHITESPACE, $stackPtr);
190-
if ($first === false) {
191-
$first = $phpcsFile->findFirstOnLine(T_INLINE_HTML, $stackPtr);
192-
if ($first !== false) {
193-
$padding = (strlen($tokens[$first]['content']) - strlen(ltrim($tokens[$first]['content'])));
194-
}
195-
} else {
196-
$padding = ($tokens[($first + 1)]['column'] - 1);
197-
}
180+
$padding = $this->calculateLineIndent($phpcsFile, $lastContentBeforeBlock);
198181

182+
$phpcsFile->fixer->beginChangeset();
199183
$phpcsFile->fixer->addContentBefore($stackPtr, $phpcsFile->eolChar.str_repeat(' ', $padding));
200-
}
184+
185+
// Make sure we don't leave trailing whitespace behind.
186+
if ($tokens[($stackPtr - 1)]['code'] === T_INLINE_HTML
187+
&& trim($tokens[($stackPtr - 1)]['content']) === ''
188+
) {
189+
$phpcsFile->fixer->replaceToken(($stackPtr - 1), '');
190+
}
191+
192+
$phpcsFile->fixer->endChangeset();
193+
}//end if
201194
} else {
202195
// Find the first token on the first non-empty line we find.
203196
for ($first = ($lastContentBeforeBlock - 1); $first > 0; $first--) {
204197
if ($tokens[$first]['line'] === $tokens[$stackPtr]['line']) {
205198
continue;
206199
} else if (trim($tokens[$first]['content']) !== '') {
207200
$first = $phpcsFile->findFirstOnLine([], $first, true);
208-
if ($tokens[$first]['code'] === T_COMMENT
209-
&& $tokens[$first]['content'] !== ltrim($tokens[$first]['content'])
210-
) {
211-
// This is a subsequent line in a star-slash comment containing leading indent.
212-
// We'll need the first line of the comment to correctly determine the indent.
213-
continue;
214-
}
215-
216201
break;
217202
}
218203
}
219204

220-
$expected = 0;
221-
if ($tokens[$first]['code'] === T_INLINE_HTML
222-
&& trim($tokens[$first]['content']) !== ''
223-
) {
224-
$expected = (strlen($tokens[$first]['content']) - strlen(ltrim($tokens[$first]['content'])));
225-
} else if ($tokens[$first]['code'] === T_WHITESPACE) {
226-
$expected = ($tokens[($first + 1)]['column'] - 1);
227-
}
228-
205+
$expected = $this->calculateLineIndent($phpcsFile, $first);
229206
$expected += 4;
230207
$found = ($tokens[$stackPtr]['column'] - 1);
231208
if ($found > $expected) {
@@ -261,17 +238,7 @@ private function validateMultilineEmbeddedPhp($phpcsFile, $stackPtr, $closingTag
261238
) {
262239
$closerIndent = $indent;
263240
} else {
264-
$first = $phpcsFile->findFirstOnLine(T_WHITESPACE, $closingTag, true);
265-
266-
while ($tokens[$first]['code'] === T_COMMENT
267-
&& $tokens[$first]['content'] !== ltrim($tokens[$first]['content'])
268-
) {
269-
// This is a subsequent line in a star-slash comment containing leading indent.
270-
// We'll need the first line of the comment to correctly determine the indent.
271-
$first = $phpcsFile->findFirstOnLine(T_WHITESPACE, ($first - 1), true);
272-
}
273-
274-
$closerIndent = ($tokens[$first]['column'] - 1);
241+
$closerIndent = $this->calculateLineIndent($phpcsFile, $closingTag);
275242
}
276243

277244
$phpcsFile->fixer->beginChangeset();
@@ -290,10 +257,10 @@ private function validateMultilineEmbeddedPhp($phpcsFile, $stackPtr, $closingTag
290257
$error = 'Closing PHP tag must be on a line by itself';
291258
$fix = $phpcsFile->addFixableError($error, $closingTag, 'ContentAfterEnd');
292259
if ($fix === true) {
293-
$first = $phpcsFile->findFirstOnLine(T_WHITESPACE, $closingTag, true);
260+
$indent = $this->calculateLineIndent($phpcsFile, $closingTag);
294261
$phpcsFile->fixer->beginChangeset();
295262
$phpcsFile->fixer->addNewline($closingTag);
296-
$phpcsFile->fixer->addContent($closingTag, str_repeat(' ', ($tokens[$first]['column'] - 1)));
263+
$phpcsFile->fixer->addContent($closingTag, str_repeat(' ', $indent));
297264

298265
if ($tokens[$firstContentAfterBlock]['code'] === T_INLINE_HTML) {
299266
$trimmedHtmlContent = ltrim($tokens[$firstContentAfterBlock]['content']);
@@ -513,4 +480,44 @@ private function reportEmptyTagSet(File $phpcsFile, $stackPtr, $closeTag)
513480
}//end reportEmptyTagSet()
514481

515482

483+
/**
484+
* Calculate the indent of the line containing the stackPtr.
485+
*
486+
* @param \PHP_CodeSniffer\Files\File $phpcsFile The file being scanned.
487+
* @param int $stackPtr The position of the current token in the
488+
* stack passed in $tokens.
489+
*
490+
* @return int
491+
*/
492+
private function calculateLineIndent(File $phpcsFile, $stackPtr)
493+
{
494+
$tokens = $phpcsFile->getTokens();
495+
496+
for ($firstOnLine = $stackPtr; $tokens[$firstOnLine]['column'] !== 1; $firstOnLine--);
497+
498+
// Check if this is a subsequent line in a star-slash comment containing leading indent.
499+
// In that case, we'll need the first line of the comment to correctly determine the indent.
500+
while ($tokens[$firstOnLine]['code'] === T_COMMENT
501+
&& $tokens[$firstOnLine]['content'] !== ltrim($tokens[$firstOnLine]['content'])
502+
) {
503+
for (--$firstOnLine; $tokens[$firstOnLine]['column'] !== 1; $firstOnLine--);
504+
}
505+
506+
$indent = 0;
507+
if ($tokens[$firstOnLine]['code'] === T_WHITESPACE) {
508+
$indent = ($tokens[($firstOnLine + 1)]['column'] - 1);
509+
} else if ($tokens[$firstOnLine]['code'] === T_INLINE_HTML
510+
|| $tokens[$firstOnLine]['code'] === T_END_HEREDOC
511+
|| $tokens[$firstOnLine]['code'] === T_END_NOWDOC
512+
) {
513+
$indent = (strlen($tokens[$firstOnLine]['content']) - strlen(ltrim($tokens[$firstOnLine]['content'])));
514+
} else if ($tokens[$firstOnLine]['code'] === T_DOC_COMMENT_WHITESPACE) {
515+
$indent = (strlen($tokens[$firstOnLine]['content']) - strlen(ltrim($tokens[$firstOnLine]['content'])) - 1);
516+
}
517+
518+
return $indent;
519+
520+
}//end calculateLineIndent()
521+
522+
516523
}//end class

‎src/Standards/Squiz/Tests/PHP/EmbeddedPhpUnitTest.1.inc

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -269,6 +269,20 @@ echo 'the PHP tag is correctly indented as an indent less than the previous code
269269

270270
<?PHP echo 'Extra space after uppercase long open tag '; ?>
271271

272+
<?php echo 'Open tag after code'; ?> <?php
273+
echo $j;
274+
?>
275+
276+
<?php echo 'Open tag after code - indented'; ?> <?php
277+
echo $j;
278+
/* comment
279+
*/ /*comment*/ ?> <?php
280+
/**
281+
* Docblock.
282+
*/ ?> <?php
283+
echo $j;
284+
?>
285+
272286
<?php
273287
// This test case file MUST always end with an unclosed long open PHP tag (with this comment) to prevent
274288
// the tests running into the "last PHP closing tag excepted" condition breaking tests.

‎src/Standards/Squiz/Tests/PHP/EmbeddedPhpUnitTest.1.inc.fixed

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -281,6 +281,26 @@ echo 'the PHP tag is correctly indented as an indent less than the previous code
281281

282282
<?PHP echo 'Extra space after uppercase long open tag '; ?>
283283

284+
<?php echo 'Open tag after code'; ?>
285+
<?php
286+
echo $j;
287+
?>
288+
289+
<?php echo 'Open tag after code - indented'; ?>
290+
<?php
291+
echo $j;
292+
/* comment
293+
*/ /*comment*/
294+
?>
295+
<?php
296+
/**
297+
* Docblock.
298+
*/
299+
?>
300+
<?php
301+
echo $j;
302+
?>
303+
284304
<?php
285305
// This test case file MUST always end with an unclosed long open PHP tag (with this comment) to prevent
286306
// the tests running into the "last PHP closing tag excepted" condition breaking tests.

‎src/Standards/Squiz/Tests/PHP/EmbeddedPhpUnitTest.25.inc

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
<?php
2+
3+
// This test case file MUST always start with a open PHP tag set (with this comment) to prevent
4+
// the tests running into the "first PHP open tag excepted" condition breaking the tests.
5+
// Tests related to that "first PHP open tag excepted" condition should go in separate files.
6+
7+
// Tests indent calculation in combination with PHP 7.3+ flexible heredoc/nowdocs.
8+
?>
9+
10+
<?php
11+
echo <<<HEREDOC
12+
HEREDOC; ?> <?php
13+
echo <<<'NOWDOC'
14+
NOWDOC; ?> <?php
15+
echo $j;
16+
?>
17+
18+
<?php
19+
// This test case file MUST always end with an unclosed open PHP tag (with this comment) to prevent
20+
// the tests running into the "last PHP closing tag excepted" condition breaking tests.
21+
// Tests related to that "last PHP closing tag excepted" condition should go in separate files.

‎src/Standards/Squiz/Tests/PHP/EmbeddedPhpUnitTest.25.inc.fixed

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
<?php
2+
3+
// This test case file MUST always start with a open PHP tag set (with this comment) to prevent
4+
// the tests running into the "first PHP open tag excepted" condition breaking the tests.
5+
// Tests related to that "first PHP open tag excepted" condition should go in separate files.
6+
7+
// Tests indent calculation in combination with PHP 7.3+ flexible heredoc/nowdocs.
8+
?>
9+
10+
<?php
11+
echo <<<HEREDOC
12+
HEREDOC;
13+
?>
14+
<?php
15+
echo <<<'NOWDOC'
16+
NOWDOC;
17+
?>
18+
<?php
19+
echo $j;
20+
?>
21+
22+
<?php
23+
// This test case file MUST always end with an unclosed open PHP tag (with this comment) to prevent
24+
// the tests running into the "last PHP closing tag excepted" condition breaking tests.
25+
// Tests related to that "last PHP closing tag excepted" condition should go in separate files.

‎src/Standards/Squiz/Tests/PHP/EmbeddedPhpUnitTest.php

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,7 @@ public function getErrorList($testFile='')
8080
169 => 1,
8181
175 => 1,
8282
176 => 2,
83-
178 => 1,
83+
178 => 2,
8484
179 => 1,
8585
180 => 2,
8686
181 => 1,
@@ -101,6 +101,10 @@ public function getErrorList($testFile='')
101101
263 => 1,
102102
264 => 1,
103103
270 => 1,
104+
272 => 1,
105+
276 => 1,
106+
279 => 2,
107+
282 => 2,
104108
];
105109

106110
case 'EmbeddedPhpUnitTest.2.inc':
@@ -148,7 +152,7 @@ public function getErrorList($testFile='')
148152
105 => 1,
149153
111 => 1,
150154
112 => 2,
151-
114 => 1,
155+
114 => 2,
152156
115 => 1,
153157
116 => 2,
154158
117 => 1,
@@ -188,7 +192,7 @@ public function getErrorList($testFile='')
188192
case 'EmbeddedPhpUnitTest.22.inc':
189193
return [
190194
14 => 1,
191-
22 => 2,
195+
22 => 1,
192196
];
193197

194198
case 'EmbeddedPhpUnitTest.24.inc':
@@ -201,6 +205,17 @@ public function getErrorList($testFile='')
201205
}
202206
return [];
203207

208+
case 'EmbeddedPhpUnitTest.25.inc':
209+
if (PHP_VERSION_ID >= 70300) {
210+
return [
211+
12 => 2,
212+
14 => 2,
213+
];
214+
}
215+
216+
// PHP 7.2 or lower: PHP version which doesn't support flexible heredocs/nowdocs yet.
217+
return [];
218+
204219
default:
205220
return [];
206221
}//end switch

0 commit comments

Comments
 (0)