[SOLID] Prefer interface if possible (#2123)

[SOLID] Prefer interface if possible

composer.json

@@ -174,7 +174,8 @@
             "packages/TypeDeclaration/tests/Rector/FunctionLike/ReturnTypeDeclarationRector/Source/MyBar.php",
             "packages/Renaming/tests/Rector/Class_/RenameClassRector/Source/Twig_Extension_Sandbox.php",
             "packages/Renaming/tests/Rector/Class_/RenameClassRector/Source/TwigFilter.php",
-            "packages/Renaming/tests/Rector/Class_/RenameClassRector/Source/Manual_Twig_Filter.php"
+            "packages/Renaming/tests/Rector/Class_/RenameClassRector/Source/Manual_Twig_Filter.php",
+            "packages/SOLID/tests/Rector/ClassMethod/UseInterfaceOverImplementationInConstructorRector/Source/Apple.php"
         ]
     },
     "scripts": {
@@ -211,4 +212,4 @@
             "dev-master": "0.6-dev"
         }
     }
-}
+}

config/set/code-quality/code-quality.yaml

@@ -42,3 +42,4 @@ services:
     Rector\CodeQuality\Rector\Catch_\ThrowWithPreviousExceptionRector: ~
     Rector\CodeQuality\Rector\FuncCall\RemoveSoleValueSprintfRector: ~
     Rector\CodeQuality\Rector\If_\ShortenElseIfRector: ~
+    Rector\SOLID\Rector\ClassMethod\UseInterfaceOverImplementationInConstructorRector: ~

ecs.yaml

@@ -66,6 +66,8 @@ parameters:
         - '*utils/ContributorTools/templates/*'
         - 'stubs/*'
         - '*/Expected/*'
+        # exclude
+        - 'src/Rector/AbstractRector.php'
 
     skip:
         PHP_CodeSniffer\Standards\PSR2\Sniffs\Methods\MethodDeclarationSniff.Underscore: ~

packages/SOLID/src/Rector/ClassMethod/UseInterfaceOverImplementationInConstructorRector.php

@@ -0,0 +1,157 @@
+<?php declare(strict_types=1);
+
+namespace Rector\SOLID\Rector\ClassMethod;
+
+use PhpParser\Node;
+use PhpParser\Node\Name\FullyQualified;
+use PhpParser\Node\Stmt\ClassMethod;
+use Rector\Rector\AbstractRector;
+use Rector\RectorDefinition\CodeSample;
+use Rector\RectorDefinition\RectorDefinition;
+
+/**
+ * @see \Rector\SOLID\Tests\Rector\ClassMethod\UseInterfaceOverImplementationInConstructorRector\UseInterfaceOverImplementationInConstructorRectorTest
+ */
+final class UseInterfaceOverImplementationInConstructorRector extends AbstractRector
+{
+    public function getDefinition(): RectorDefinition
+    {
+        return new RectorDefinition('Use interface instead of specific class', [
+            new CodeSample(
+                <<<'PHP'
+class SomeClass
+{
+    public function __construct(SomeImplementation $someImplementation)
+    {
+    }
+}
+
+class SomeImplementation implements SomeInterface
+{
+}
+
+interface SomeInterface
+{
+}
+PHP
+                ,
+                <<<'PHP'
+class SomeClass
+{
+    public function __construct(SomeInterface $someImplementation)
+    {
+    }
+}
+
+class SomeImplementation implements SomeInterface
+{
+}
+
+interface SomeInterface
+{
+}
+PHP
+            ),
+        ]);
+    }
+
+    /**
+     * @return string[]
+     */
+    public function getNodeTypes(): array
+    {
+        return [ClassMethod::class];
+    }
+
+    /**
+     * @param ClassMethod $node
+     */
+    public function refactor(Node $node): ?Node
+    {
+        if (! $this->isName($node, '__construct')) {
+            return null;
+        }
+
+        foreach ($node->params as $param) {
+            if ($param->type === null) {
+                continue;
+            }
+
+            $typeName = $this->getName($param->type);
+            if ($typeName === null) {
+                continue;
+            }
+
+            if (! class_exists($typeName)) {
+                continue;
+            }
+
+            $implementedInterfaces = class_implements($typeName);
+            if ($implementedInterfaces === []) {
+                continue;
+            }
+
+            $interfaceNames = $this->getClassDirectInterfaces($typeName);
+            $interfaceNames = $this->filterOutInterfaceThatHaveTwoAndMoreImplementers($interfaceNames);
+
+            if (count($interfaceNames) !== 1) {
+                continue;
+            }
+
+            $param->type = new FullyQualified($interfaceNames[0]);
+        }
+
+        return $node;
+    }
+
+    /**
+     * @return string[]
+     */
+    private function getClassDirectInterfaces(string $typeName): array
+    {
+        $interfaceNames = class_implements($typeName);
+        foreach ($interfaceNames as $possibleDirectInterfaceName) {
+            foreach ($interfaceNames as $key => $interfaceName) {
+                if ($possibleDirectInterfaceName === $interfaceName) {
+                    continue;
+                }
+
+                if (! is_a($possibleDirectInterfaceName, $interfaceName, true)) {
+                    continue;
+                }
+
+                unset($interfaceNames[$key]);
+            }
+        }
+
+        return array_values($interfaceNames);
+    }
+
+    /**
+     * 2 and more classes that implement the same interface → better skip it, the particular implementation is used on purpose probably
+     *
+     * @param string[] $interfaceNames
+     * @return string[]
+     */
+    private function filterOutInterfaceThatHaveTwoAndMoreImplementers(array $interfaceNames): array
+    {
+        $classes = get_declared_classes();
+        foreach ($interfaceNames as $key => $interfaceName) {
+            $implementations = [];
+            foreach ($classes as $class) {
+                $interfacesImplementedByClass = class_implements($class);
+                if (! in_array($interfaceName, $interfacesImplementedByClass, true)) {
+                    continue;
+                }
+
+                $implementations[] = $class;
+            }
+
+            if (count($implementations) > 1) {
+                unset($interfaceNames[$key]);
+            }
+        }
+
+        return $interfaceNames;
+    }
+}

packages/SOLID/tests/Rector/ClassMethod/UseInterfaceOverImplementationInConstructorRector/Fixture/fixture.php.inc

@@ -0,0 +1,41 @@
+<?php
+
+namespace Rector\SOLID\Tests\Rector\ClassMethod\UseInterfaceOverImplementationInConstructorRector\Fixture;
+
+class SomeClass
+{
+    public function __construct(\Rector\SOLID\Tests\Rector\ClassMethod\UseInterfaceOverImplementationInConstructorRector\Fixture\SomeImplementation $someImplementation)
+    {
+    }
+}
+
+class SomeImplementation implements SomeInterface
+{
+}
+
+interface SomeInterface
+{
+}
+
+?>
+-----
+<?php
+
+namespace Rector\SOLID\Tests\Rector\ClassMethod\UseInterfaceOverImplementationInConstructorRector\Fixture;
+
+class SomeClass
+{
+    public function __construct(\Rector\SOLID\Tests\Rector\ClassMethod\UseInterfaceOverImplementationInConstructorRector\Fixture\SomeInterface $someImplementation)
+    {
+    }
+}
+
+class SomeImplementation implements SomeInterface
+{
+}
+
+interface SomeInterface
+{
+}
+
+?>

packages/SOLID/tests/Rector/ClassMethod/UseInterfaceOverImplementationInConstructorRector/Fixture/prefer_first_parent_interface.php.inc

@@ -0,0 +1,29 @@
+<?php
+
+namespace Rector\SOLID\Tests\Rector\ClassMethod\UseInterfaceOverImplementationInConstructorRector\Fixture;
+
+use Rector\SOLID\Tests\Rector\ClassMethod\UseInterfaceOverImplementationInConstructorRector\Source\ClassThatImplementsInterfaceThatExtendsInterface;
+
+class PreferFirstParentInterface
+{
+    public function __construct(ClassThatImplementsInterfaceThatExtendsInterface $someImplementation)
+    {
+    }
+}
+
+?>
+-----
+<?php
+
+namespace Rector\SOLID\Tests\Rector\ClassMethod\UseInterfaceOverImplementationInConstructorRector\Fixture;
+
+use Rector\SOLID\Tests\Rector\ClassMethod\UseInterfaceOverImplementationInConstructorRector\Source\ClassThatImplementsInterfaceThatExtendsInterface;
+
+class PreferFirstParentInterface
+{
+    public function __construct(\Rector\SOLID\Tests\Rector\ClassMethod\UseInterfaceOverImplementationInConstructorRector\Source\InterfaceExtendingInterface $someImplementation)
+    {
+    }
+}
+
+?>

packages/SOLID/tests/Rector/ClassMethod/UseInterfaceOverImplementationInConstructorRector/Fixture/skip_class_that_implements_interface_with_multiple_children.php.inc

@@ -0,0 +1,12 @@
+<?php
+
+namespace Rector\SOLID\Tests\Rector\ClassMethod\UseInterfaceOverImplementationInConstructorRector\Fixture;
+
+use Rector\SOLID\Tests\Rector\ClassMethod\UseInterfaceOverImplementationInConstructorRector\Source\Orange;
+
+class SkipClassThatImplementsInterfaceWithMultipleChildren
+{
+    public function __construct(Orange $someImplementation)
+    {
+    }
+}

packages/SOLID/tests/Rector/ClassMethod/UseInterfaceOverImplementationInConstructorRector/Fixture/skip_interface.php.inc

@@ -0,0 +1,12 @@
+<?php
+
+namespace Rector\SOLID\Tests\Rector\ClassMethod\UseInterfaceOverImplementationInConstructorRector\Fixture;
+
+use Rector\SOLID\Tests\Rector\ClassMethod\UseInterfaceOverImplementationInConstructorRector\Source\InterfaceExtendingInterface;
+
+class SkipInterface
+{
+    public function __construct(InterfaceExtendingInterface $someImplementation)
+    {
+    }
+}

packages/SOLID/tests/Rector/ClassMethod/UseInterfaceOverImplementationInConstructorRector/Fixture/skip_multiple_interfaces.php.inc

@@ -0,0 +1,12 @@
+<?php
+
+namespace Rector\SOLID\Tests\Rector\ClassMethod\UseInterfaceOverImplementationInConstructorRector\Fixture;
+
+use Rector\SOLID\Tests\Rector\ClassMethod\UseInterfaceOverImplementationInConstructorRector\Source\ClassThatImplementsMoreInterfaces;
+
+class SkipMultipleInterfaces
+{
+    public function __construct(ClassThatImplementsMoreInterfaces $someImplementation)
+    {
+    }
+}

packages/SOLID/tests/Rector/ClassMethod/UseInterfaceOverImplementationInConstructorRector/Fixture/skip_simple_type.php.inc

@@ -0,0 +1,10 @@
+<?php
+
+namespace Rector\SOLID\Tests\Rector\ClassMethod\UseInterfaceOverImplementationInConstructorRector\Fixture;
+
+class SkipSimpleType
+{
+    public function __construct(int $someImplementation)
+    {
+    }
+}

packages/SOLID/tests/Rector/ClassMethod/UseInterfaceOverImplementationInConstructorRector/Fixture/skip_sole_class.php.inc

@@ -0,0 +1,14 @@
+<?php
+
+namespace Rector\SOLID\Tests\Rector\ClassMethod\UseInterfaceOverImplementationInConstructorRector\Fixture;
+
+class SkipSoleClass
+{
+    public function __construct(NoInterface $someImplementation)
+    {
+    }
+}
+
+class NoInterface
+{
+}

packages/SOLID/tests/Rector/ClassMethod/UseInterfaceOverImplementationInConstructorRector/Source/Apple.php

@@ -0,0 +1,8 @@
+<?php declare(strict_types=1);
+
+namespace Rector\SOLID\Tests\Rector\ClassMethod\UseInterfaceOverImplementationInConstructorRector\Source;
+
+class Apple implements Fruit
+{
+
+}

packages/SOLID/tests/Rector/ClassMethod/UseInterfaceOverImplementationInConstructorRector/Source/ClassThatImplementsInterfaceThatExtendsInterface.php

@@ -0,0 +1,8 @@
+<?php declare(strict_types=1);
+
+namespace Rector\SOLID\Tests\Rector\ClassMethod\UseInterfaceOverImplementationInConstructorRector\Source;
+
+final class ClassThatImplementsInterfaceThatExtendsInterface implements InterfaceExtendingInterface
+{
+
+}

packages/SOLID/tests/Rector/ClassMethod/UseInterfaceOverImplementationInConstructorRector/Source/ClassThatImplementsMoreInterfaces.php

@@ -0,0 +1,8 @@
+<?php declare(strict_types=1);
+
+namespace Rector\SOLID\Tests\Rector\ClassMethod\UseInterfaceOverImplementationInConstructorRector\Source;
+
+final class ClassThatImplementsMoreInterfaces implements InterfaceThree, InterfaceTwo
+{
+
+}

packages/SOLID/tests/Rector/ClassMethod/UseInterfaceOverImplementationInConstructorRector/Source/Fruit.php

@@ -0,0 +1,8 @@
+<?php declare(strict_types=1);
+
+namespace Rector\SOLID\Tests\Rector\ClassMethod\UseInterfaceOverImplementationInConstructorRector\Source;
+
+interface Fruit
+{
+
+}

packages/SOLID/tests/Rector/ClassMethod/UseInterfaceOverImplementationInConstructorRector/Source/InterfaceExtendingInterface.php

@@ -0,0 +1,8 @@
+<?php declare(strict_types=1);
+
+namespace Rector\SOLID\Tests\Rector\ClassMethod\UseInterfaceOverImplementationInConstructorRector\Source;
+
+interface InterfaceExtendingInterface extends InterfaceOne
+{
+
+}

packages/SOLID/tests/Rector/ClassMethod/UseInterfaceOverImplementationInConstructorRector/Source/InterfaceOne.php

@@ -0,0 +1,8 @@
+<?php declare(strict_types=1);
+
+namespace Rector\SOLID\Tests\Rector\ClassMethod\UseInterfaceOverImplementationInConstructorRector\Source;
+
+interface InterfaceOne
+{
+
+}

packages/SOLID/tests/Rector/ClassMethod/UseInterfaceOverImplementationInConstructorRector/Source/InterfaceThree.php

@@ -0,0 +1,8 @@
+<?php declare(strict_types=1);
+
+namespace Rector\SOLID\Tests\Rector\ClassMethod\UseInterfaceOverImplementationInConstructorRector\Source;
+
+interface InterfaceThree
+{
+
+}

packages/SOLID/tests/Rector/ClassMethod/UseInterfaceOverImplementationInConstructorRector/Source/InterfaceTwo.php

@@ -0,0 +1,8 @@
+<?php declare(strict_types=1);
+
+namespace Rector\SOLID\Tests\Rector\ClassMethod\UseInterfaceOverImplementationInConstructorRector\Source;
+
+interface InterfaceTwo
+{
+
+}

packages/SOLID/tests/Rector/ClassMethod/UseInterfaceOverImplementationInConstructorRector/Source/Orange.php

@@ -0,0 +1,8 @@
+<?php declare(strict_types=1);
+
+namespace Rector\SOLID\Tests\Rector\ClassMethod\UseInterfaceOverImplementationInConstructorRector\Source;
+
+class Orange implements Fruit
+{
+
+}

packages/SOLID/tests/Rector/ClassMethod/UseInterfaceOverImplementationInConstructorRector/UseInterfaceOverImplementationInConstructorRectorTest.php

@@ -0,0 +1,34 @@
+<?php declare(strict_types=1);
+
+namespace Rector\SOLID\Tests\Rector\ClassMethod\UseInterfaceOverImplementationInConstructorRector;
+
+use Iterator;
+use Rector\SOLID\Rector\ClassMethod\UseInterfaceOverImplementationInConstructorRector;
+use Rector\Testing\PHPUnit\AbstractRectorTestCase;
+
+final class UseInterfaceOverImplementationInConstructorRectorTest extends AbstractRectorTestCase
+{
+    /**
+     * @dataProvider provideDataForTest()
+     */
+    public function test(string $file): void
+    {
+        $this->doTestFile($file);
+    }
+
+    public function provideDataForTest(): Iterator
+    {
+        yield [__DIR__ . '/Fixture/fixture.php.inc'];
+        yield [__DIR__ . '/Fixture/prefer_first_parent_interface.php.inc'];
+        yield [__DIR__ . '/Fixture/skip_multiple_interfaces.php.inc'];
+        yield [__DIR__ . '/Fixture/skip_simple_type.php.inc'];
+        yield [__DIR__ . '/Fixture/skip_interface.php.inc'];
+        yield [__DIR__ . '/Fixture/skip_sole_class.php.inc'];
+        yield [__DIR__ . '/Fixture/skip_class_that_implements_interface_with_multiple_children.php.inc'];
+    }
+
+    protected function getRectorClass(): string
+    {
+        return UseInterfaceOverImplementationInConstructorRector::class;
+    }
+}

phpstan.neon

@@ -204,3 +204,6 @@ parameters:
         # known value
         - '#Parameter \#1 \$node of method Rector\\DeadCode\\Rector\\ClassMethod\\RemoveDelegatingParentCallRector\:\:unwrapExpression\(\) expects PhpParser\\Node, PhpParser\\Node\\Stmt\|null given#'
         - '#Method Rector\\StrictCodeQuality\\Rector\\Stmt\\VarInlineAnnotationToAssertRector\:\:findVariableByName\(\) should return PhpParser\\Node\\Expr\\Variable\|null but returns PhpParser\\Node\|null#'
+        - '#PHPDoc tag @param references unknown parameter\: \$commanders#'
+        - '#In method "Rector\\Rector\\AbstractRector\:\:autowireAbstractRectorDependencies", parameter \$symfonyStyle type is type\-hinted to "Symfony\\Component\\Console\\Style\\SymfonyStyle" but the @param annotation says it is a "array<Rector\\Contract\\PhpParser\\Node\\CommanderInterface\>"\. Please fix the @param annotation#'
+        - '#PHPDoc tag @param for parameter \$symfonyStyle with type array<Rector\\Contract\\PhpParser\\Node\\CommanderInterface\> is incompatible with native type Symfony\\Component\\Console\\Style\\SymfonyStyle#'

rector.yaml

@@ -18,4 +18,4 @@ parameters:
     php_version_features: '7.1'
 
 services:
-    Rector\DeadCode\Rector\Stmt\RemoveUnreachableStatementRector: ~
+    Rector\SOLID\Rector\ClassMethod\UseInterfaceOverImplementationInConstructorRector: ~

src/Rector/AbstractRector.php

@@ -9,6 +9,7 @@ use PhpParser\Node\Name;
 use PhpParser\Node\Stmt;
 use PhpParser\Node\Stmt\Expression;
 use PhpParser\NodeVisitorAbstract;
+use Rector\Contract\PhpParser\Node\CommanderInterface;
 use Rector\Contract\Rector\PhpRectorInterface;
 use Rector\NodeTypeResolver\Node\AttributeKey;
 use Rector\Php\PhpVersionProvider;
@@ -36,6 +37,11 @@ abstract class AbstractRector extends NodeVisitorAbstract implements PhpRectorIn
      */
     private $phpVersionProvider;
 
+    /**
+     * @var CommanderInterface[]
+     */
+    private $commanders = [];
+
     /**
      * Run once in the every end of one processed file
      */
@@ -45,15 +51,18 @@ abstract class AbstractRector extends NodeVisitorAbstract implements PhpRectorIn
 
     /**
      * @required
+     * @param CommanderInterface[] $symfonyStyle
      */
     public function autowireAbstractRectorDependencies(
         SymfonyStyle $symfonyStyle,
         PhpVersionProvider $phpVersionProvider,
         BuilderFactory $builderFactory
+        // array $commanders = []
     ): void {
         $this->symfonyStyle = $symfonyStyle;
         $this->phpVersionProvider = $phpVersionProvider;
         $this->builderFactory = $builderFactory;
+        // $this->commanders = $commanders;
     }
 
     /**
@@ -114,6 +123,13 @@ abstract class AbstractRector extends NodeVisitorAbstract implements PhpRectorIn
      */
     public function afterTraverse(array $nodes): array
     {
+//        foreach ($this->commanders as $commander) {
+//            if (! $commander->isActive()) {
+//                continue;
+//            }
+//
+//            $nodes = $commander->traverseNodes($nodes);
+//        }
         if ($this->nodeAddingCommander->isActive()) {
             $nodes = $this->nodeAddingCommander->traverseNodes($nodes);
         }
@@ -135,8 +151,6 @@ abstract class AbstractRector extends NodeVisitorAbstract implements PhpRectorIn
             $nodes = $this->useAddingCommander->traverseNodes($nodes);
         }
 
-        // @todo class like renaming
-
         $this->tearDown();
 
         return $nodes;