Merge pull request #190 from rectorphp/phpunit-narrow-methods

[PHPUnit] Add NarrowMethodsRector

easy-coding-standard.neon

@@ -44,3 +44,9 @@ parameters:
         PHP_CodeSniffer\Standards\Generic\Sniffs\CodeAnalysis\EmptyStatementSniff:
             # intentionally
             - packages/BetterReflection/src/Reflector/SmartClassReflector.php
+
+    skip_codes:
+        SlevomatCodingStandard\Sniffs\TypeHints\TypeHintDeclarationSniff.MissingParameterTypeHint:
+            # forcing "mixed" types
+            - src/Node/NodeFactory.php
+            - packages/NodeTypeResolver/tests/AbstractNodeTypeResolverTest.php

packages/NodeTypeResolver/tests/AbstractNodeTypeResolverTest.php

@@ -48,9 +48,6 @@ abstract class AbstractNodeTypeResolverTest extends AbstractContainerAwareTestCa
         return $newStmts;
     }
 
-    /**
-     * @param mixed $expectedContent
-     */
     protected function doTestAttributeEquals(Node $node, string $attribute, $expectedContent): void
     {
         $this->assertSame($expectedContent, $node->getAttribute($attribute));

packages/ReflectionDocBlock/src/NodeAnalyzer/DocBlockAnalyzer.php

@@ -175,7 +175,6 @@ final class DocBlockAnalyzer
      * Magic untill, since object is read only
      *
      * @param object $object
-     * @param mixed $value
      */
     private function setPrivatePropertyValue($object, string $name, $value): void
     {

phpstan.neon

@@ -39,6 +39,10 @@ parameters:
         - '#Instanceof between PhpParser\\Node\\Expr\|string and PhpParser\\Node\\Name will always evaluate to false#'
         - '#Method Rector\\BetterReflection\\Reflector\\SmartClassReflector::reflect\(\) should return Rector\\BetterReflection\\Reflection\\ReflectionClass\|null but returns Rector\\BetterReflection\\Reflection\\Reflection#'
 
+        # known value of Name of MethodCall
+        - '#Call to an undefined method PhpParser\\Node\\Expr\|PhpParser\\Node\\Name::toString\(\)#'
+        - '#Cannot call method toString\(\) on PhpParser\\Node\\Expr\|string#'
+
         # buggy
         - '#Parameter \#1 \$classLikeNode of method Rector\\NodeAnalyzer\\ClassLikeAnalyzer::resolveExtendsTypes\(\) expects PhpParser\\Node\\Stmt\\Class_\|PhpParser\\Node\\Stmt\\Interface_, PhpParser\\Node\\Stmt\\ClassLike given#'
         - '#Parameter \#1 \$functionLikeNode of method Rector\\NodeTypeResolver\\TypeContext::getFunctionReflection\(\) expects PhpParser\\Node\\Expr\\Closure\|PhpParser\\Node\\Stmt\\ClassMethod\|PhpParser\\Node\\Stmt\\Function_, PhpParser\\Node\\FunctionLike given#'

src/Node/NodeFactory.php

@@ -175,9 +175,6 @@ final class NodeFactory
         return new Expression($assign);
     }
 
-    /**
-     * @param mixed $argument
-     */
     public function createArg($argument): Arg
     {
         $value = BuilderHelpers::normalizeValue($argument);

src/Rector/Contrib/PHPUnit/SpecificMethodBoolNullRector.php

@@ -0,0 +1,93 @@
+<?php declare(strict_types=1);
+
+namespace Rector\Rector\Contrib\PHPUnit;
+
+use PhpParser\Node;
+use PhpParser\Node\Expr\ConstFetch;
+use PhpParser\Node\Expr\MethodCall;
+use PhpParser\Node\Identifier;
+use Rector\NodeAnalyzer\MethodCallAnalyzer;
+use Rector\Rector\AbstractRector;
+
+/**
+ * Before:
+ * - $this->assertSame(null, $anything);
+ * - $this->assertSame(true, $anything);
+ * - $this->assertSame(false, $anything);
+ *
+ * After:
+ * - $this->assertNull($anything);
+ * - $this->assertTrue($anything);
+ * - $this->assertFalse($anything);
+ */
+final class SpecificMethodBoolNullRector extends AbstractRector
+{
+    /**
+     * @var string[]
+     */
+    private $constValueToMethodNames = [
+        'null' => 'assertNull',
+        'true' => 'assertTrue',
+        'false' => 'assertFalse',
+    ];
+
+    /**
+     * @var MethodCallAnalyzer
+     */
+    private $methodCallAnalyzer;
+
+    /**
+     * @var string|null
+     */
+    private $activeFuncCallName;
+
+    public function __construct(MethodCallAnalyzer $methodCallAnalyzer)
+    {
+        $this->methodCallAnalyzer = $methodCallAnalyzer;
+    }
+
+    public function isCandidate(Node $node): bool
+    {
+        $this->activeFuncCallName = null;
+
+        if (! $this->methodCallAnalyzer->isTypesAndMethods(
+            $node,
+            ['PHPUnit\Framework\TestCase', 'PHPUnit_Framework_TestCase'],
+            ['assertSame']
+        )) {
+            return false;
+        }
+
+        /** @var MethodCall $methodCallNode */
+        $methodCallNode = $node;
+
+        $firstArgumentValue = $methodCallNode->args[0]->value;
+        if (! $firstArgumentValue instanceof ConstFetch) {
+            return false;
+        }
+
+        $costName = $firstArgumentValue->name->toString();
+
+        return isset($this->constValueToMethodNames[$costName]);
+    }
+
+    /**
+     * @param MethodCall $methodCallNode
+     */
+    public function refactor(Node $methodCallNode): ?Node
+    {
+        /** @var ConstFetch $constFetchNode */
+        $constFetchNode = $methodCallNode->args[0]->value;
+        $constValue = $constFetchNode->name->toString();
+
+        $newMethodName = $this->constValueToMethodNames[$constValue];
+        $methodCallNode->name = new Identifier($newMethodName);
+
+        $methodArguments = $methodCallNode->args;
+        array_shift($methodArguments);
+
+        $methodCallNode->args = $methodArguments;
+
+        return $methodCallNode;
+    }
+}

src/Rector/Contrib/PHPUnit/SpecificMethodCountRector.php

@@ -0,0 +1,80 @@
+<?php declare(strict_types=1);
+
+namespace Rector\Rector\Contrib\PHPUnit;
+
+use PhpParser\Node;
+use PhpParser\Node\Expr\FuncCall;
+use PhpParser\Node\Expr\MethodCall;
+use PhpParser\Node\Identifier;
+use PhpParser\Node\Scalar\LNumber;
+use Rector\NodeAnalyzer\MethodCallAnalyzer;
+use Rector\Rector\AbstractRector;
+
+/**
+ * Before:
+ * - $this->assertSame(5, count($anything));
+ *
+ * After:
+ * - $this->assertCount(5, $anything);
+ */
+final class SpecificMethodCountRector extends AbstractRector
+{
+    /**
+     * @var MethodCallAnalyzer
+     */
+    private $methodCallAnalyzer;
+
+    /**
+     * @var string|null
+     */
+    private $activeFuncCallName;
+
+    public function __construct(MethodCallAnalyzer $methodCallAnalyzer)
+    {
+        $this->methodCallAnalyzer = $methodCallAnalyzer;
+    }
+
+    public function isCandidate(Node $node): bool
+    {
+        $this->activeFuncCallName = null;
+
+        if (! $this->methodCallAnalyzer->isTypesAndMethods(
+            $node,
+            ['PHPUnit\Framework\TestCase', 'PHPUnit_Framework_TestCase'],
+            ['assertSame']
+        )) {
+            return false;
+        }
+
+        /** @var MethodCall $methodCallNode */
+        $methodCallNode = $node;
+
+        $firstArgumentValue = $methodCallNode->args[0]->value;
+        if (! $firstArgumentValue instanceof LNumber) {
+            return false;
+        }
+
+        $secondArgumentValue = $methodCallNode->args[1]->value;
+
+        if (! $secondArgumentValue instanceof FuncCall) {
+            return false;
+        }
+
+        return $secondArgumentValue->name->toString() === 'count';
+    }
+
+    /**
+     * @param MethodCall $methodCallNode
+     */
+    public function refactor(Node $methodCallNode): ?Node
+    {
+        $methodCallNode->name = new Identifier('assertCount');
+
+        /** @var FuncCall $secondArgument */
+        $secondArgument = $methodCallNode->args[1]->value;
+
+        $methodCallNode->args[1] = $secondArgument->args[0];
+
+        return $methodCallNode;
+    }
+}

src/Rector/Contrib/PHPUnit/SpecificMethodObjectAttributeRector.php

@@ -0,0 +1,94 @@
+<?php declare(strict_types=1);
+
+namespace Rector\Rector\Contrib\PHPUnit;
+
+use PhpParser\Node;
+use PhpParser\Node\Arg;
+use PhpParser\Node\Expr\Isset_;
+use PhpParser\Node\Expr\MethodCall;
+use PhpParser\Node\Expr\PropertyFetch;
+use PhpParser\Node\Identifier;
+use PhpParser\Node\Scalar\String_;
+use Rector\NodeAnalyzer\MethodCallAnalyzer;
+use Rector\Rector\AbstractRector;
+
+/**
+ * Before:
+ * - $this->assertSame(5, count($anything));
+ *
+ * After:
+ * - $this->assertCount(5, $anything);
+ */
+final class SpecificMethodObjectAttributeRector extends AbstractRector
+{
+    /**
+     * @var MethodCallAnalyzer
+     */
+    private $methodCallAnalyzer;
+
+    /**
+     * @var string|null
+     */
+    private $activeFuncCallName;
+
+    public function __construct(MethodCallAnalyzer $methodCallAnalyzer)
+    {
+        $this->methodCallAnalyzer = $methodCallAnalyzer;
+    }
+
+    public function isCandidate(Node $node): bool
+    {
+        $this->activeFuncCallName = null;
+
+        if (! $this->methodCallAnalyzer->isTypesAndMethods(
+            $node,
+            ['PHPUnit\Framework\TestCase', 'PHPUnit_Framework_TestCase'],
+            ['assertTrue', 'assertFalse']
+        )) {
+            return false;
+        }
+
+        /** @var MethodCall $methodCallNode */
+        $methodCallNode = $node;
+
+        $firstArgumentValue = $methodCallNode->args[0]->value;
+
+        // is property access
+        if (! $firstArgumentValue instanceof Isset_) {
+            return false;
+        }
+
+        /** @var Isset_ $issetNode */
+        $issetNode = $firstArgumentValue;
+
+        return $issetNode->vars[0] instanceof PropertyFetch;
+    }
+
+    /**
+     * @param MethodCall $methodCallNode
+     */
+    public function refactor(Node $methodCallNode): ?Node
+    {
+        // rename method
+        if ($methodCallNode->name->toString() === 'assertTrue') {
+            $methodCallNode->name = new Identifier('assertObjectHasAttribute');
+        } else {
+            $methodCallNode->name = new Identifier('assertObjectNotHasAttribute');
+        }
+
+        // move isset to property and object
+        /** @var Isset_ $issetNode */
+        $issetNode = $methodCallNode->args[0]->value;
+
+        /** @var PropertyFetch $propertyFetchNode */
+        $propertyFetchNode = $issetNode->vars[0];
+
+        // and set as arguments
+        $methodCallNode->args = [
+            new Arg(new String_($propertyFetchNode->name->toString())),
+            new Arg($propertyFetchNode->var),
+        ];
+
+        return $methodCallNode;
+    }
+}

src/config/level/phpunit/phpunit60.yml

@@ -1,7 +1,6 @@
 rectors:
     Rector\Rector\Contrib\PHPUnit\ExceptionAnnotationRector: ~
     Rector\Rector\Contrib\PHPUnit\GetMockRector: ~
-    Rector\Rector\Contrib\PHPUnit\SpecificMethodRector: ~
 
     # ref. https://github.com/sebastianbergmann/phpunit/compare/5.7.9...6.0.0
     Rector\Rector\Dynamic\PseudoNamespaceToNamespaceRector:
@@ -16,3 +15,9 @@ rectors:
         'PHPUnit\Framework\TestClass':
             'setExpectedException': 'expectedException'
             'setExpectedExceptionRegExp': 'expectedException'
+
+    # possibly earlier
+    Rector\Rector\Contrib\PHPUnit\SpecificMethodRector: ~
+    Rector\Rector\Contrib\PHPUnit\SpecificMethodBoolNullRector: ~
+    Rector\Rector\Contrib\PHPUnit\SpecificMethodCountRector: ~
+    Rector\Rector\Contrib\PHPUnit\SpecificMethodObjectAttributeRector: ~

tests/Rector/Contrib/PHPUnit/SpecificMethodBoolNullRector/Correct/correct.php.inc

@@ -0,0 +1,11 @@
+<?php declare(strict_types=1);
+
+final class MyTest extends \PHPUnit\Framework\TestCase
+{
+    public function test()
+    {
+        $this->assertNull('second argument');
+        $this->assertFalse('second argument');
+        $this->assertTrue('second argument');
+    }
+}

tests/Rector/Contrib/PHPUnit/SpecificMethodBoolNullRector/Test.php

@@ -0,0 +1,25 @@
+<?php declare(strict_types=1);
+
+namespace Rector\Tests\Rector\Contrib\PHPUnit\SpecificMethodBoolNullRector;
+
+use Rector\Rector\Contrib\PHPUnit\SpecificMethodBoolNullRector;
+use Rector\Testing\PHPUnit\AbstractRectorTestCase;
+
+final class Test extends AbstractRectorTestCase
+{
+    public function test(): void
+    {
+        $this->doTestFileMatchesExpectedContent(
+            __DIR__ . '/Wrong/wrong.php.inc',
+            __DIR__ . '/Correct/correct.php.inc'
+        );
+    }
+
+    /**
+     * @return string[]
+     */
+    protected function getRectorClasses(): array
+    {
+        return [SpecificMethodBoolNullRector::class];
+    }
+}

tests/Rector/Contrib/PHPUnit/SpecificMethodBoolNullRector/Wrong/wrong.php.inc

@@ -0,0 +1,11 @@
+<?php declare(strict_types=1);
+
+final class MyTest extends \PHPUnit\Framework\TestCase
+{
+    public function test()
+    {
+        $this->assertSame(null, 'second argument');
+        $this->assertSame(false, 'second argument');
+        $this->assertSame(true, 'second argument');
+    }
+}

tests/Rector/Contrib/PHPUnit/SpecificMethodCountRector/Correct/correct.php.inc

@@ -0,0 +1,9 @@
+<?php declare(strict_types=1);
+
+final class MyTest extends \PHPUnit\Framework\TestCase
+{
+    public function test()
+    {
+        $this->assertCount(5, $something);
+    }
+}

tests/Rector/Contrib/PHPUnit/SpecificMethodCountRector/Test.php

@@ -0,0 +1,25 @@
+<?php declare(strict_types=1);
+
+namespace Rector\Tests\Rector\Contrib\PHPUnit\SpecificMethodCountRector;
+
+use Rector\Rector\Contrib\PHPUnit\SpecificMethodCountRector;
+use Rector\Testing\PHPUnit\AbstractRectorTestCase;
+
+final class Test extends AbstractRectorTestCase
+{
+    public function test(): void
+    {
+        $this->doTestFileMatchesExpectedContent(
+            __DIR__ . '/Wrong/wrong.php.inc',
+            __DIR__ . '/Correct/correct.php.inc'
+        );
+    }
+
+    /**
+     * @return string[]
+     */
+    protected function getRectorClasses(): array
+    {
+        return [SpecificMethodCountRector::class];
+    }
+}

tests/Rector/Contrib/PHPUnit/SpecificMethodCountRector/Wrong/wrong.php.inc

@@ -0,0 +1,9 @@
+<?php declare(strict_types=1);
+
+final class MyTest extends \PHPUnit\Framework\TestCase
+{
+    public function test()
+    {
+        $this->assertSame(5, count($something));
+    }
+}

tests/Rector/Contrib/PHPUnit/SpecificMethodObjectAttributeRector/Correct/correct.php.inc

@@ -0,0 +1,10 @@
+<?php declare(strict_types=1);
+
+final class MyTest extends \PHPUnit\Framework\TestCase
+{
+    public function test()
+    {
+        $this->assertObjectHasAttribute('value1', $node);
+        $this->assertObjectNotHasAttribute('value2', $node);
+    }
+}

tests/Rector/Contrib/PHPUnit/SpecificMethodObjectAttributeRector/Test.php

@@ -0,0 +1,25 @@
+<?php declare(strict_types=1);
+
+namespace Rector\Tests\Rector\Contrib\PHPUnit\SpecificMethodObjectAttributeRector;
+
+use Rector\Rector\Contrib\PHPUnit\SpecificMethodObjectAttributeRector;
+use Rector\Testing\PHPUnit\AbstractRectorTestCase;
+
+final class Test extends AbstractRectorTestCase
+{
+    public function test(): void
+    {
+        $this->doTestFileMatchesExpectedContent(
+            __DIR__ . '/Wrong/wrong.php.inc',
+            __DIR__ . '/Correct/correct.php.inc'
+        );
+    }
+
+    /**
+     * @return string[]
+     */
+    protected function getRectorClasses(): array
+    {
+        return [SpecificMethodObjectAttributeRector::class];
+    }
+}

tests/Rector/Contrib/PHPUnit/SpecificMethodObjectAttributeRector/Wrong/wrong.php.inc

@@ -0,0 +1,10 @@
+<?php declare(strict_types=1);
+
+final class MyTest extends \PHPUnit\Framework\TestCase
+{
+    public function test()
+    {
+        $this->assertTrue(isset($node->value1));
+        $this->assertFalse(isset($node->value2));
+    }
+}