Skip to content

Commit

Permalink
Merge pull request phpmd#1068 from phpmd/feature/unused-private-method
Browse files Browse the repository at this point in the history
Consider [$this, 'foo'] as a usage of foo() method of the current class
  • Loading branch information
kylekatarnls committed Feb 17, 2024
2 parents 7a047e7 + 4fcbb31 commit 3b3961c
Show file tree
Hide file tree
Showing 4 changed files with 117 additions and 4 deletions.
71 changes: 67 additions & 4 deletions src/main/php/PHPMD/Rule/UnusedPrivateMethod.php
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ public function apply(AbstractNode $class)
* as private and are not used in the same class' context.
*
* @param ClassNode $class
* @return ASTMethodPostfix[]
* @return array<string, MethodNode>
*/
protected function collectUnusedPrivateMethods(ClassNode $class)
{
Expand Down Expand Up @@ -102,13 +102,27 @@ protected function acceptMethod(ClassNode $class, MethodNode $method)
* This method removes all used methods from the given methods array.
*
* @param ClassNode $class
* @param MethodNode[] $methods
* @return ASTMethodPostfix[]
* @param array<string, MethodNode> $methods
* @return array<string, MethodNode>
*/
protected function removeUsedMethods(ClassNode $class, array $methods)
{
$methods = $this->removeExplicitCalls($class, $methods);
$methods = $this->removeCallableArrayRepresentations($class, $methods);

return $methods;
}

/**
* $this->privateMethod() makes "privateMethod" marked as used as an explicit call.
*
* @param ClassNode $class
* @param array<string, MethodNode> $methods
* @return array<string, MethodNode>
*/
protected function removeExplicitCalls(ClassNode $class, array $methods)
{
foreach ($class->findChildrenOfType('MethodPostfix') as $postfix) {
/** @var $postfix ASTNode */
if ($this->isClassScope($class, $postfix)) {
unset($methods[strtolower($postfix->getImage())]);
}
Expand All @@ -117,6 +131,55 @@ protected function removeUsedMethods(ClassNode $class, array $methods)
return $methods;
}

/**
* [$this 'privateMethod'] makes "privateMethod" marked as used as very likely to be used as a callable value.
*
* @param ClassNode $class
* @param array<string, MethodNode> $methods
* @return array<string, MethodNode>
*/
protected function removeCallableArrayRepresentations(ClassNode $class, array $methods)
{
foreach ($class->findChildrenOfType('Variable') as $variable) {
if ($this->isClassScope($class, $variable) && $variable->getImage() === '$this') {
$method = $this->getMethodNameFromArraySecondElement($variable->getParent());

if ($method) {
unset($methods[strtolower($method)]);
}
}
}

return $methods;
}

/**
* Return represented method name if the given element is a 2-items array
* and that the second one is a literal static string.
*
* @param ASTNode|null $parent
* @return string|null
*/
protected function getMethodNameFromArraySecondElement($parent)
{
if ($parent instanceof ASTNode && $parent->isInstanceOf('ArrayElement')) {
$array = $parent->getParent();

if ($array instanceof ASTNode
&& $array->isInstanceOf('Array')
&& count($array->getChildren()) === 2
) {
$secondElement = $array->getChild(1)->getChild(0);

if ($secondElement->isInstanceOf('Literal')) {
return substr($secondElement->getImage(), 1, -1);
}
}
}

return null;
}

/**
* This method checks that the given method postfix is accessed on an
* instance or static reference to the given class.
Expand Down
12 changes: 12 additions & 0 deletions src/test/php/PHPMD/Rule/UnusedPrivateMethodTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,18 @@ public function testRuleAppliesWhenMethodWithSimilarNameIsInInvocationChain()
$rule->apply($this->getClass());
}

/**
* testRuleDoesNotApplyToMethodUsedViaCallable
*
* @return void
*/
public function testRuleDoesNotApplyToMethodUsedViaCallable()
{
$rule = new UnusedPrivateMethod();
$rule->setReport($this->getReportWithNoViolation());
$rule->apply($this->getClass());
}

/**
* testRuleDoesNotApplyToPrivateConstructor
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,4 +21,13 @@ private function foo()
{

}

public function allOfThoseWontMakeItUsed()
{
foo();
$other->foo();
array_map([$other, 'foo'], [1]);
array_map([$this, 'foo' . 'bar'], [1]);
array_map(array($this, 'foo' . 'bar'), [1]);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
<?php
/**
* This file is part of PHP Mess Detector.
*
* Copyright (c) Manuel Pichler <mapi@phpmd.org>.
* All rights reserved.
*
* Licensed under BSD License
* For full copyright and license information, please see the LICENSE file.
* Redistributions of files must retain the above copyright notice.
*
* @author Manuel Pichler <mapi@phpmd.org>
* @copyright Manuel Pichler. All rights reserved.
* @license https://opensource.org/licenses/bsd-license.php BSD License
* @link http://phpmd.org/
*/

class testRuleDoesNotApplyToMethodUsedViaCallable
{
private function foo()
{

}

public function allOfThoseWontMakeItUsed()
{
array_map(array($this, 'foo'), array(1));
}
}

0 comments on commit 3b3961c

Please sign in to comment.