Skip to content

Commit 769aa73

Browse files
committed
Squiz/ScopeClosingBrace: bug fix - prevent removing inline HTML when fixing
When non-empty inline HTML is on the same line, before the scope closing, the sniff would incorrectly throw the `Closing brace indented incorrectly; expected %s spaces, found %s` error, instead of the `Closing brace must be on a line by itself` error. This would ultimately lead to the fixer removing the non-empty inline HTML, potentially breaking HTML display/structure. Fixed now. Includes unit test. Fixes 3422
1 parent f619f0b commit 769aa73

File tree

4 files changed

+25
-2
lines changed

4 files changed

+25
-2
lines changed

src/Standards/Squiz/Sniffs/WhiteSpace/ScopeClosingBraceSniff.php

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -65,11 +65,20 @@ public function process(File $phpcsFile, $stackPtr)
6565

6666
// Check that the closing brace is on it's own line.
6767
$lastContent = $phpcsFile->findPrevious([T_INLINE_HTML, T_WHITESPACE, T_OPEN_TAG], ($scopeEnd - 1), $scopeStart, true);
68-
if ($tokens[$lastContent]['line'] === $tokens[$scopeEnd]['line']) {
68+
for ($lineStart = $scopeEnd; $tokens[$lineStart]['column'] > 1; $lineStart--);
69+
70+
if ($tokens[$lastContent]['line'] === $tokens[$scopeEnd]['line']
71+
|| ($tokens[$lineStart]['code'] === T_INLINE_HTML
72+
&& trim($tokens[$lineStart]['content']) !== '')
73+
) {
6974
$error = 'Closing brace must be on a line by itself';
7075
$fix = $phpcsFile->addFixableError($error, $scopeEnd, 'ContentBefore');
7176
if ($fix === true) {
72-
$phpcsFile->fixer->addNewlineBefore($scopeEnd);
77+
if ($tokens[$lastContent]['line'] === $tokens[$scopeEnd]['line']) {
78+
$phpcsFile->fixer->addNewlineBefore($scopeEnd);
79+
} else {
80+
$phpcsFile->fixer->addNewlineBefore(($lineStart + 1));
81+
}
7382
}
7483

7584
return;

src/Standards/Squiz/Tests/WhiteSpace/ScopeClosingBraceUnitTest.inc

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -114,3 +114,9 @@ $match = match ($test) {
114114
1 => 'a',
115115
2 => 'b'
116116
};
117+
118+
?>
119+
<?php if ($someConditional) : ?>
120+
<div class="container">
121+
122+
</div> <?php endif; ?>

src/Standards/Squiz/Tests/WhiteSpace/ScopeClosingBraceUnitTest.inc.fixed

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -116,3 +116,10 @@ $match = match ($test) {
116116
1 => 'a',
117117
2 => 'b'
118118
};
119+
120+
?>
121+
<?php if ($someConditional) : ?>
122+
<div class="container">
123+
124+
</div>
125+
<?php endif; ?>

src/Standards/Squiz/Tests/WhiteSpace/ScopeClosingBraceUnitTest.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ public function getErrorList()
3333
102 => 1,
3434
111 => 1,
3535
116 => 1,
36+
122 => 1,
3637
];
3738

3839
}//end getErrorList()

0 commit comments

Comments
 (0)