Skip to content
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

Add exceptions and ignorepattern properties to BooleanArgumentFlag rule #983

Merged
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
51 changes: 51 additions & 0 deletions src/main/php/PHPMD/Rule/CleanCode/BooleanArgumentFlag.php
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,13 @@

namespace PHPMD\Rule\CleanCode;

use PDepend\Source\AST\AbstractASTClassOrInterface;
use PDepend\Source\AST\ASTValue;
use PHPMD\AbstractNode;
use PHPMD\AbstractRule;
use PHPMD\Rule\FunctionAware;
use PHPMD\Rule\MethodAware;
use PHPMD\Utility\Strings;

/**
* Check for a boolean flag in the method/function signature.
Expand All @@ -30,6 +32,13 @@
*/
class BooleanArgumentFlag extends AbstractRule implements MethodAware, FunctionAware
{
/**
* Temporary cache of configured exceptions.
*
* @return array<string, int>
*/
protected $exceptions;

/**
* This method checks if a method/function has boolean flag arguments and warns about them.
*
Expand All @@ -38,6 +47,29 @@ class BooleanArgumentFlag extends AbstractRule implements MethodAware, FunctionA
*/
public function apply(AbstractNode $node)
{
$name = $node->getName();

if ($name) {
$ignorePattern = trim($this->getStringProperty('ignorepattern', ''));

if ($ignorePattern !== '' && preg_match($ignorePattern, $node->getName())) {
return;
}
}

$parent = $node->getNode()->getParent();

if ($parent &&
($parent instanceof AbstractASTClassOrInterface) &&
($name = $parent->getName())
) {
$exceptions = $this->getExceptionsList();

if (isset($exceptions[$name])) {
return;
}
}

foreach ($node->findChildrenOfType('FormalParameter') as $param) {
$declarator = $param->getFirstChildOfType('VariableDeclarator');
$value = $declarator->getValue();
Expand All @@ -54,4 +86,23 @@ protected function isBooleanValue(ASTValue $value = null)
{
return $value && $value->isValueAvailable() && ($value->getValue() === true || $value->getValue() === false);
}

/**
* Gets exceptions from property
*
* @return array<string, int>
*/
protected function getExceptionsList()
{
if ($this->exceptions === null) {
$this->exceptions = array_flip(
Strings::splitToList(
$this->getStringProperty('exceptions', ''),
','
)
);
}

return $this->exceptions;
}
}
7 changes: 5 additions & 2 deletions src/main/resources/rulesets/cleancode.xml
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,10 @@ or method.
]]>
</description>
<priority>1</priority>
<properties />
<properties>
<property name="exceptions" description="Comma-separated class name list of exceptions" value=""/>
<property name="ignorepattern" description="Ignore methods matching this regex. Example: /^(__construct|get.*Filtered)$/" value=""/>
</properties>
<example>
<![CDATA[
class Foo {
Expand Down Expand Up @@ -83,7 +86,7 @@ operand could result in zero, null or an empty string and the like.
]]>
</description>
<priority>1</priority>
<properties></properties>
<properties/>
<example>
<![CDATA[
class Foo
Expand Down
8 changes: 8 additions & 0 deletions src/site/rst/rules/cleancode.rst
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,14 @@ Example: ::
}
}

+-----------------------------------+---------------+------------------------------------------------------------------------------+
| Name | Default Value | Description |
+===================================+===============+==============================================================================+
| exceptions | | Comma-separated class name list of exceptions |
+-----------------------------------+---------------+------------------------------------------------------------------------------+
| ignorepattern | | Ignore methods matching this regex. Example: /^(__construct|get.*Filtered)$/ |
+-----------------------------------+---------------+------------------------------------------------------------------------------+

ElseExpression
==============

Expand Down
65 changes: 65 additions & 0 deletions src/test/php/PHPMD/Rule/CleanCode/BooleanArgumentFlagTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
<?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/
*/

namespace PHPMD\Rule\CleanCode;

use PHPMD\AbstractTest;

/**
* @coversDefaultClass \PHPMD\Rule\CleanCode\BooleanArgumentFlag
*/
class BooleanArgumentFlagTest extends AbstractTest
{
/**
* Get the rule under test.
*
* @return BooleanArgumentFlag
*/
public function getRule()
{
$rule = new BooleanArgumentFlag();

$rule->addProperty('exceptions', 'MyClass');
$rule->addProperty('ignorepattern', '/WhitelistedMethod$/');

return $rule;
}

/**
* Tests the rule for cases where it should apply.
*
* @param string $file The test file to test against.
* @return void
* @dataProvider getApplyingCases
*/
public function testRuleAppliesTo($file)
{
$this->expectRuleHasViolationsForFile($this->getRule(), static::ONE_VIOLATION, $file);
}

/**
* Tests the rule for cases where it should not apply.
*
* @param string $file The test file to test against.
* @return void
* @dataProvider getNotApplyingCases
*/
public function testRuleDoesNotApplyTo($file)
{
$this->expectRuleHasViolationsForFile($this->getRule(), static::NO_VIOLATION, $file);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
<?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 Foo
{
public function testRuleAppliesToTypedParameter(bool $stuff = true): void
{
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
<?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 Foo
{
public function testRuleAppliesToUntypedParameter($stuff = false)
{
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
<?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 Foo
{
public function testRuleDoesNotApplyToTypedParameterWithoutDefaultValue(bool $stuff): void
{
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
<?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 Foo
{
public function testRuleDoesNotApplyToUntypedParameterWithoutDefaultValue($stuff)
{
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
<?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 MyClass
{
public function testRuleDoesNotApplyToWhitelistedClass(bool $stuff = false): void
{
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
<?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 Foo
{
public function testRuleDoesNotApplyToWhitelistedMethod(bool $stuff = false): void
{
}
}