Skip to content

Commit 7071004

Browse files
rodrigoprimojrfnl
authored andcommitted
Generic/UselessOverridingMethod: bail if not checking a class method
The UselessOverridingMethod is triggered when the T_FUNCTION token is found. This token is used for regular functions and also methods. The sniff should run only for class and trait methods (including abstract and anonymous classes). Those are the only methods that can have a parent. So this commit changes the sniff to bail early when dealing with a regular functions, nested functions, interfaces methods and enum methods.
1 parent f38ea41 commit 7071004

File tree

3 files changed

+86
-6
lines changed

3 files changed

+86
-6
lines changed

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

Lines changed: 19 additions & 0 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.
@@ -60,6 +71,14 @@ public function process(File $phpcsFile, $stackPtr)
6071
return;
6172
}
6273

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) {
79+
return;
80+
}
81+
6382
// Get function name.
6483
$methodName = $phpcsFile->getDeclarationName($stackPtr);
6584

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

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,10 +78,26 @@ class Bar {
7878
// This should not be flagged as non-ASCII chars have changed case, making this a different method name.
7979
return parent::DIFFERENTcaseDifferentNonAnsiiCharÁctÊrs();
8080
}
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+
}
8188
}
8289

8390
abstract class AbstractFoo {
8491
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+
}
85101
}
86102

87103
interface InterfaceFoo {
@@ -90,4 +106,45 @@ interface InterfaceFoo {
90106

91107
trait TraitFoo {
92108
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+
};
93150
}

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

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -50,12 +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,
57-
68 => 1,
58-
72 => 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,
5963
];
6064
default:
6165
return [];

0 commit comments

Comments
 (0)