Skip to content

Commit 0f90d83

Browse files
jrfnljrfnl
authored
Merge pull request #366 from rodrigoprimo/useless-overriding-method-improvements
Generic/UselessOverridingMethod: small improvements to the sniff code
2 parents 776bc88 + 7071004 commit 0f90d83

File tree

4 files changed

+117
-12
lines changed

4 files changed

+117
-12
lines changed

‎src/Standards/Generic/Sniffs/CodeAnalysis/UselessOverridingMethodSniff.php

Lines changed: 26 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,17 @@
2828
class UselessOverridingMethodSniff implements Sniff
2929
{
3030

31+
/**
32+
* Object-Oriented scopes in which a call to parent::method() can exist.
33+
*
34+
* @var array<int|string, bool> Keys are the token constants, value is irrelevant.
35+
*/
36+
private $validOOScopes = [
37+
T_CLASS => true,
38+
T_ANON_CLASS => true,
39+
T_TRAIT => true,
40+
];
41+
3142

3243
/**
3344
* Registers the tokens that this sniff wants to listen for.
@@ -56,7 +67,15 @@ public function process(File $phpcsFile, $stackPtr)
5667
$token = $tokens[$stackPtr];
5768

5869
// Skip function without body.
59-
if (isset($token['scope_opener']) === false) {
70+
if (isset($token['scope_opener'], $token['scope_closer']) === false) {
71+
return;
72+
}
73+
74+
$conditions = $token['conditions'];
75+
$lastCondition = end($conditions);
76+
77+
// Skip functions that are not a method part of a class, anon class or trait.
78+
if (isset($this->validOOScopes[$lastCondition]) === false) {
6079
return;
6180
}
6281

@@ -93,30 +112,29 @@ public function process(File $phpcsFile, $stackPtr)
93112
$next = $phpcsFile->findNext(Tokens::$emptyTokens, ($next + 1), null, true);
94113

95114
// Skip for invalid code.
96-
if ($next === false || $tokens[$next]['code'] !== T_DOUBLE_COLON) {
115+
if ($tokens[$next]['code'] !== T_DOUBLE_COLON) {
97116
return;
98117
}
99118

100-
// Find next non empty token index, should be the function name.
119+
// Find next non empty token index, should be the name of the method being called.
101120
$next = $phpcsFile->findNext(Tokens::$emptyTokens, ($next + 1), null, true);
102121

103122
// Skip for invalid code or other method.
104-
if ($next === false || $tokens[$next]['content'] !== $methodName) {
123+
if (strcasecmp($tokens[$next]['content'], $methodName) !== 0) {
105124
return;
106125
}
107126

108127
// Find next non empty token index, should be the open parenthesis.
109128
$next = $phpcsFile->findNext(Tokens::$emptyTokens, ($next + 1), null, true);
110129

111130
// Skip for invalid code.
112-
if ($next === false || $tokens[$next]['code'] !== T_OPEN_PARENTHESIS) {
131+
if ($tokens[$next]['code'] !== T_OPEN_PARENTHESIS || isset($tokens[$next]['parenthesis_closer']) === false) {
113132
return;
114133
}
115134

116135
$parameters = [''];
117136
$parenthesisCount = 1;
118-
$count = count($tokens);
119-
for (++$next; $next < $count; ++$next) {
137+
for (++$next; $next < $phpcsFile->numTokens; ++$next) {
120138
$code = $tokens[$next]['code'];
121139

122140
if ($code === T_OPEN_PARENTHESIS) {
@@ -135,7 +153,7 @@ public function process(File $phpcsFile, $stackPtr)
135153
}//end for
136154

137155
$next = $phpcsFile->findNext(Tokens::$emptyTokens, ($next + 1), null, true);
138-
if ($next === false || $tokens[$next]['code'] !== T_SEMICOLON) {
156+
if ($tokens[$next]['code'] !== T_SEMICOLON) {
139157
return;
140158
}
141159

‎src/Standards/Generic/Tests/CodeAnalysis/UselessOverridingMethodUnitTest.1.inc

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,10 +64,40 @@ class Bar {
6464
public function sameNumberDifferentParameters($a, $b) {
6565
return parent::sameNumberDifferentParameters($this->prop[$a], $this->{$b});
6666
}
67+
68+
public function differentCase() {
69+
return parent::DIFFERENTcase();
70+
}
71+
72+
public function differentCaseSameNonAnsiiCharáctêrs() {
73+
// This should be flagged, only ASCII chars have changed case.
74+
return parent::DIFFERENTcaseSameNonAnsiiCharáctêrs();
75+
}
76+
77+
public function differentCaseDifferentNonAnsiiCharáctêrs() {
78+
// This should not be flagged as non-ASCII chars have changed case, making this a different method name.
79+
return parent::DIFFERENTcaseDifferentNonAnsiiCharÁctÊrs();
80+
}
81+
82+
public function nestedFunctionShouldBailEarly() {
83+
function nestedFunctionShouldBailEarly() {
84+
// Invalid code needed to ensure an error is NOT triggered and the sniff bails early when handling nested function.
85+
parent::nestedFunctionShouldBailEarly();
86+
}
87+
}
6788
}
6889

6990
abstract class AbstractFoo {
7091
abstract public function sniffShouldBailEarly();
92+
93+
public function uselessMethodInAbstractClass() {
94+
parent::uselessMethodInAbstractClass();
95+
}
96+
97+
public function usefulMethodInAbstractClass() {
98+
$a = 1;
99+
parent::usefulMethodInAbstractClass($a);
100+
}
71101
}
72102

73103
interface InterfaceFoo {
@@ -76,4 +106,45 @@ interface InterfaceFoo {
76106

77107
trait TraitFoo {
78108
abstract public function sniffShouldBailEarly();
109+
110+
public function usefulMethodInTrait() {
111+
parent::usefulMethodInTrait();
112+
113+
return 1;
114+
}
115+
116+
public function uselessMethodInTrait() {
117+
return parent::uselessMethodInTrait();
118+
}
119+
}
120+
121+
enum EnumFoo {
122+
public function sniffShouldBailEarly() {
123+
// Invalid code needed to ensure an error is NOT triggered and the sniff bails early when handling an enum method.
124+
parent::sniffShouldBailEarly();
125+
}
126+
}
127+
128+
function shouldBailEarly() {
129+
// Invalid code needed to ensure an error is NOT triggered and the sniff bails early when handling a regular function.
130+
parent::shouldBailEarly();
131+
}
132+
133+
$anon = new class extends ParentClass {
134+
public function uselessOverridingMethod() {
135+
parent::uselessOverridingMethod();
136+
}
137+
138+
public function usefulOverridingMethod() {
139+
$a = 10;
140+
parent::usefulOverridingMethod($a);
141+
}
142+
};
143+
144+
function foo() {
145+
$anon = new class extends ParentClass {
146+
public function uselessOverridingMethod() {
147+
parent::uselessOverridingMethod();
148+
}
149+
};
79150
}

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

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
<?php
2+
3+
// Intentional parse error (missing closing parenthesis in parent method call).
4+
// Testing that the sniff is *not* triggered in this case.
5+
6+
class FooBar {
7+
public function __construct() {
8+
parent::__construct(
9+
}
10+
}

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

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -50,10 +50,16 @@ public function getWarningList($testFile='')
5050
switch ($testFile) {
5151
case 'UselessOverridingMethodUnitTest.1.inc':
5252
return [
53-
4 => 1,
54-
16 => 1,
55-
38 => 1,
56-
56 => 1,
53+
4 => 1,
54+
16 => 1,
55+
38 => 1,
56+
56 => 1,
57+
68 => 1,
58+
72 => 1,
59+
93 => 1,
60+
116 => 1,
61+
134 => 1,
62+
146 => 1,
5763
];
5864
default:
5965
return [];

0 commit comments

Comments
 (0)