Skip to content

Commit b2f4d32

Browse files
jrfnljrfnl
committed
Squiz/EmbeddedPhp: empty tag set fixer should remove trailing spaces
As things were, the "empty embed" fixers would in certain circumstances leave trailing spaces 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. Aside from that, it also makes updating the test case file fiddly. This commit fixes the "empty embed" fixers to remove trailing spaces, when applicable, and consolidates the code for this in a separate method. Note: _surrounding_ HTML whitespace, which is not trailing, will not be affected by this fix as the sniff should have no opinion on that. Covered by pre-existing tests + some additional newly added tests.
1 parent 38a90c1 commit b2f4d32

File tree

4 files changed

+94
-23
lines changed

4 files changed

+94
-23
lines changed

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

Lines changed: 41 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -86,17 +86,7 @@ private function validateMultilineEmbeddedPhp($phpcsFile, $stackPtr, $closingTag
8686

8787
// We have an opening and a closing tag, that lie within other content.
8888
if ($firstContent === $closingTag) {
89-
$error = 'Empty embedded PHP tag found';
90-
$fix = $phpcsFile->addFixableError($error, $stackPtr, 'Empty');
91-
if ($fix === true) {
92-
$phpcsFile->fixer->beginChangeset();
93-
for ($i = $stackPtr; $i <= $closingTag; $i++) {
94-
$phpcsFile->fixer->replaceToken($i, '');
95-
}
96-
97-
$phpcsFile->fixer->endChangeset();
98-
}
99-
89+
$this->reportEmptyTagSet($phpcsFile, $stackPtr, $closingTag);
10090
return;
10191
}
10292
}//end if
@@ -307,17 +297,7 @@ private function validateInlineEmbeddedPhp($phpcsFile, $stackPtr, $closeTag)
307297
$firstContent = $phpcsFile->findNext(T_WHITESPACE, ($stackPtr + 1), $closeTag, true);
308298

309299
if ($firstContent === false) {
310-
$error = 'Empty embedded PHP tag found';
311-
$fix = $phpcsFile->addFixableError($error, $stackPtr, 'Empty');
312-
if ($fix === true) {
313-
$phpcsFile->fixer->beginChangeset();
314-
for ($i = $stackPtr; $i <= $closeTag; $i++) {
315-
$phpcsFile->fixer->replaceToken($i, '');
316-
}
317-
318-
$phpcsFile->fixer->endChangeset();
319-
}
320-
300+
$this->reportEmptyTagSet($phpcsFile, $stackPtr, $closeTag);
321301
return;
322302
}
323303

@@ -396,4 +376,43 @@ private function validateInlineEmbeddedPhp($phpcsFile, $stackPtr, $closeTag)
396376
}//end validateInlineEmbeddedPhp()
397377

398378

379+
/**
380+
* Report and fix an set of empty PHP tags.
381+
*
382+
* @param \PHP_CodeSniffer\Files\File $phpcsFile The file being scanned.
383+
* @param int $stackPtr The position of the current token in the
384+
* stack passed in $tokens.
385+
* @param int $closeTag The position of the PHP close tag in the
386+
* stack passed in $tokens.
387+
*
388+
* @return void
389+
*/
390+
private function reportEmptyTagSet(File $phpcsFile, $stackPtr, $closeTag)
391+
{
392+
$tokens = $phpcsFile->getTokens();
393+
$error = 'Empty embedded PHP tag found';
394+
$fix = $phpcsFile->addFixableError($error, $stackPtr, 'Empty');
395+
if ($fix === true) {
396+
$phpcsFile->fixer->beginChangeset();
397+
for ($i = $stackPtr; $i <= $closeTag; $i++) {
398+
$phpcsFile->fixer->replaceToken($i, '');
399+
}
400+
401+
// Prevent leaving indentation whitespace behind when the empty tag set is the only thing on the affected lines.
402+
if (isset($tokens[($closeTag + 1)]) === true
403+
&& $tokens[($closeTag + 1)]['line'] !== $tokens[$closeTag]['line']
404+
&& $tokens[($stackPtr - 1)]['code'] === T_INLINE_HTML
405+
&& $tokens[($stackPtr - 1)]['line'] === $tokens[$stackPtr]['line']
406+
&& $tokens[($stackPtr - 1)]['column'] === 1
407+
&& trim($tokens[($stackPtr - 1)]['content']) === ''
408+
) {
409+
$phpcsFile->fixer->replaceToken(($stackPtr - 1), '');
410+
}
411+
412+
$phpcsFile->fixer->endChangeset();
413+
}
414+
415+
}//end reportEmptyTagSet()
416+
417+
399418
}//end class

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

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -120,6 +120,31 @@ function foo()
120120
<?php echo 'too much space before close'; // phpcs:ignore Standard.Category.Sniff -- reason. ?>
121121
<?php echo 'no space before close'; // phpcs:ignore Standard.Category.Sniff -- reason.?>
122122

123+
<!--
124+
Make sure the empty tag set fixer removes the complete line, including indentation for an empty tag set and ensure
125+
that the fixer does not remove too many tokens, like new lines tokens _before_ the affected line
126+
or indentation if there is code _after_ the removed empty tag.
127+
-->
128+
<div><?php ?></div>
129+
<?php ?><?php echo $i; ?>
130+
131+
132+
<?php ?>
133+
134+
<?php if (true) { ?><?php echo $i; ?> <?php ?> <?php } ?>
135+
136+
<div><?php
137+
?></div>
138+
<?php
139+
?><?php echo $i; ?>
140+
141+
142+
<?php
143+
?>
144+
145+
<?php if (true) { ?><?php echo $i; ?> <?php
146+
?> <?php } ?>
147+
123148
<?php
124149
// This test case file MUST always end with an unclosed long open PHP tag (with this comment) to prevent
125150
// the tests running into the "last PHP closing tag excepted" condition breaking tests.

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

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@
2121
<?php echo 'fourA'; echo 'fourB;'; ?>
2222
<?php echo 'fiveA'; echo 'fiveB;'; ?>
2323

24-
24+
2525
<?php
2626
echo $start - $indent + $end;
2727
?>
@@ -120,6 +120,25 @@ function foo()
120120
<?php echo 'too much space before close'; // phpcs:ignore Standard.Category.Sniff -- reason. ?>
121121
<?php echo 'no space before close'; // phpcs:ignore Standard.Category.Sniff -- reason. ?>
122122

123+
<!--
124+
Make sure the empty tag set fixer removes the complete line, including indentation for an empty tag set and ensure
125+
that the fixer does not remove too many tokens, like new lines tokens _before_ the affected line
126+
or indentation if there is code _after_ the removed empty tag.
127+
-->
128+
<div></div>
129+
<?php echo $i; ?>
130+
131+
132+
133+
<?php if (true) { ?><?php echo $i; ?> <?php } ?>
134+
135+
<div></div>
136+
<?php echo $i; ?>
137+
138+
139+
140+
<?php if (true) { ?><?php echo $i; ?> <?php } ?>
141+
123142
<?php
124143
// This test case file MUST always end with an unclosed long open PHP tag (with this comment) to prevent
125144
// the tests running into the "last PHP closing tag excepted" condition breaking tests.

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

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,14 @@ public function getErrorList()
6262
117 => 1,
6363
120 => 1,
6464
121 => 1,
65+
128 => 1,
66+
129 => 1,
67+
132 => 1,
68+
134 => 1,
69+
136 => 1,
70+
138 => 1,
71+
142 => 1,
72+
145 => 1,
6573
];
6674

6775
}//end getErrorList()

0 commit comments

Comments
 (0)