+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
Inline HTML with indent to demonstrate the bug in the indent calculation.
+
+
+
+
+
+Inline HTML with indent to demonstrate the bug in the indent calculation.
+
+
+
+
+
+ 2,
181 => 1,
189 => 1,
+ 212 => 1,
+ 214 => 2,
+ 219 => 1,
+ 223 => 1,
+ 225 => 1,
+ 226 => 1,
+ 227 => 2,
+ 228 => 1,
];
case 'EmbeddedPhpUnitTest.2.inc':
@@ -169,6 +177,9 @@ public function getErrorList($testFile='')
case 'EmbeddedPhpUnitTest.21.inc':
return [12 => 2];
+ case 'EmbeddedPhpUnitTest.22.inc':
+ return [14 => 1];
+
default:
return [];
}//end switch
From d1c63af4ef181eb491a282d724c2698e5d270bfb Mon Sep 17 00:00:00 2001
From: jrfnl
Date: Wed, 27 Dec 2023 05:29:20 +0100
Subject: [PATCH 21/25] Squiz/EmbeddedPhp: bug fix - open tag on own line
intended indent calculation
Basically, this is the same bug as fixed in a previous commit for the intended content indent calculation.
The indentation of the first token in the file would be used to calculate the intended indentation, instead of the indentation for the first token on the line containing the PHP open tag.
Fixed now, includes test.
Note: The only reason the test doesn't actually show the bug is because of the earlier "fixer adds stray new line" bugfix in this PR, which means that the fixer for the previous close tag will prevent the buggy fixer from running.
Without _that_ fix, this bug would be visible when running with `-vvv` (indent would get set to 32 and then corrected to 4 on the next fixer round via the `OpenTagIndent` fixer).
---
src/Standards/Squiz/Sniffs/PHP/EmbeddedPhpSniff.php | 9 ++++++---
.../Squiz/Tests/PHP/EmbeddedPhpUnitTest.22.inc | 10 ++++++++++
.../Squiz/Tests/PHP/EmbeddedPhpUnitTest.22.inc.fixed | 11 +++++++++++
src/Standards/Squiz/Tests/PHP/EmbeddedPhpUnitTest.php | 5 ++++-
4 files changed, 31 insertions(+), 4 deletions(-)
diff --git a/src/Standards/Squiz/Sniffs/PHP/EmbeddedPhpSniff.php b/src/Standards/Squiz/Sniffs/PHP/EmbeddedPhpSniff.php
index c88864f360..99ff3968db 100644
--- a/src/Standards/Squiz/Sniffs/PHP/EmbeddedPhpSniff.php
+++ b/src/Standards/Squiz/Sniffs/PHP/EmbeddedPhpSniff.php
@@ -185,10 +185,13 @@ private function validateMultilineEmbeddedPhp($phpcsFile, $stackPtr, $closingTag
$error = 'Opening PHP tag must be on a line by itself';
$fix = $phpcsFile->addFixableError($error, $stackPtr, 'ContentBeforeOpen');
if ($fix === true) {
- $first = $phpcsFile->findFirstOnLine(T_WHITESPACE, $stackPtr);
+ $padding = 0;
+ $first = $phpcsFile->findFirstOnLine(T_WHITESPACE, $stackPtr);
if ($first === false) {
- $first = $phpcsFile->findFirstOnLine(T_INLINE_HTML, $stackPtr);
- $padding = (strlen($tokens[$first]['content']) - strlen(ltrim($tokens[$first]['content'])));
+ $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);
}
diff --git a/src/Standards/Squiz/Tests/PHP/EmbeddedPhpUnitTest.22.inc b/src/Standards/Squiz/Tests/PHP/EmbeddedPhpUnitTest.22.inc
index 8d08da0313..5196a3b45b 100644
--- a/src/Standards/Squiz/Tests/PHP/EmbeddedPhpUnitTest.22.inc
+++ b/src/Standards/Squiz/Tests/PHP/EmbeddedPhpUnitTest.22.inc
@@ -14,6 +14,16 @@ echo 'indent is correct - first on line for open tag is open tag';
echo 'indent is incorrect - first on line for open tag is inline HTML';
?>
+
+
+
+
+
+
+
2];
case 'EmbeddedPhpUnitTest.22.inc':
- return [14 => 1];
+ return [
+ 14 => 1,
+ 22 => 2,
+ ];
default:
return [];
From b4c16825d2454ed5d832d8de22ce7c9c4b85bc0f Mon Sep 17 00:00:00 2001
From: jrfnl
Date: Wed, 27 Dec 2023 07:57:56 +0100
Subject: [PATCH 22/25] Squiz/EmbeddedPhp: bug fix - close tag intended indent
calculation
Yet another indentation calculation bug.
If the close tag is not on its own line, but is on a line containing a subsequent line of a star-slash style comment, the indentation would not be calculated correctly as the comment token will include the indentation.
Fixed now, includes test.
---
.../Squiz/Sniffs/PHP/EmbeddedPhpSniff.php | 11 ++++++++++-
.../Squiz/Tests/PHP/EmbeddedPhpUnitTest.1.inc | 12 ++++++++++++
.../Tests/PHP/EmbeddedPhpUnitTest.1.inc.fixed | 14 ++++++++++++++
.../Squiz/Tests/PHP/EmbeddedPhpUnitTest.php | 2 ++
4 files changed, 38 insertions(+), 1 deletion(-)
diff --git a/src/Standards/Squiz/Sniffs/PHP/EmbeddedPhpSniff.php b/src/Standards/Squiz/Sniffs/PHP/EmbeddedPhpSniff.php
index 99ff3968db..99e9e94e98 100644
--- a/src/Standards/Squiz/Sniffs/PHP/EmbeddedPhpSniff.php
+++ b/src/Standards/Squiz/Sniffs/PHP/EmbeddedPhpSniff.php
@@ -253,7 +253,16 @@ private function validateMultilineEmbeddedPhp($phpcsFile, $stackPtr, $closingTag
) {
$closerIndent = $indent;
} else {
- $first = $phpcsFile->findFirstOnLine(T_WHITESPACE, $closingTag, true);
+ $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);
}
diff --git a/src/Standards/Squiz/Tests/PHP/EmbeddedPhpUnitTest.1.inc b/src/Standards/Squiz/Tests/PHP/EmbeddedPhpUnitTest.1.inc
index ddf0c6c918..f2be4f7bce 100644
--- a/src/Standards/Squiz/Tests/PHP/EmbeddedPhpUnitTest.1.inc
+++ b/src/Standards/Squiz/Tests/PHP/EmbeddedPhpUnitTest.1.inc
@@ -228,6 +228,18 @@ echo 'indent is correct - first on line for open tag is open tag';
echo 'indent is incorrect - first on line for open tag is whitespace';
?>
+
+
+
+
+
+
+
+
+
+
1,
227 => 2,
228 => 1,
+ 235 => 1,
+ 241 => 1,
];
case 'EmbeddedPhpUnitTest.2.inc':
From 46f710263ddb7cd7e857be79b5216aa71fefb196 Mon Sep 17 00:00:00 2001
From: jrfnl
Date: Wed, 27 Dec 2023 08:08:46 +0100
Subject: [PATCH 23/25] Squiz/EmbeddedPhp: bug fix - open tag intended indent
calculation
Basically, this is the same bug as fixed in a previous commit for the intended close tag indent calculation.
If the first token on the line before the open tag would be a subsequent line for a star-slash comment, the indentation would not be calculated correctly, but would always be set to `0`, even when there was indentation.
Fixed now, includes test.
---
.../Squiz/Sniffs/PHP/EmbeddedPhpSniff.php | 10 ++++++-
.../Squiz/Tests/PHP/EmbeddedPhpUnitTest.1.inc | 25 ++++++++++++++++
.../Tests/PHP/EmbeddedPhpUnitTest.1.inc.fixed | 29 +++++++++++++++++++
.../Squiz/Tests/PHP/EmbeddedPhpUnitTest.php | 5 ++++
4 files changed, 68 insertions(+), 1 deletion(-)
diff --git a/src/Standards/Squiz/Sniffs/PHP/EmbeddedPhpSniff.php b/src/Standards/Squiz/Sniffs/PHP/EmbeddedPhpSniff.php
index 99e9e94e98..be75265917 100644
--- a/src/Standards/Squiz/Sniffs/PHP/EmbeddedPhpSniff.php
+++ b/src/Standards/Squiz/Sniffs/PHP/EmbeddedPhpSniff.php
@@ -200,11 +200,19 @@ private function validateMultilineEmbeddedPhp($phpcsFile, $stackPtr, $closingTag
}
} else {
// Find the first token on the first non-empty line we find.
- for ($first = ($stackPtr - 1); $first > 0; $first--) {
+ 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;
}
}
diff --git a/src/Standards/Squiz/Tests/PHP/EmbeddedPhpUnitTest.1.inc b/src/Standards/Squiz/Tests/PHP/EmbeddedPhpUnitTest.1.inc
index f2be4f7bce..f28c559894 100644
--- a/src/Standards/Squiz/Tests/PHP/EmbeddedPhpUnitTest.1.inc
+++ b/src/Standards/Squiz/Tests/PHP/EmbeddedPhpUnitTest.1.inc
@@ -240,6 +240,31 @@ Safeguard correct close tag indent calculation.
This means the calculated indent would be 0 unless we
walk to the first line in the comment. */ ?>
+
+
+
+
+
+
+
+
+
+
+
+
+
+
1,
235 => 1,
241 => 1,
+ 248 => 1,
+ 253 => 1,
+ 258 => 1,
+ 263 => 1,
+ 264 => 1,
];
case 'EmbeddedPhpUnitTest.2.inc':
From ac5fd07fa2db2335fd739e19c34e39ad923bc758 Mon Sep 17 00:00:00 2001
From: jrfnl
Date: Wed, 27 Dec 2023 10:54:43 +0100
Subject: [PATCH 24/25] Squiz/EmbeddedPhp: add some extra defensive coding
`$firstContentAfterBlock` should not be able to be `false` at this point as it is already checked and handled at the top of the function, but better to be safe than sorry.
---
src/Standards/Squiz/Sniffs/PHP/EmbeddedPhpSniff.php | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/src/Standards/Squiz/Sniffs/PHP/EmbeddedPhpSniff.php b/src/Standards/Squiz/Sniffs/PHP/EmbeddedPhpSniff.php
index be75265917..0be6118e43 100644
--- a/src/Standards/Squiz/Sniffs/PHP/EmbeddedPhpSniff.php
+++ b/src/Standards/Squiz/Sniffs/PHP/EmbeddedPhpSniff.php
@@ -284,7 +284,9 @@ private function validateMultilineEmbeddedPhp($phpcsFile, $stackPtr, $closingTag
$phpcsFile->fixer->addNewlineBefore($closingTag);
$phpcsFile->fixer->endChangeset();
}//end if
- } else if ($tokens[$firstContentAfterBlock]['line'] === $tokens[$closingTag]['line']) {
+ } else if ($firstContentAfterBlock !== false
+ && $tokens[$firstContentAfterBlock]['line'] === $tokens[$closingTag]['line']
+ ) {
$error = 'Closing PHP tag must be on a line by itself';
$fix = $phpcsFile->addFixableError($error, $closingTag, 'ContentAfterEnd');
if ($fix === true) {
From 50ae189f4bf6375e3fc5d67198d55d3b9639b464 Mon Sep 17 00:00:00 2001
From: jrfnl
Date: Wed, 27 Dec 2023 11:20:21 +0100
Subject: [PATCH 25/25] Squiz/EmbeddedPhp: document the behaviour for closed
tag block at end of file [3]
While the last tag block in a file will **not** be ignored if it is followed by non-empty inline HTML, the "blank lines before the close tag" check _will_ be ignored if this is the last embedded PHP block.
This test documents this behaviour.
---
.../Tests/PHP/EmbeddedPhpUnitTest.23.inc | 23 +++++++++++++++++++
1 file changed, 23 insertions(+)
create mode 100644 src/Standards/Squiz/Tests/PHP/EmbeddedPhpUnitTest.23.inc
diff --git a/src/Standards/Squiz/Tests/PHP/EmbeddedPhpUnitTest.23.inc b/src/Standards/Squiz/Tests/PHP/EmbeddedPhpUnitTest.23.inc
new file mode 100644
index 0000000000..6642ef6b63
--- /dev/null
+++ b/src/Standards/Squiz/Tests/PHP/EmbeddedPhpUnitTest.23.inc
@@ -0,0 +1,23 @@
+
+
+
+
+
+
+Some more content after the last PHP tag block.