diff --git a/src/Standards/Generic/Sniffs/CodeAnalysis/UselessOverridingMethodSniff.php b/src/Standards/Generic/Sniffs/CodeAnalysis/UselessOverridingMethodSniff.php index 1b79fd3393..4c984a16b1 100644 --- a/src/Standards/Generic/Sniffs/CodeAnalysis/UselessOverridingMethodSniff.php +++ b/src/Standards/Generic/Sniffs/CodeAnalysis/UselessOverridingMethodSniff.php @@ -28,6 +28,17 @@ class UselessOverridingMethodSniff implements Sniff { + /** + * Object-Oriented scopes in which a call to parent::method() can exist. + * + * @var array Keys are the token constants, value is irrelevant. + */ + private $validOOScopes = [ + T_CLASS => true, + T_ANON_CLASS => true, + T_TRAIT => true, + ]; + /** * Registers the tokens that this sniff wants to listen for. @@ -56,7 +67,15 @@ public function process(File $phpcsFile, $stackPtr) $token = $tokens[$stackPtr]; // Skip function without body. - if (isset($token['scope_opener']) === false) { + if (isset($token['scope_opener'], $token['scope_closer']) === false) { + return; + } + + $conditions = $token['conditions']; + $lastCondition = end($conditions); + + // Skip functions that are not a method part of a class, anon class or trait. + if (isset($this->validOOScopes[$lastCondition]) === false) { return; } @@ -93,15 +112,15 @@ public function process(File $phpcsFile, $stackPtr) $next = $phpcsFile->findNext(Tokens::$emptyTokens, ($next + 1), null, true); // Skip for invalid code. - if ($next === false || $tokens[$next]['code'] !== T_DOUBLE_COLON) { + if ($tokens[$next]['code'] !== T_DOUBLE_COLON) { return; } - // Find next non empty token index, should be the function name. + // Find next non empty token index, should be the name of the method being called. $next = $phpcsFile->findNext(Tokens::$emptyTokens, ($next + 1), null, true); // Skip for invalid code or other method. - if ($next === false || $tokens[$next]['content'] !== $methodName) { + if (strcasecmp($tokens[$next]['content'], $methodName) !== 0) { return; } @@ -109,14 +128,13 @@ public function process(File $phpcsFile, $stackPtr) $next = $phpcsFile->findNext(Tokens::$emptyTokens, ($next + 1), null, true); // Skip for invalid code. - if ($next === false || $tokens[$next]['code'] !== T_OPEN_PARENTHESIS) { + if ($tokens[$next]['code'] !== T_OPEN_PARENTHESIS || isset($tokens[$next]['parenthesis_closer']) === false) { return; } $parameters = ['']; $parenthesisCount = 1; - $count = count($tokens); - for (++$next; $next < $count; ++$next) { + for (++$next; $next < $phpcsFile->numTokens; ++$next) { $code = $tokens[$next]['code']; if ($code === T_OPEN_PARENTHESIS) { @@ -135,7 +153,7 @@ public function process(File $phpcsFile, $stackPtr) }//end for $next = $phpcsFile->findNext(Tokens::$emptyTokens, ($next + 1), null, true); - if ($next === false || $tokens[$next]['code'] !== T_SEMICOLON) { + if ($tokens[$next]['code'] !== T_SEMICOLON) { return; } diff --git a/src/Standards/Generic/Tests/CodeAnalysis/UselessOverridingMethodUnitTest.1.inc b/src/Standards/Generic/Tests/CodeAnalysis/UselessOverridingMethodUnitTest.1.inc index 0d8ca6d172..8cdd04d326 100644 --- a/src/Standards/Generic/Tests/CodeAnalysis/UselessOverridingMethodUnitTest.1.inc +++ b/src/Standards/Generic/Tests/CodeAnalysis/UselessOverridingMethodUnitTest.1.inc @@ -64,10 +64,40 @@ class Bar { public function sameNumberDifferentParameters($a, $b) { return parent::sameNumberDifferentParameters($this->prop[$a], $this->{$b}); } + + public function differentCase() { + return parent::DIFFERENTcase(); + } + + public function differentCaseSameNonAnsiiCharáctêrs() { + // This should be flagged, only ASCII chars have changed case. + return parent::DIFFERENTcaseSameNonAnsiiCharáctêrs(); + } + + public function differentCaseDifferentNonAnsiiCharáctêrs() { + // This should not be flagged as non-ASCII chars have changed case, making this a different method name. + return parent::DIFFERENTcaseDifferentNonAnsiiCharÁctÊrs(); + } + + public function nestedFunctionShouldBailEarly() { + function nestedFunctionShouldBailEarly() { + // Invalid code needed to ensure an error is NOT triggered and the sniff bails early when handling nested function. + parent::nestedFunctionShouldBailEarly(); + } + } } abstract class AbstractFoo { abstract public function sniffShouldBailEarly(); + + public function uselessMethodInAbstractClass() { + parent::uselessMethodInAbstractClass(); + } + + public function usefulMethodInAbstractClass() { + $a = 1; + parent::usefulMethodInAbstractClass($a); + } } interface InterfaceFoo { @@ -76,4 +106,45 @@ interface InterfaceFoo { trait TraitFoo { abstract public function sniffShouldBailEarly(); + + public function usefulMethodInTrait() { + parent::usefulMethodInTrait(); + + return 1; + } + + public function uselessMethodInTrait() { + return parent::uselessMethodInTrait(); + } +} + +enum EnumFoo { + public function sniffShouldBailEarly() { + // Invalid code needed to ensure an error is NOT triggered and the sniff bails early when handling an enum method. + parent::sniffShouldBailEarly(); + } +} + +function shouldBailEarly() { + // Invalid code needed to ensure an error is NOT triggered and the sniff bails early when handling a regular function. + parent::shouldBailEarly(); +} + +$anon = new class extends ParentClass { + public function uselessOverridingMethod() { + parent::uselessOverridingMethod(); + } + + public function usefulOverridingMethod() { + $a = 10; + parent::usefulOverridingMethod($a); + } +}; + +function foo() { + $anon = new class extends ParentClass { + public function uselessOverridingMethod() { + parent::uselessOverridingMethod(); + } + }; } diff --git a/src/Standards/Generic/Tests/CodeAnalysis/UselessOverridingMethodUnitTest.6.inc b/src/Standards/Generic/Tests/CodeAnalysis/UselessOverridingMethodUnitTest.6.inc new file mode 100644 index 0000000000..186d4ae30a --- /dev/null +++ b/src/Standards/Generic/Tests/CodeAnalysis/UselessOverridingMethodUnitTest.6.inc @@ -0,0 +1,10 @@ + 1, - 16 => 1, - 38 => 1, - 56 => 1, + 4 => 1, + 16 => 1, + 38 => 1, + 56 => 1, + 68 => 1, + 72 => 1, + 93 => 1, + 116 => 1, + 134 => 1, + 146 => 1, ]; default: return [];