Generic/UselessOverridingMethod: small improvements to the sniff code

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<int|string, bool> 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,30 +112,29 @@ 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;
         }
 
         // Find next non empty token index, should be the open parenthesis.
         $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;
         }
 

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();
+        }
+    };
 }

src/Standards/Generic/Tests/CodeAnalysis/UselessOverridingMethodUnitTest.6.inc

@@ -0,0 +1,10 @@
+<?php
+
+// Intentional parse error (missing closing parenthesis in parent method call).
+// Testing that the sniff is *not* triggered in this case.
+
+class FooBar {
+    public function __construct() {
+        parent::__construct(
+    }
+}

src/Standards/Generic/Tests/CodeAnalysis/UselessOverridingMethodUnitTest.php

@@ -50,10 +50,16 @@ public function getWarningList($testFile='')
         switch ($testFile) {
         case 'UselessOverridingMethodUnitTest.1.inc':
             return [
-                4  => 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 [];