Skip to content

Squiz/EmbeddedPhp: bug fix - fixer conflict with itself #833

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Feb 20, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
117 changes: 62 additions & 55 deletions src/Standards/Squiz/Sniffs/PHP/EmbeddedPhpSniff.php
Original file line number Diff line number Diff line change
Expand Up @@ -102,8 +102,7 @@ private function validateMultilineEmbeddedPhp($phpcsFile, $stackPtr, $closingTag
$error = 'Opening PHP tag must be on a line by itself';
$fix = $phpcsFile->addFixableError($error, $stackPtr, 'ContentAfterOpen');
if ($fix === true) {
$first = $phpcsFile->findFirstOnLine(T_WHITESPACE, $stackPtr, true);
$padding = (strlen($tokens[$first]['content']) - strlen(ltrim($tokens[$first]['content'])));
$padding = $this->calculateLineIndent($phpcsFile, $stackPtr);

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

$indent = 0;
$first = $phpcsFile->findFirstOnLine(T_WHITESPACE, $stackPtr);
if ($first === false) {
$first = $phpcsFile->findFirstOnLine(T_INLINE_HTML, $stackPtr);
if ($first !== false) {
$indent = (strlen($tokens[$first]['content']) - strlen(ltrim($tokens[$first]['content'])));
}
} else {
$indent = ($tokens[($first + 1)]['column'] - 1);
}

$indent = $this->calculateLineIndent($phpcsFile, $stackPtr);
$contentColumn = ($tokens[$firstContent]['column'] - 1);
if ($contentColumn !== $indent) {
$error = 'First line of embedded PHP code must be indented %s spaces; %s found';
Expand All @@ -180,52 +169,40 @@ private function validateMultilineEmbeddedPhp($phpcsFile, $stackPtr, $closingTag

$lastContentBeforeBlock = $phpcsFile->findPrevious(T_WHITESPACE, ($stackPtr - 1), null, true);
if ($tokens[$lastContentBeforeBlock]['line'] === $tokens[$stackPtr]['line']
&& trim($tokens[$lastContentBeforeBlock]['content']) !== ''
&& (($tokens[$lastContentBeforeBlock]['code'] === T_INLINE_HTML
&& trim($tokens[$lastContentBeforeBlock]['content']) !== '')
|| ($tokens[($lastContentBeforeBlock - 1)]['code'] !== T_INLINE_HTML
&& $tokens[($lastContentBeforeBlock - 1)]['line'] === $tokens[$stackPtr]['line']))
) {
$error = 'Opening PHP tag must be on a line by itself';
$fix = $phpcsFile->addFixableError($error, $stackPtr, 'ContentBeforeOpen');
if ($fix === true) {
$padding = 0;
$first = $phpcsFile->findFirstOnLine(T_WHITESPACE, $stackPtr);
if ($first === false) {
$first = $phpcsFile->findFirstOnLine(T_INLINE_HTML, $stackPtr);
if ($first !== false) {
$padding = (strlen($tokens[$first]['content']) - strlen(ltrim($tokens[$first]['content'])));
}
} else {
$padding = ($tokens[($first + 1)]['column'] - 1);
}
$padding = $this->calculateLineIndent($phpcsFile, $lastContentBeforeBlock);

$phpcsFile->fixer->beginChangeset();
$phpcsFile->fixer->addContentBefore($stackPtr, $phpcsFile->eolChar.str_repeat(' ', $padding));
}

// Make sure we don't leave trailing whitespace behind.
if ($tokens[($stackPtr - 1)]['code'] === T_INLINE_HTML
&& trim($tokens[($stackPtr - 1)]['content']) === ''
) {
$phpcsFile->fixer->replaceToken(($stackPtr - 1), '');
}

$phpcsFile->fixer->endChangeset();
}//end if
} else {
// Find the first token on the first non-empty line we find.
for ($first = ($lastContentBeforeBlock - 1); $first > 0; $first--) {
if ($tokens[$first]['line'] === $tokens[$stackPtr]['line']) {
continue;
} else if (trim($tokens[$first]['content']) !== '') {
$first = $phpcsFile->findFirstOnLine([], $first, true);
if ($tokens[$first]['code'] === T_COMMENT
&& $tokens[$first]['content'] !== ltrim($tokens[$first]['content'])
) {
// This is a subsequent line in a star-slash comment containing leading indent.
// We'll need the first line of the comment to correctly determine the indent.
continue;
}

break;
}
}

$expected = 0;
if ($tokens[$first]['code'] === T_INLINE_HTML
&& trim($tokens[$first]['content']) !== ''
) {
$expected = (strlen($tokens[$first]['content']) - strlen(ltrim($tokens[$first]['content'])));
} else if ($tokens[$first]['code'] === T_WHITESPACE) {
$expected = ($tokens[($first + 1)]['column'] - 1);
}

$expected = $this->calculateLineIndent($phpcsFile, $first);
$expected += 4;
$found = ($tokens[$stackPtr]['column'] - 1);
if ($found > $expected) {
Expand Down Expand Up @@ -261,17 +238,7 @@ private function validateMultilineEmbeddedPhp($phpcsFile, $stackPtr, $closingTag
) {
$closerIndent = $indent;
} else {
$first = $phpcsFile->findFirstOnLine(T_WHITESPACE, $closingTag, true);

while ($tokens[$first]['code'] === T_COMMENT
&& $tokens[$first]['content'] !== ltrim($tokens[$first]['content'])
) {
// This is a subsequent line in a star-slash comment containing leading indent.
// We'll need the first line of the comment to correctly determine the indent.
$first = $phpcsFile->findFirstOnLine(T_WHITESPACE, ($first - 1), true);
}

$closerIndent = ($tokens[$first]['column'] - 1);
$closerIndent = $this->calculateLineIndent($phpcsFile, $closingTag);
}

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

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


/**
* Calculate the indent of the line containing the stackPtr.
*
* @param \PHP_CodeSniffer\Files\File $phpcsFile The file being scanned.
* @param int $stackPtr The position of the current token in the
* stack passed in $tokens.
*
* @return int
*/
private function calculateLineIndent(File $phpcsFile, $stackPtr)
{
$tokens = $phpcsFile->getTokens();

for ($firstOnLine = $stackPtr; $tokens[$firstOnLine]['column'] !== 1; $firstOnLine--);

// Check if this is a subsequent line in a star-slash comment containing leading indent.
// In that case, we'll need the first line of the comment to correctly determine the indent.
while ($tokens[$firstOnLine]['code'] === T_COMMENT
&& $tokens[$firstOnLine]['content'] !== ltrim($tokens[$firstOnLine]['content'])
) {
for (--$firstOnLine; $tokens[$firstOnLine]['column'] !== 1; $firstOnLine--);
}

$indent = 0;
if ($tokens[$firstOnLine]['code'] === T_WHITESPACE) {
$indent = ($tokens[($firstOnLine + 1)]['column'] - 1);
} else if ($tokens[$firstOnLine]['code'] === T_INLINE_HTML
|| $tokens[$firstOnLine]['code'] === T_END_HEREDOC
|| $tokens[$firstOnLine]['code'] === T_END_NOWDOC
) {
$indent = (strlen($tokens[$firstOnLine]['content']) - strlen(ltrim($tokens[$firstOnLine]['content'])));
} else if ($tokens[$firstOnLine]['code'] === T_DOC_COMMENT_WHITESPACE) {
$indent = (strlen($tokens[$firstOnLine]['content']) - strlen(ltrim($tokens[$firstOnLine]['content'])) - 1);
}

return $indent;

}//end calculateLineIndent()


}//end class
14 changes: 14 additions & 0 deletions src/Standards/Squiz/Tests/PHP/EmbeddedPhpUnitTest.1.inc
Original file line number Diff line number Diff line change
Expand Up @@ -269,6 +269,20 @@ echo 'the PHP tag is correctly indented as an indent less than the previous code

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

<?php echo 'Open tag after code'; ?> <?php
echo $j;
?>

<?php echo 'Open tag after code - indented'; ?> <?php
echo $j;
/* comment
*/ /*comment*/ ?> <?php
/**
* Docblock.
*/ ?> <?php
echo $j;
?>

<?php
// This test case file MUST always end with an unclosed long open PHP tag (with this comment) to prevent
// the tests running into the "last PHP closing tag excepted" condition breaking tests.
Expand Down
20 changes: 20 additions & 0 deletions src/Standards/Squiz/Tests/PHP/EmbeddedPhpUnitTest.1.inc.fixed
Original file line number Diff line number Diff line change
Expand Up @@ -281,6 +281,26 @@ echo 'the PHP tag is correctly indented as an indent less than the previous code

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

<?php echo 'Open tag after code'; ?>
<?php
echo $j;
?>

<?php echo 'Open tag after code - indented'; ?>
<?php
echo $j;
/* comment
*/ /*comment*/
?>
<?php
/**
* Docblock.
*/
?>
<?php
echo $j;
?>

<?php
// This test case file MUST always end with an unclosed long open PHP tag (with this comment) to prevent
// the tests running into the "last PHP closing tag excepted" condition breaking tests.
Expand Down
21 changes: 21 additions & 0 deletions src/Standards/Squiz/Tests/PHP/EmbeddedPhpUnitTest.25.inc
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
<?php

// This test case file MUST always start with a open PHP tag set (with this comment) to prevent
// the tests running into the "first PHP open tag excepted" condition breaking the tests.
// Tests related to that "first PHP open tag excepted" condition should go in separate files.

// Tests indent calculation in combination with PHP 7.3+ flexible heredoc/nowdocs.
?>

<?php
echo <<<HEREDOC
HEREDOC; ?> <?php
echo <<<'NOWDOC'
NOWDOC; ?> <?php
echo $j;
?>

<?php
// This test case file MUST always end with an unclosed open PHP tag (with this comment) to prevent
// the tests running into the "last PHP closing tag excepted" condition breaking tests.
// Tests related to that "last PHP closing tag excepted" condition should go in separate files.
25 changes: 25 additions & 0 deletions src/Standards/Squiz/Tests/PHP/EmbeddedPhpUnitTest.25.inc.fixed
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
<?php

// This test case file MUST always start with a open PHP tag set (with this comment) to prevent
// the tests running into the "first PHP open tag excepted" condition breaking the tests.
// Tests related to that "first PHP open tag excepted" condition should go in separate files.

// Tests indent calculation in combination with PHP 7.3+ flexible heredoc/nowdocs.
?>

<?php
echo <<<HEREDOC
HEREDOC;
?>
<?php
echo <<<'NOWDOC'
NOWDOC;
?>
<?php
echo $j;
?>

<?php
// This test case file MUST always end with an unclosed open PHP tag (with this comment) to prevent
// the tests running into the "last PHP closing tag excepted" condition breaking tests.
// Tests related to that "last PHP closing tag excepted" condition should go in separate files.
21 changes: 18 additions & 3 deletions src/Standards/Squiz/Tests/PHP/EmbeddedPhpUnitTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ public function getErrorList($testFile='')
169 => 1,
175 => 1,
176 => 2,
178 => 1,
178 => 2,
179 => 1,
180 => 2,
181 => 1,
Expand All @@ -101,6 +101,10 @@ public function getErrorList($testFile='')
263 => 1,
264 => 1,
270 => 1,
272 => 1,
276 => 1,
279 => 2,
282 => 2,
];

case 'EmbeddedPhpUnitTest.2.inc':
Expand Down Expand Up @@ -148,7 +152,7 @@ public function getErrorList($testFile='')
105 => 1,
111 => 1,
112 => 2,
114 => 1,
114 => 2,
115 => 1,
116 => 2,
117 => 1,
Expand Down Expand Up @@ -188,7 +192,7 @@ public function getErrorList($testFile='')
case 'EmbeddedPhpUnitTest.22.inc':
return [
14 => 1,
22 => 2,
22 => 1,
];

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

case 'EmbeddedPhpUnitTest.25.inc':
if (PHP_VERSION_ID >= 70300) {
return [
12 => 2,
14 => 2,
];
}

// PHP 7.2 or lower: PHP version which doesn't support flexible heredocs/nowdocs yet.
return [];

default:
return [];
}//end switch
Expand Down
Loading