Skip to content

Ignore unused parameters when function in abstract class is empty #270

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Aug 13, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Tests/VariableAnalysisSniff/VariableAnalysisTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -209,6 +209,7 @@ public function testClassWithMembersWarnings()
66,
115,
116,
174,
];
$this->assertSame($expectedWarnings, $lines);
}
Expand Down
21 changes: 18 additions & 3 deletions Tests/VariableAnalysisSniff/fixtures/ClassWithMembersFixture.php
Original file line number Diff line number Diff line change
Expand Up @@ -154,9 +154,24 @@ abstract class AbstractClassWithStaticProperties {
public static int $static_with_visibility_and_type;
public static ?int $static_with_visibility_and_nullable_type;

public function use_vars();
abstract public function use_vars();

public static function getIntOrNull($value);
abstract public static function getIntOrNull($value);

static function getIntOrNull2($value);
abstract static function getIntOrNull2($value);
}

abstract class AbstractClassWithEmptyMethodBodies {
abstract public function bar($param);

public function baz($param) {
}

public function baz2($param) {
return;
}

public function baz3($param) { // Unused variable $param
return 'foobar';
}
}
61 changes: 57 additions & 4 deletions VariableAnalysis/Lib/Helpers.php
Original file line number Diff line number Diff line change
Expand Up @@ -91,14 +91,15 @@ public static function areAnyConditionsAClass(array $conditions)
}

/**
* Return true if the token conditions are within a function before they are
* within a class.
*
* @param (int|string)[] $conditions
*
* @return bool
*/
public static function areConditionsWithinFunctionBeforeClass(array $conditions)
{
// Return true if the token conditions are within a function before
// they are within a class.
$classTypes = [T_CLASS, T_ANON_CLASS, T_TRAIT];
foreach (array_reverse($conditions, true) as $scopeCode) {
if (in_array($scopeCode, $classTypes)) {
Expand All @@ -112,14 +113,15 @@ public static function areConditionsWithinFunctionBeforeClass(array $conditions)
}

/**
* Return true if the token conditions are within an if block before they are
* within a class or function.
*
* @param (int|string)[] $conditions
*
* @return int|string|null
*/
public static function getClosestIfPositionIfBeforeOtherConditions(array $conditions)
{
// Return true if the token conditions are within an if block before
// they are within a class or function.
$conditionsInsideOut = array_reverse($conditions, true);
if (empty($conditions)) {
return null;
Expand Down Expand Up @@ -1396,4 +1398,55 @@ public static function isConstructorPromotion(File $phpcsFile, $stackPtr)

return false;
}

/**
* Return true if the token is inside an abstract class.
*
* @param File $phpcsFile
* @param int $stackPtr
*
* @return bool
*/
public static function isInAbstractClass(File $phpcsFile, $stackPtr)
{
$classIndex = $phpcsFile->getCondition($stackPtr, T_CLASS);
if (! is_int($classIndex)) {
return false;
}
$classProperties = $phpcsFile->getClassProperties($classIndex);
return $classProperties['is_abstract'];
}

/**
* Return true if the function body is empty or contains only `return;`
*
* @param File $phpcsFile
* @param int $stackPtr The index of the function keyword.
*
* @return bool
*/
public static function isFunctionBodyEmpty(File $phpcsFile, $stackPtr)
{
$tokens = $phpcsFile->getTokens();
if ($tokens[$stackPtr]['code'] !== T_FUNCTION) {
return false;
}
$functionScopeStart = $tokens[$stackPtr]['scope_opener'];
$functionScopeEnd = $tokens[$stackPtr]['scope_closer'];
$tokensToIgnore = array_merge(
Tokens::$emptyTokens,
[
T_RETURN,
T_SEMICOLON,
T_OPEN_CURLY_BRACKET,
T_CLOSE_CURLY_BRACKET,
]
);
for ($i = $functionScopeStart; $i < $functionScopeEnd; $i++) {
if (! in_array($tokens[$i]['code'], $tokensToIgnore, true)) {
return false;
}
}
return true;
}
}
18 changes: 18 additions & 0 deletions VariableAnalysis/Sniffs/CodeAnalysis/VariableAnalysisSniff.php
Original file line number Diff line number Diff line change
Expand Up @@ -1949,6 +1949,24 @@ protected function processScopeCloseForVariable(File $phpcsFile, VariableInfo $v
if ($scopeInfo->scopeStartIndex === 0 && $this->allowUnusedVariablesInFileScope) {
return;
}
if (
! empty($varInfo->firstDeclared)
&& $varInfo->scopeType === ScopeType::PARAM
&& Helpers::isInAbstractClass(
$phpcsFile,
Helpers::getFunctionIndexForFunctionParameter($phpcsFile, $varInfo->firstDeclared) ?: 0
)
&& Helpers::isFunctionBodyEmpty(
$phpcsFile,
Helpers::getFunctionIndexForFunctionParameter($phpcsFile, $varInfo->firstDeclared) ?: 0
)
) {
// Allow non-abstract methods inside an abstract class to have unused
// parameters if the method body does nothing. Such methods are
// effectively optional abstract methods so their unused parameters
// should be ignored as we do with abstract method parameters.
return;
}

$this->warnAboutUnusedVariable($phpcsFile, $varInfo);
}
Expand Down