Merge pull request #983 from kylekatarnls/fix/issue-924-bool-arg-in-c…

…onstructor Add exceptions and ignorepattern properties to BooleanArgumentFlag rule
phpmd · Sep 13, 2022 · 79c937c · 79c937c
2 parents dad0228 + bde506a
commit 79c937c
Show file tree

Hide file tree

Showing 10 changed files with 267 additions and 2 deletions.
diff --git a/src/main/php/PHPMD/Rule/CleanCode/BooleanArgumentFlag.php b/src/main/php/PHPMD/Rule/CleanCode/BooleanArgumentFlag.php
@@ -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.
@@ -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.
      *
@@ -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();
@@ -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;
+    }
 }
diff --git a/src/main/resources/rulesets/cleancode.xml b/src/main/resources/rulesets/cleancode.xml
@@ -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 {
@@ -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

diff --git a/src/site/rst/rules/cleancode.rst b/src/site/rst/rules/cleancode.rst
@@ -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
 ==============
 

diff --git a/src/test/php/PHPMD/Rule/CleanCode/BooleanArgumentFlagTest.php b/src/test/php/PHPMD/Rule/CleanCode/BooleanArgumentFlagTest.php
@@ -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);
+    }
+}
diff --git a/...st/resources/files/Rule/CleanCode/BooleanArgumentFlag/testRuleAppliesToTypedParameter.php b/...st/resources/files/Rule/CleanCode/BooleanArgumentFlag/testRuleAppliesToTypedParameter.php
@@ -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
+    {
+    }
+}
diff --git a/.../resources/files/Rule/CleanCode/BooleanArgumentFlag/testRuleAppliesToUntypedParameter.php b/.../resources/files/Rule/CleanCode/BooleanArgumentFlag/testRuleAppliesToUntypedParameter.php
@@ -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)
+    {
+    }
+}
diff --git a/...CleanCode/BooleanArgumentFlag/testRuleDoesNotApplyToTypedParameterWithoutDefaultValue.php b/...CleanCode/BooleanArgumentFlag/testRuleDoesNotApplyToTypedParameterWithoutDefaultValue.php
@@ -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
+    {
+    }
+}
diff --git a/...eanCode/BooleanArgumentFlag/testRuleDoesNotApplyToUntypedParameterWithoutDefaultValue.php b/...eanCode/BooleanArgumentFlag/testRuleDoesNotApplyToUntypedParameterWithoutDefaultValue.php
@@ -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)
+    {
+    }
+}
diff --git a/...urces/files/Rule/CleanCode/BooleanArgumentFlag/testRuleDoesNotApplyToWhitelistedClass.php b/...urces/files/Rule/CleanCode/BooleanArgumentFlag/testRuleDoesNotApplyToWhitelistedClass.php
@@ -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
+    {
+    }
+}
diff --git a/...rces/files/Rule/CleanCode/BooleanArgumentFlag/testRuleDoesNotApplyToWhitelistedMethod.php b/...rces/files/Rule/CleanCode/BooleanArgumentFlag/testRuleDoesNotApplyToWhitelistedMethod.php
@@ -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
+    {
+    }
+}