Skip to content

Commit 9fbbe04

Browse files
committed
Squiz/EmbeddedPhp: closer on own line fixers should remove trailing spaces
As things were, the "closer on own line" fixers would, in most cases, leave trailing whitespace behind. For most codebases, this means the sniff would always need to be run in conjunction with a sniff which removes trailing spaces and that there is always a second fixer round needed: the first to fix the embedded PHP formatting, the second to allow the other sniff to remove the trailing spaces introduced by the fixes made by this sniff. It also makes updating the test case file fiddly. Additionally, when the next token is inline HTML, it could lead to weird/incorrect indents, which would not be fixable via a sniff (as determining intended indent for inline HTML is non-trivial). This commit fixes the "closer on own line" fixers to remove trailing spaces and prevents them from affecting the indent of inline HTML unnecessarily. Covered by pre-existing tests + additional newly added tests.
1 parent 77ebd28 commit 9fbbe04

File tree

4 files changed

+59
-2
lines changed

4 files changed

+59
-2
lines changed

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

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -233,6 +233,11 @@ private function validateMultilineEmbeddedPhp($phpcsFile, $stackPtr, $closingTag
233233
if ($fix === true) {
234234
$first = $phpcsFile->findFirstOnLine(T_WHITESPACE, $closingTag, true);
235235
$phpcsFile->fixer->beginChangeset();
236+
237+
if ($tokens[($closingTag - 1)]['code'] === T_WHITESPACE) {
238+
$phpcsFile->fixer->replaceToken(($closingTag - 1), '');
239+
}
240+
236241
$phpcsFile->fixer->addContentBefore($closingTag, str_repeat(' ', ($tokens[$first]['column'] - 1)));
237242
$phpcsFile->fixer->addNewlineBefore($closingTag);
238243
$phpcsFile->fixer->endChangeset();
@@ -245,8 +250,20 @@ private function validateMultilineEmbeddedPhp($phpcsFile, $stackPtr, $closingTag
245250
$phpcsFile->fixer->beginChangeset();
246251
$phpcsFile->fixer->addNewline($closingTag);
247252
$phpcsFile->fixer->addContent($closingTag, str_repeat(' ', ($tokens[$first]['column'] - 1)));
253+
254+
if ($tokens[$firstContentAfterBlock]['code'] === T_INLINE_HTML) {
255+
$trimmedHtmlContent = ltrim($tokens[$firstContentAfterBlock]['content']);
256+
if ($trimmedHtmlContent === '') {
257+
// HTML token contains only whitespace and the next token after is PHP, not HTML, so remove the whitespace.
258+
$phpcsFile->fixer->replaceToken($firstContentAfterBlock, '');
259+
} else {
260+
// The HTML token has content, so remove leading whitespace in favour of the indent.
261+
$phpcsFile->fixer->replaceToken($firstContentAfterBlock, $trimmedHtmlContent);
262+
}
263+
}
264+
248265
$phpcsFile->fixer->endChangeset();
249-
}
266+
}//end if
250267
}//end if
251268

252269
$next = $phpcsFile->findNext(T_OPEN_TAG, ($closingTag + 1));

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

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -151,6 +151,23 @@ Make sure the "content after opener" fixer does not leave trailing space behind.
151151
<?php echo $openerNeedsOwnLine;
152152
?>
153153

154+
<!--
155+
Make sure the "content before closer" fixer does not leave trailing space behind.
156+
-->
157+
<?php
158+
echo $closerNeedsOwnLine; ?>
159+
160+
<!--
161+
Make sure the "content after closer" fixer does not leave trailing space behind.
162+
-->
163+
<?php
164+
echo $closerNeedsOwnLine;
165+
?> </div>
166+
167+
<?php
168+
echo $closerNeedsOwnLine;
169+
?> <?php echo $i; ?>
170+
154171
<?php
155172
// This test case file MUST always end with an unclosed long open PHP tag (with this comment) to prevent
156173
// the tests running into the "last PHP closing tag excepted" condition breaking tests.

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

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@
3030
?>
3131

3232
<?php
33-
echo $closerNeedsOwnLine;
33+
echo $closerNeedsOwnLine;
3434
?>
3535
<?php
3636
echo $openerNeedsOwnLine;
@@ -146,6 +146,26 @@ Make sure the "content after opener" fixer does not leave trailing space behind.
146146
echo $openerNeedsOwnLine;
147147
?>
148148

149+
<!--
150+
Make sure the "content before closer" fixer does not leave trailing space behind.
151+
-->
152+
<?php
153+
echo $closerNeedsOwnLine;
154+
?>
155+
156+
<!--
157+
Make sure the "content after closer" fixer does not leave trailing space behind.
158+
-->
159+
<?php
160+
echo $closerNeedsOwnLine;
161+
?>
162+
</div>
163+
164+
<?php
165+
echo $closerNeedsOwnLine;
166+
?>
167+
<?php echo $i; ?>
168+
149169
<?php
150170
// This test case file MUST always end with an unclosed long open PHP tag (with this comment) to prevent
151171
// the tests running into the "last PHP closing tag excepted" condition breaking tests.

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,9 @@ public function getErrorList()
7171
142 => 1,
7272
145 => 1,
7373
151 => 1,
74+
158 => 1,
75+
165 => 1,
76+
169 => 1,
7477
];
7578

7679
}//end getErrorList()

0 commit comments

Comments
 (0)