Skip to content

Commit

Permalink
PEAR/ScopeClosingBrace: prevent fixer conflict with itself
Browse files Browse the repository at this point in the history
The `PEAR.WhiteSpace.ScopeClosingBrace` sniff could get into a conflict with itself when the scope closer was preceded by a PHP open tag and before that non-empty (i.e. non-indent) inline HTML.

In that case, the sniff would not recognize that the close brace was not on a line by itself, as the search for the last content before would disregard the non-empty HTML and would continue searching, which meant that the "last relevant content before" token would point to a token on a previous line.

This, in turn, then led to the sniff continuing on to the next error "Closing brace indented incorrectly", where the indent would now be incorrectly determined as `-1`, which in the fixer would lead to the original content and the replacement content being exactly the same, which created a fixer conflict.

Fixed now by improving the "last content before" determination.

Includes unit test.

Related to 152
  • Loading branch information
jrfnl committed Mar 29, 2024
1 parent c5e1807 commit 7903dd7
Show file tree
Hide file tree
Showing 4 changed files with 27 additions and 10 deletions.
23 changes: 13 additions & 10 deletions src/Standards/PEAR/Sniffs/WhiteSpace/ScopeClosingBraceSniff.php
Original file line number Diff line number Diff line change
Expand Up @@ -93,16 +93,19 @@ public function process(File $phpcsFile, $stackPtr)
}

// Check that the closing brace is on it's own line.
$lastContent = $phpcsFile->findPrevious(
[
T_WHITESPACE,
T_INLINE_HTML,
T_OPEN_TAG,
],
($scopeEnd - 1),
$scopeStart,
true
);
for ($lastContent = ($scopeEnd - 1); $lastContent > $scopeStart; $lastContent--) {
if ($tokens[$lastContent]['code'] === T_WHITESPACE || $tokens[$lastContent]['code'] === T_OPEN_TAG) {
continue;
}

if ($tokens[$lastContent]['code'] === T_INLINE_HTML
&& ltrim($tokens[$lastContent]['content']) === ''
) {
continue;
}

break;
}

if ($tokens[$lastContent]['line'] === $tokens[$scopeEnd]['line']) {
$error = 'Closing brace must be on a line by itself';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -162,3 +162,9 @@ enum Suits {}
enum Cards
{
}

?>
<!-- Prevent fixer conflict with itself when the scope closer is preceded by non-empty/non-indentation inline HTML. -->
<?php if ($someConditional) : ?>
<div class="container">
</div> <?php endif;
Original file line number Diff line number Diff line change
Expand Up @@ -168,3 +168,10 @@ enum Suits {
enum Cards
{
}

?>
<!-- Prevent fixer conflict with itself when the scope closer is preceded by non-empty/non-indentation inline HTML. -->
<?php if ($someConditional) : ?>
<div class="container">
</div> <?php
endif;
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ public function getErrorList()
154 => 1,
160 => 1,
164 => 1,
170 => 1,
];

}//end getErrorList()
Expand Down

0 comments on commit 7903dd7

Please sign in to comment.