Merge pull request #31 from 123inkt/Add_option_to_opt_out_of_parent_m…

…ethod_checks Added option to exclude class methods or all parent methods
123inkt · Jul 18, 2021 · 7ebae9a · 7ebae9a
2 parents d042975 + bc4583e
commit 7ebae9a
Show file tree

Hide file tree

Showing 63 changed files with 827 additions and 115 deletions.
diff --git a/src/AccessorPairAsserter.php b/src/AccessorPairAsserter.php
@@ -9,6 +9,19 @@
 
 trait AccessorPairAsserter
 {
+    /**
+     * Assert the constructor parameter - method pairs. Will use default configuration
+     * with ConstraintConfig::setAssertPropertyDefaults(true)
+     *
+     * @param string $object  The fully qualified name of the class that should be tested
+     * @param string $message Custom PHPUnit error message in case of constraint failure
+     */
+    public static function assertAccessorPropertyDefaults(string $object, string $message = ''): void
+    {
+        $config = (new ConstraintConfig())->setAssertPropertyDefaults(true);
+        Assert::assertThat($object, new AccessorPairConstraint($config), $message);
+    }
+
     /**
      * @param string                $object  The fully qualified name of the class that should be tested
      * @param ConstraintConfig|null $config  Configuration of the constraint.
@@ -23,4 +36,17 @@ public static function assertAccessorPairs(string $object, ConstraintConfig $con
 
         Assert::assertThat($object, new AccessorPairConstraint($config), $message);
     }
+
+    /**
+     * Only assert accessor pairs from the class itself and none of it's parents. Will use default configuration
+     * with ConstraintConfig::setAssertParentMethods(false)
+     *
+     * @param string $object  The fully qualified name of the class that should be tested
+     * @param string $message Custom PHPUnit error message in case of constraint failure
+     */
+    public static function assertOwnAccessorPairs(string $object, string $message = ''): void
+    {
+        $config = (new ConstraintConfig())->setAssertParentMethods(false);
+        Assert::assertThat($object, new AccessorPairConstraint($config), $message);
+    }
 }
diff --git a/src/Constraint/AccessorPairConstraint.php b/src/Constraint/AccessorPairConstraint.php
@@ -40,8 +40,8 @@ class AccessorPairConstraint extends Constraint
 
     public function __construct(ConstraintConfig $config)
     {
-        $this->accessorPairProvider    = new AccessorPairProvider();
-        $this->constructorPairProvider = new ConstructorPairProvider();
+        $this->accessorPairProvider    = new AccessorPairProvider($config);
+        $this->constructorPairProvider = new ConstructorPairProvider($config);
         $this->valueProviderFactory    = new ValueProviderFactory();
         $this->config                  = $config;
 

diff --git a/src/Constraint/ConstraintConfig.php b/src/Constraint/ConstraintConfig.php
@@ -14,6 +14,12 @@ class ConstraintConfig
     /** @var bool */
     private $assertPropertyDefaults = false;
 
+    /** @var bool */
+    private $assertParentMethods = true;
+
+    /** @var string[] */
+    private $excludedMethods = [];
+
     /** @var null|callable(): mixed[] */
     private $constructorCallback = null;
 
@@ -69,6 +75,41 @@ public function setAssertPropertyDefaults(bool $assertPropertyDefaults): self
         return $this;
     }
 
+    public function isAssertParentMethods(): bool
+    {
+        return $this->assertParentMethods;
+    }
+
+    /**
+     * Enabled by default.
+     * When disabled, only the direct class methods will be asserted and none of the parent's
+     * class methods.
+     */
+    public function setAssertParentMethods(bool $assertParentMethods): self
+    {
+        $this->assertParentMethods = $assertParentMethods;
+
+        return $this;
+    }
+
+    /**
+     * @return string[]
+     */
+    public function getExcludedMethods(): array
+    {
+        return $this->excludedMethods;
+    }
+
+    /**
+     * @param string[] $excludedMethods A list of exact method names that should be excluded from the assertions.
+     */
+    public function setExcludedMethods(array $excludedMethods): self
+    {
+        $this->excludedMethods = $excludedMethods;
+
+        return $this;
+    }
+
     public function getConstructorCallback(): ?callable
     {
         return $this->constructorCallback;

diff --git a/src/Constraint/MethodPair/AccessorPair/AccessorPairProvider.php b/src/Constraint/MethodPair/AccessorPair/AccessorPairProvider.php
@@ -3,13 +3,14 @@
 
 namespace DigitalRevolution\AccessorPairConstraint\Constraint\MethodPair\AccessorPair;
 
+use DigitalRevolution\AccessorPairConstraint\Constraint\ConstraintConfig;
+use DigitalRevolution\AccessorPairConstraint\Constraint\MethodPair\ClassMethodProvider;
 use DigitalRevolution\AccessorPairConstraint\Constraint\Typehint\TypehintResolver;
 use Doctrine\Inflector\Inflector;
 use Doctrine\Inflector\InflectorFactory;
 use LogicException;
 use phpDocumentor\Reflection\Types\Array_;
 use ReflectionClass;
-use ReflectionException;
 use ReflectionMethod;
 
 class AccessorPairProvider
@@ -20,8 +21,12 @@ class AccessorPairProvider
     /** @var Inflector */
     private $inflector;
 
-    public function __construct()
+    /** @var ConstraintConfig */
+    private $config;
+
+    public function __construct(ConstraintConfig $config)
     {
+        $this->config    = $config;
         $this->inflector = InflectorFactory::create()->build();
     }
 
@@ -30,13 +35,12 @@ public function __construct()
      * Loops over the public methods, and for each "getter" it tries to find the corresponding "set" and/or "add" method
      *
      * @return AccessorPair[]
-     * @throws ReflectionException
      * @throws LogicException
      */
     public function getAccessorPairs(ReflectionClass $class): array
     {
         $pairs = [];
-        foreach ($class->getMethods(ReflectionMethod::IS_PUBLIC) as $method) {
+        foreach ((new ClassMethodProvider($this->config))->getMethods($class) as $method) {
             // Check multiple "getter" prefixes, add each getter method with corresponding setter to the inspectionMethod list
             $methodName = $method->getName();
             foreach (static::GET_PREFIXES as $getterPrefix) {
@@ -47,17 +51,7 @@ public function getAccessorPairs(ReflectionClass $class): array
                 // Try and find the corresponding set/add method
                 $baseMethodNames = $this->getMethodBaseNames($methodName, $getterPrefix);
                 foreach ($baseMethodNames as $baseMethodName) {
-                    foreach (static::SET_PREFIXES as $setterPrefix) {
-                        $setterName = $setterPrefix . $baseMethodName;
-                        if ($class->hasMethod($setterName) === false) {
-                            continue;
-                        }
-
-                        $setterMethod = $class->getMethod($setterName);
-                        if ($setterMethod->isPublic() === false) {
-                            continue;
-                        }
-
+                    foreach ($this->getSetters($class, $baseMethodName) as $setterMethod) {
                         $accessorPair = new AccessorPair($class, $method, $setterMethod);
                         if ($this->validateAccessorPair($accessorPair)) {
                             $pairs[] = $accessorPair;
@@ -85,6 +79,37 @@ protected function getMethodBaseNames(string $methodName, string $getterPrefix):
         return $baseMethodNames;
     }
 
+    /**
+     * @return ReflectionMethod[]
+     */
+    protected function getSetters(ReflectionClass $class, string $baseMethodName): array
+    {
+        $setters = [];
+        foreach (static::SET_PREFIXES as $setterPrefix) {
+            $setterName = $setterPrefix . $baseMethodName;
+            if ($class->hasMethod($setterName) === false) {
+                continue;
+            }
+
+            $setterMethod = $class->getMethod($setterName);
+            if ($setterMethod->isPublic() === false) {
+                continue;
+            }
+
+            if (in_array($setterMethod->getName(), $this->config->getExcludedMethods(), true)) {
+                continue;
+            }
+
+            if ($this->config->isAssertParentMethods() === false && $class->getName() !== $setterMethod->getDeclaringClass()->getName()) {
+                continue;
+            }
+
+            $setters[] = $setterMethod;
+        }
+
+        return $setters;
+    }
+
     /**
      * @throws LogicException
      */

diff --git a/src/Constraint/MethodPair/ClassMethodProvider.php b/src/Constraint/MethodPair/ClassMethodProvider.php
@@ -0,0 +1,45 @@
+<?php
+declare(strict_types=1);
+
+namespace DigitalRevolution\AccessorPairConstraint\Constraint\MethodPair;
+
+use DigitalRevolution\AccessorPairConstraint\Constraint\ConstraintConfig;
+use ReflectionClass;
+use ReflectionMethod;
+
+class ClassMethodProvider
+{
+    /** @var ConstraintConfig */
+    private $config;
+
+    public function __construct(ConstraintConfig $config)
+    {
+        $this->config = $config;
+    }
+
+    /**
+     * @return ReflectionMethod[]
+     */
+    public function getMethods(ReflectionClass $class): array
+    {
+        $excludeParentMethods = $this->config->isAssertParentMethods() === false;
+        $excludedMethods      = $this->config->getExcludedMethods();
+
+        $methods = [];
+        foreach ($class->getMethods(ReflectionMethod::IS_PUBLIC) as $method) {
+            // exclude all methods that are not from the class' parent class.
+            if ($excludeParentMethods && $class->getName() !== $method->getDeclaringClass()->getName()) {
+                continue;
+            }
+
+            // method is specifically excluded
+            if (in_array($method->getName(), $excludedMethods, true)) {
+                continue;
+            }
+
+            $methods[] = $method;
+        }
+
+        return $methods;
+    }
+}
diff --git a/src/Constraint/MethodPair/ConstructorPair/ConstructorPairProvider.php b/src/Constraint/MethodPair/ConstructorPair/ConstructorPairProvider.php
@@ -3,13 +3,14 @@
 
 namespace DigitalRevolution\AccessorPairConstraint\Constraint\MethodPair\ConstructorPair;
 
+use DigitalRevolution\AccessorPairConstraint\Constraint\ConstraintConfig;
+use DigitalRevolution\AccessorPairConstraint\Constraint\MethodPair\ClassMethodProvider;
 use DigitalRevolution\AccessorPairConstraint\Constraint\Typehint\TypehintResolver;
 use Doctrine\Inflector\Inflector;
 use Doctrine\Inflector\InflectorFactory;
 use LogicException;
 use phpDocumentor\Reflection\Types\Array_;
 use ReflectionClass;
-use ReflectionMethod;
 use ReflectionParameter;
 
 class ConstructorPairProvider
@@ -19,8 +20,12 @@ class ConstructorPairProvider
     /** @var Inflector */
     private $inflector;
 
-    public function __construct()
+    /** @var ConstraintConfig */
+    private $config;
+
+    public function __construct(ConstraintConfig $config)
     {
+        $this->config    = $config;
         $this->inflector = InflectorFactory::create()->build();
     }
 
@@ -36,7 +41,7 @@ public function getConstructorPairs(ReflectionClass $class): array
         }
 
         $pairs = [];
-        foreach ($class->getMethods(ReflectionMethod::IS_PUBLIC) as $method) {
+        foreach ((new ClassMethodProvider($this->config))->getMethods($class) as $method) {
             // Check multiple "getter" prefixes, add each getter method with corresponding setter to the inspectionMethod list
             $methodName = $method->getName();
 
@@ -75,6 +80,12 @@ protected function getParameters(ReflectionClass $class): array
             return [];
         }
 
+        // skip parent constructor
+        $excludeParentMethods = $this->config->isAssertParentMethods() === false;
+        if ($excludeParentMethods && $constructor->getDeclaringClass()->getName() !== $class->getName()) {
+            return [];
+        }
+
         $parameters = [];
         foreach ($constructor->getParameters() as $parameter) {
             $parameters[strtolower($parameter->getName())] = $parameter;

diff --git a/tests/Integration/AccessorPairAsserterTest.php b/tests/Integration/AccessorPairAsserterTest.php
@@ -73,6 +73,14 @@ public function testMatchesSuccessInitialState(object $class): void
         static::assertAccessorPairs(get_class($class), (new ConstraintConfig())->setAssertPropertyDefaults(true));
     }
 
+    /**
+     * @dataProvider successInitialStateDataProvider
+     */
+    public function testMatchesSuccessInitialStateWithDefaultMethod(object $class): void
+    {
+        static::assertAccessorPropertyDefaults(get_class($class));
+    }
+
     /**
      * When turning off the propertyDefaultCheck, we can safely pass classes we know will fail the constraint
      *

diff --git a/tests/TestCase.php b/tests/TestCase.php
@@ -27,6 +27,10 @@ public function getClassDataProvider(string $path, string $namespacePrefix): Gen
                 continue;
             }
 
+            if (strpos($file->getFilename(), 'Abstract') === 0) {
+                continue;
+            }
+
             require_once $file->getPathname();
 
             $key       = str_replace([$path, '/'], ['', '\\'], $file->getPath()) . '\\' . $file->getBasename('.php');

diff --git a/tests/Unit/Constraint/ConstraintConfigTest.php b/tests/Unit/Constraint/ConstraintConfigTest.php
@@ -18,6 +18,10 @@ class ConstraintConfigTest extends TestCase
      * @covers ::hasAssertConstructor
      * @covers ::setAssertPropertyDefaults
      * @covers ::hasPropertyDefaultCheck
+     * @covers ::setAssertParentMethods
+     * @covers ::isAssertParentMethods
+     * @covers ::setExcludedMethods
+     * @covers ::getExcludedMethods
      * @covers ::getConstructorCallback
      * @covers ::setConstructorCallback
      */
@@ -27,16 +31,21 @@ public function testConfig(): void
         static::assertTrue($config->hasAccessorPairCheck());
         static::assertTrue($config->hasAssertConstructor());
         static::assertFalse($config->hasPropertyDefaultCheck());
+        static::assertTrue($config->isAssertParentMethods());
+        static::assertSame([], $config->getExcludedMethods());
 
         $config = new ConstraintConfig();
         static::assertFalse($config->setAssertAccessorPair(false)->hasAccessorPairCheck());
         static::assertFalse($config->setAssertConstructor(false)->hasAssertConstructor());
         static::assertFalse($config->setAssertPropertyDefaults(false)->hasPropertyDefaultCheck());
+        static::assertFalse($config->setAssertParentMethods(false)->isAssertParentMethods());
+        static::assertSame(['foobar'], $config->setExcludedMethods(['foobar'])->getExcludedMethods());
 
         $config = new ConstraintConfig();
         static::assertTrue($config->setAssertAccessorPair(true)->hasAccessorPairCheck());
         static::assertTrue($config->setAssertConstructor(true)->hasAssertConstructor());
         static::assertTrue($config->setAssertPropertyDefaults(true)->hasPropertyDefaultCheck());
+        static::assertTrue($config->setAssertParentMethods(true)->isAssertParentMethods());
 
         $config = new ConstraintConfig();
         $callback = static function (): array {

diff --git a/tests/Unit/Constraint/MethodPair/AbstractDataClass.php b/tests/Unit/Constraint/MethodPair/AbstractDataClass.php
@@ -0,0 +1,29 @@
+<?php
+declare(strict_types=1);
+
+namespace DigitalRevolution\AccessorPairConstraint\Tests\Unit\Constraint\MethodPair;
+
+use DigitalRevolution\AccessorPairConstraint\Constraint\ConstraintConfig;
+
+/**
+ * @suppressWarnings(PHPMD.NumberOfChildren)
+ */
+abstract class AbstractDataClass
+{
+    /**
+     * Return the config used for the AccessorPairProvider test
+     */
+    public function getConfig(): ConstraintConfig
+    {
+        return new ConstraintConfig();
+    }
+
+    /**
+     * Expect to return:
+     * - method name of getter
+     * - method name of setter
+     * - true if the method has multi setter
+     * @return array<int, array{0:string, 1:string, 2:bool}>
+     */
+    abstract public function getExpectedPairs(): array;
+}