Skip to content

Commit

Permalink
Generic/UselessOverridingMethod: bail if not checking a class method
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
rodrigoprimo authored and jrfnl committed Mar 4, 2024
1 parent f38ea41 commit 7071004
Show file tree
Hide file tree
Showing 3 changed files with 86 additions and 6 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -60,6 +71,14 @@ public function process(File $phpcsFile, $stackPtr)
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;
}

// Get function name.
$methodName = $phpcsFile->getDeclarationName($stackPtr);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,10 +78,26 @@ class Bar {
// 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 {
Expand All @@ -90,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();
}
};
}
Original file line number Diff line number Diff line change
Expand Up @@ -50,12 +50,16 @@ public function getWarningList($testFile='')
switch ($testFile) {
case 'UselessOverridingMethodUnitTest.1.inc':
return [
4 => 1,
16 => 1,
38 => 1,
56 => 1,
68 => 1,
72 => 1,
4 => 1,
16 => 1,
38 => 1,
56 => 1,
68 => 1,
72 => 1,
93 => 1,
116 => 1,
134 => 1,
146 => 1,
];
default:
return [];
Expand Down

0 comments on commit 7071004

Please sign in to comment.