Skip to content

Commit

Permalink
Generic/UselessOverridingMethod: improve handling of PHP open/close t…
Browse files Browse the repository at this point in the history
…ags between statements

This change prevents an edge-case false negative where a `parent::method()` function call statement ended by a PHP close tag without any "function content" of note after it, would not be reported as a useless overriding method.

Includes unit tests.

Related to 552
  • Loading branch information
jrfnl committed Jul 19, 2024
1 parent eb3454f commit 7642636
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -153,15 +153,20 @@ public function process(File $phpcsFile, $stackPtr)
}//end for

$next = $phpcsFile->findNext(Tokens::$emptyTokens, ($next + 1), null, true);
if ($tokens[$next]['code'] !== T_SEMICOLON) {
if ($tokens[$next]['code'] !== T_SEMICOLON && $tokens[$next]['code'] !== T_CLOSE_TAG) {
return;
}

// This list deliberately does not include the `T_OPEN_TAG_WITH_ECHO` as that token implicitly is an echo statement, i.e. content.
$nonContent = Tokens::$emptyTokens;
$nonContent[T_OPEN_TAG] = T_OPEN_TAG;
$nonContent[T_CLOSE_TAG] = T_CLOSE_TAG;

// Check rest of the scope.
for (++$next; $next <= $end; ++$next) {
$code = $tokens[$next]['code'];
// Skip for any other content.
if (isset(Tokens::$emptyTokens[$code]) === false) {
if (isset($nonContent[$code]) === false) {
return;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -148,3 +148,26 @@ function foo() {
}
};
}

class SniffShouldHandlePHPOpenCloseTagsCorrectly {
public function thisIsStillAUselessOverride($a, $b) {
return parent::thisIsStillAUselessOverride($a, $b) ?><?php
// Even with a comment here.
}

public function butNotWithANewLineBetweenThePHPTagsAsThenWeEchoOutTheNewLine($a, $b) {
parent::butNotWithANewLineBetweenThePHPTagsAsThenWeEchoOutTheNewLine($a, $b) ?>
<?php
}

public function embeddedHTMLAfterCallingParent() {
parent::embeddedHTMLAfterCallingParent() ?>
<div>HTML</div>
<?php
}

public function contentAfterUselessEmbedBlock() {
parent::contentAfterUselessEmbedBlock() ?><?php
return 1;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ public function getWarningList($testFile='')
116 => 1,
134 => 1,
146 => 1,
153 => 1,
];
default:
return [];
Expand Down

0 comments on commit 7642636

Please sign in to comment.