Skip to content

Commit e9e2011

Browse files
committed
Squiz/EmbeddedPhp: bug fix - closer on own line indent is not calculated correctly
When the PHP close tag is on the same line as the first PHP content in a multi-line embedded PHP block and the indent of the first PHP content in the block is incorrect, the indent for the close tag when it is moved to its own line will be based on the (incorrect) indent of the first PHP content line and not on the corrected indent for the first PHP content, which the indent fixer uses. This commit fixes this. Covered by pre-existing tests and adjusting the "fixed" expectations for those. Additionally, an extra test has been added for the case when `$indent` is not pre-calculated (= when the first content in the block is a scope closer).
1 parent 1016d40 commit e9e2011

File tree

6 files changed

+33
-5
lines changed

6 files changed

+33
-5
lines changed

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

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -238,17 +238,29 @@ private function validateMultilineEmbeddedPhp($phpcsFile, $stackPtr, $closingTag
238238
$error = 'Closing PHP tag must be on a line by itself';
239239
$fix = $phpcsFile->addFixableError($error, $closingTag, 'ContentBeforeEnd');
240240
if ($fix === true) {
241-
$first = $phpcsFile->findFirstOnLine(T_WHITESPACE, $closingTag, true);
241+
// Calculate the indent for the close tag.
242+
// If the close tag is on the same line as the first content, re-use the indent
243+
// calculated for the first content line to prevent the indent being based on an
244+
// "old" indent, not the _new_ (fixed) indent.
245+
if ($tokens[$firstContent]['line'] === $tokens[$lastContent]['line']
246+
&& isset($indent) === true
247+
) {
248+
$closerIndent = $indent;
249+
} else {
250+
$first = $phpcsFile->findFirstOnLine(T_WHITESPACE, $closingTag, true);
251+
$closerIndent = ($tokens[$first]['column'] - 1);
252+
}
253+
242254
$phpcsFile->fixer->beginChangeset();
243255

244256
if ($tokens[($closingTag - 1)]['code'] === T_WHITESPACE) {
245257
$phpcsFile->fixer->replaceToken(($closingTag - 1), '');
246258
}
247259

248-
$phpcsFile->fixer->addContentBefore($closingTag, str_repeat(' ', ($tokens[$first]['column'] - 1)));
260+
$phpcsFile->fixer->addContentBefore($closingTag, str_repeat(' ', $closerIndent));
249261
$phpcsFile->fixer->addNewlineBefore($closingTag);
250262
$phpcsFile->fixer->endChangeset();
251-
}
263+
}//end if
252264
} else if ($tokens[$firstContentAfterBlock]['line'] === $tokens[$closingTag]['line']) {
253265
$error = 'Closing PHP tag must be on a line by itself';
254266
$fix = $phpcsFile->addFixableError($error, $closingTag, 'ContentAfterEnd');

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

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -181,6 +181,13 @@ Make sure the fixer does not add stray new lines when there are consecutive PHP
181181
echo 'embedded';
182182
?>
183183

184+
<!--
185+
Safeguard closing tag indent calculation for when the last content on the close tag line is a scope closer.
186+
-->
187+
<?php if (true) { ?>
188+
<?php
189+
} ?>
190+
184191
<?php
185192
// This test case file MUST always end with an unclosed long open PHP tag (with this comment) to prevent
186193
// the tests running into the "last PHP closing tag excepted" condition breaking tests.

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

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -182,6 +182,14 @@ Make sure the fixer does not add stray new lines when there are consecutive PHP
182182
echo 'embedded';
183183
?>
184184

185+
<!--
186+
Safeguard closing tag indent calculation for when the last content on the close tag line is a scope closer.
187+
-->
188+
<?php if (true) { ?>
189+
<?php
190+
}
191+
?>
192+
185193
<?php
186194
// This test case file MUST always end with an unclosed long open PHP tag (with this comment) to prevent
187195
// the tests running into the "last PHP closing tag excepted" condition breaking tests.

src/Standards/Squiz/Tests/PHP/EmbeddedPhpUnitTest.20.inc.fixed

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ as long as it is followed by non-empty inline HTML.
1010
<div>
1111
<?php
1212
echo 'too much indent and close tag not on own line';
13-
?>
13+
?>
1414

1515
</div>
1616
<p>Some more content after the last PHP tag block.</p>

src/Standards/Squiz/Tests/PHP/EmbeddedPhpUnitTest.21.inc.fixed

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ as long as it is followed by non-empty inline HTML.
1010
<div>
1111
<?=
1212
'too much indent and close tag not on own line';
13-
?>
13+
?>
1414

1515
</div>
1616
<p>Some more content after the last PHP tag block.</p>

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,7 @@ public function getErrorList($testFile='')
8484
179 => 1,
8585
180 => 2,
8686
181 => 1,
87+
189 => 1,
8788
];
8889

8990
case 'EmbeddedPhpUnitTest.2.inc':

0 commit comments

Comments
 (0)