Skip to content

Commit

Permalink
Fixed #23278127: PHPMD should exclude unused parameters from inherite…
Browse files Browse the repository at this point in the history
…d methods
  • Loading branch information
manuelpichler committed Jan 24, 2012
1 parent 02b8d59 commit d162b21
Show file tree
Hide file tree
Showing 13 changed files with 307 additions and 117 deletions.
31 changes: 31 additions & 0 deletions src/main/php/PHP/PMD/Node/Method.php
Expand Up @@ -133,4 +133,35 @@ public function getParentType()
}
return new PHP_PMD_Node_Interface($parentNode);
}

/**
* Returns <b>true</b> when this method is the initial method declaration.
* Otherwise this method will return <b>false</b>.
*
* @return boolean
* @since 1.2.1
*/
public function isDeclaration()
{
if ($this->isPrivate()) {
return true;
}

$parentNode = $this->getNode()->getParent();
foreach ($parentNode->getInterfaces() as $parentType) {
$methods = $parentType->getAllMethods();
if (isset($methods[$this->getName()])) {
return false;
}
}

if (is_object($parentType = $parentNode->getParentClass())) {
$methods = $parentType->getAllMethods();
if (isset($methods[$this->getName()])) {
return false;
}
}

return true;
}
}
21 changes: 21 additions & 0 deletions src/main/php/PHP/PMD/Rule/UnusedFormalParameter.php
Expand Up @@ -89,6 +89,10 @@ public function apply(PHP_PMD_AbstractNode $node)
return;
}

if ($this->_isNotDeclaration($node)) {
return;
}

$this->_nodes = array();

$this->_collectParameters($node);
Expand All @@ -114,6 +118,23 @@ private function _isAbstractMethod(PHP_PMD_AbstractNode $node)
return false;
}

/**
* Tests if the given <b>$node</b> is a method and if this method is also
* the initial declaration.
*
* @param PHP_PMD_AbstractNode $node The context method or a function instance.
*
* @return boolean
* @since 1.2.1
*/
private function _isNotDeclaration(PHP_PMD_AbstractNode $node)
{
if ($node instanceof PHP_PMD_Node_Method) {
return !$node->isDeclaration();
}
return false;
}

/**
* This method extracts all parameters for the given function or method node
* and it stores the parameter images in the <b>$_images</b> property.
Expand Down
4 changes: 4 additions & 0 deletions src/site/docx/changes.xml
Expand Up @@ -16,6 +16,10 @@
<action date="183fbd5" dev="mapi" issue="14990109" system="pivotaltracker" due-to="tdxtik" type="fix">
False detection of unused variable
</action>

<action date="" dev="mapi" issue="23278127" system="pivotaltracker" due-to="Hugo Hamon" due-to-email="hugo.hamon@sensio.com" type="fix">
PHPMD should exclude unused parameters from inherited methods
</action>
</release>

<release version="1.2.0"
Expand Down
99 changes: 69 additions & 30 deletions src/test/php/PHP/PMD/Node/MethodTest.php
Expand Up @@ -61,17 +61,18 @@
* @license http://www.opensource.org/licenses/bsd-license.php BSD License
* @version Release: @package_version@
* @link http://phpmd.org
*
* @covers PHP_PMD_Node_Method
* @group phpmd
* @group phpmd::node
* @group unittest
*/
class PHP_PMD_Node_MethodTest extends PHP_PMD_AbstractTest
{
/**
* testMagicCallDelegatesToWrappedPHPDependMethod
*
* @return void
* @covers PHP_PMD_Node_AbstractCallable::__call
* @group phpmd
* @group phpmd::node
* @group unittest
*/
public function testMagicCallDelegatesToWrappedPHPDependMethod()
{
Expand All @@ -87,10 +88,6 @@ public function testMagicCallDelegatesToWrappedPHPDependMethod()
* testMagicCallThrowsExceptionWhenNoMatchingMethodExists
*
* @return void
* @covers PHP_PMD_Node_AbstractCallable::__call
* @group phpmd
* @group phpmd::node
* @group unittest
* @expectedException BadMethodCallException
*/
public function testMagicCallThrowsExceptionWhenNoMatchingMethodExists()
Expand All @@ -103,25 +100,19 @@ public function testMagicCallThrowsExceptionWhenNoMatchingMethodExists()
* testGetParentTypeReturnsInterfaceForInterfaceMethod
*
* @return void
* @covers PHP_PMD_Node_Method::getParentType
* @group phpmd
* @group phpmd::node
* @group unittest
*/
public function testGetParentTypeReturnsInterfaceForInterfaceMethod()
{
$method = $this->getMethod();
self::assertInstanceOf(PHP_PMD_Node_Interface::CLAZZ, $method->getParentType());
$this->assertInstanceOf(
PHP_PMD_Node_Interface::CLAZZ,
$this->getMethod()->getParentType()
);
}

/**
* testGetParentTypeReturnsClassForClassMethod
*
* @return void
* @covers PHP_PMD_Node_Method::getParentType
* @group phpmd
* @group phpmd::node
* @group unittest
*/
public function testGetParentTypeReturnsClassForClassMethod()
{
Expand All @@ -133,10 +124,6 @@ public function testGetParentTypeReturnsClassForClassMethod()
* testHasSuppressWarningsExecutesDefaultImplementation
*
* @return void
* @covers PHP_PMD_Node_Method::hasSuppressWarningsAnnotationFor
* @group phpmd
* @group phpmd::node
* @group unittest
*/
public function testHasSuppressWarningsExecutesDefaultImplementation()
{
Expand All @@ -151,10 +138,6 @@ public function testHasSuppressWarningsExecutesDefaultImplementation()
* testHasSuppressWarningsDelegatesToParentClassMethod
*
* @return void
* @covers PHP_PMD_Node_Method::hasSuppressWarningsAnnotationFor
* @group phpmd
* @group phpmd::node
* @group unittest
*/
public function testHasSuppressWarningsDelegatesToParentClassMethod()
{
Expand All @@ -169,10 +152,6 @@ public function testHasSuppressWarningsDelegatesToParentClassMethod()
* testHasSuppressWarningsDelegatesToParentInterfaceMethod
*
* @return void
* @covers PHP_PMD_Node_Method::hasSuppressWarningsAnnotationFor
* @group phpmd
* @group phpmd::node
* @group unittest
*/
public function testHasSuppressWarningsDelegatesToParentInterfaceMethod()
{
Expand All @@ -182,4 +161,64 @@ public function testHasSuppressWarningsDelegatesToParentInterfaceMethod()
$method = $this->getMethod();
$this->assertTrue($method->hasSuppressWarningsAnnotationFor($rule));
}

/**
* testIsDeclarationReturnsTrueForMethodDeclaration
*
* @return void
* @since 1.2.1
*/
public function testIsDeclarationReturnsTrueForMethodDeclaration()
{
$method = $this->getMethod();
$this->assertTrue($method->isDeclaration());
}

/**
* testIsDeclarationReturnsTrueForMethodDeclarationWithParent
*
* @return void
* @since 1.2.1
*/
public function testIsDeclarationReturnsTrueForMethodDeclarationWithParent()
{
$method = $this->getMethod();
$this->assertTrue($method->isDeclaration());
}

/**
* testIsDeclarationReturnsFalseForInheritMethodDeclaration
*
* @return void
* @since 1.2.1
*/
public function testIsDeclarationReturnsFalseForInheritMethodDeclaration()
{
$method = $this->getMethod();
$this->assertFalse($method->isDeclaration());
}

/**
* testIsDeclarationReturnsFalseForImplementedAbstractMethod
*
* @return void
* @since 1.2.1
*/
public function testIsDeclarationReturnsFalseForImplementedAbstractMethod()
{
$method = $this->getMethod();
$this->assertFalse($method->isDeclaration());
}

/**
* testIsDeclarationReturnsFalseForImplementedInterfaceMethod
*
* @return void
* @since 1.2.1
*/
public function testIsDeclarationReturnsFalseForImplementedInterfaceMethod()
{
$method = $this->getMethod();
$this->assertFalse($method->isDeclaration());
}
}

0 comments on commit d162b21

Please sign in to comment.