Fix Nullable Types

Added Nullable Types and added some just integration-tested coverage in unit tests
isfett · Jan 21, 2020 · 1d2b287 · 1d2b287
1 parent 9e1081c
commit 1d2b287
Show file tree

Hide file tree

Showing 12 changed files with 167 additions and 22 deletions.
diff --git a/.travis.yml b/.travis.yml
@@ -15,7 +15,8 @@ script:
   - vendor/bin/phpmd src text vendor/isfett/coding-standard/phpmd/isfett.xml
   - bin/php-analyzer magic-number-detector --visitors=Argument,Array,Assign,Condition,DefaultParameter,Operation,Property,Return,SwitchCase,Ternary --processors=IgnoreDefineFunction --excludes=vendor,var,tests,docs,bin --exclude-files=Kernel.php
   - bin/php-analyzer magic-string-detector --visitors=Argument,Array,Assign,Condition,DefaultParameter,Operation,Property,Return,SwitchCase,Ternary --processors=IgnoreDefineFunction --excludes=vendor,var,tests,docs,bin --exclude-files=Kernel.php
-  - vendor/bin/phpunit --testsuite=unit,integration --coverage-clover=coverage.xml
+  - vendor/bin/phpunit --testsuite=unit --coverage-clover=coverage.xml
+  - vendor/bin/phpunit --testsuite=integration --no-coverage
 
 after_success:
   - bash <(curl -s https://codecov.io/bash)

diff --git a/src/Console/Application.php b/src/Console/Application.php
@@ -22,7 +22,7 @@ class Application extends BaseApplication
     private const APPLICATION_NAME = 'php-analyzer';
 
     /** @var string */
-    private const APPLICATION_VERSION = '1.2.3';
+    private const APPLICATION_VERSION = '1.2.4';
 
     /** @var string */
     private const ARGUMENT_HELP = 'help';

diff --git a/src/Node/Representation/NullableType.php b/src/Node/Representation/NullableType.php
@@ -0,0 +1,27 @@
+<?php
+declare(strict_types = 1);
+
+namespace Isfett\PhpAnalyzer\Node\Representation;
+
+/**
+ * Class NullableType
+ */
+class NullableType extends AbstractRepresentation
+{
+    /** @var string */
+    private const NULLABLE_OPERATOR = '?';
+
+    /** @var string */
+    private const REPRESENTATION_FORMAT = '%s%s';
+
+    /**
+     * @return string
+     */
+    public function representation(): string
+    {
+        /** @var \PhpParser\Node\NullableType $node */
+        $node = $this->node;
+
+        return sprintf(self::REPRESENTATION_FORMAT, self::NULLABLE_OPERATOR, $this->representate($node->type));
+    }
+}
diff --git a/src/Node/Representation/Param.php b/src/Node/Representation/Param.php
@@ -20,7 +20,7 @@ public function representation(): string
         $node = $this->node;
 
         $byRef = $node->byRef ? self::REF_SIGN : self::EMPTY_STRING;
-        $type = $node->type ? $node->type . self::SPACE : self::EMPTY_STRING;
+        $type = $node->type ? $this->representate($node->type) . self::SPACE : self::EMPTY_STRING;
         $default = $node->default ?
             self::SPACE . self::EQUAL_SIGN . self::SPACE . $this->representate($node->default) :
             self::EMPTY_STRING;

diff --git a/tests/Integration/Console/ApplicationTest.php b/tests/Integration/Console/ApplicationTest.php
@@ -14,7 +14,7 @@
 class ApplicationTest extends TestCase
 {
     /** @var string */
-    private const APPLICATION_INFO = 'php-analyzer 1.2.3 by Christopher Stenke <[email protected]>' . \PHP_EOL;
+    private const APPLICATION_INFO = 'php-analyzer 1.2.4 by Christopher Stenke <[email protected]>' . \PHP_EOL;
 
     /** @var Application */
     private $application;

diff --git a/tests/Integration/Console/Command/MagicNumberDetectorCommandTest.php b/tests/Integration/Console/Command/MagicNumberDetectorCommandTest.php
@@ -92,7 +92,7 @@ public function testRun(): void
 |--------------------------------------------------------------|----------------------------------------------|
 | round(\$input, <focus>4</focus>)                              | magic_number_detector_integrationtest.php:25 |
 |--------------------------------------------------------------|----------------------------------------------|
-| \$input = <focus>4</focus>                                    | magic_number_detector_integrationtest.php:13 |
+| ?int \$input = <focus>4</focus>                               | magic_number_detector_integrationtest.php:13 |
 |--------------------------------------------------------------|----------------------------------------------|
 | case <focus>5</focus>:                                       | magic_number_detector_integrationtest.php:20 |
 |--------------------------------------------------------------|----------------------------------------------|

diff --git a/tests/Unit/Node/AbstractNodeTestCase.php b/tests/Unit/Node/AbstractNodeTestCase.php
@@ -94,12 +94,19 @@ protected function createNameNode(string $name = 'mockedName'): Node\Name
 
     /**
      * @param string $name
+     * @param bool   $nullable
      *
      * @return Node\Identifier
      */
-    protected function createIdentifierNode(string $name = 'mockedName'): Node\Identifier
+    protected function createIdentifierNode(string $name = 'mockedName', bool $nullable = false): Node
     {
-        return new Node\Identifier($name, $this->getNodeAttributes());
+        $identifier = new Node\Identifier($name, $this->getNodeAttributes());
+
+        if ($nullable) {
+            $identifier = new Node\NullableType($identifier, $this->getNodeAttributes());
+        }
+
+        return $identifier;
     }
 
     /**

diff --git a/tests/Unit/Node/Processor/MagicNumber/IgnoreArrayKeyProcessorTest.php b/tests/Unit/Node/Processor/MagicNumber/IgnoreArrayKeyProcessorTest.php
@@ -8,6 +8,7 @@
 use Isfett\PhpAnalyzer\Tests\Unit\Node\AbstractNodeTestCase;
 use PhpParser\Node\Expr\ArrayItem;
 use PhpParser\Node\Scalar\LNumber;
+use PhpParser\Node\Stmt\Case_;
 
 /**
  * Class IgnoreArrayKeyProcessorTest
@@ -104,4 +105,28 @@ public function testProcessWillNotRemoveOtherValues(): void
 
         $this->assertCount(1, $nodeOccurrenceList->getOccurrences());
     }
+
+    /**
+     * @return void
+     */
+    public function testProcessWithoutArrayItemWillNotGetRemoved(): void
+    {
+        $node = new LNumber(36);
+        $node->setAttribute('parent', new Case_(
+            new LNumber(36)
+        ));
+
+        $occurrence = $this->createOccurrence($node);
+
+        $nodeOccurrenceList = new OccurrenceList();
+        $nodeOccurrenceList->addOccurrence($occurrence);
+        $this->processor->setNodeOccurrenceList($nodeOccurrenceList);
+
+        $this->assertCount(1, $nodeOccurrenceList->getOccurrences());
+        $this->assertEmpty($occurrence->getAffectedByProcessors());
+
+        $this->processor->process($occurrence);
+
+        $this->assertCount(1, $nodeOccurrenceList->getOccurrences());
+    }
 }
diff --git a/tests/Unit/Node/Processor/MagicString/IgnoreArrayKeyProcessorTest.php b/tests/Unit/Node/Processor/MagicString/IgnoreArrayKeyProcessorTest.php
@@ -9,6 +9,7 @@
 use PhpParser\Node\Expr\ArrayItem;
 use PhpParser\Node\Scalar\LNumber;
 use PhpParser\Node\Scalar\String_;
+use PhpParser\Node\Stmt\Case_;
 
 /**
  * Class IgnoreArrayKeyProcessorTest
@@ -105,4 +106,28 @@ public function testProcessWillNotRemoveOtherValues(): void
 
         $this->assertCount(1, $nodeOccurrenceList->getOccurrences());
     }
+
+    /**
+     * @return void
+     */
+    public function testProcessWithoutArrayItemWillNotGetRemoved(): void
+    {
+        $node = new String_('test');
+        $node->setAttribute('parent', new Case_(
+            new String_('test')
+        ));
+
+        $occurrence = $this->createOccurrence($node);
+
+        $nodeOccurrenceList = new OccurrenceList();
+        $nodeOccurrenceList->addOccurrence($occurrence);
+        $this->processor->setNodeOccurrenceList($nodeOccurrenceList);
+
+        $this->assertCount(1, $nodeOccurrenceList->getOccurrences());
+        $this->assertEmpty($occurrence->getAffectedByProcessors());
+
+        $this->processor->process($occurrence);
+
+        $this->assertCount(1, $nodeOccurrenceList->getOccurrences());
+    }
 }
diff --git a/tests/Unit/Node/Representation/NullableTypeTest.php b/tests/Unit/Node/Representation/NullableTypeTest.php
@@ -0,0 +1,47 @@
+<?php
+declare(strict_types = 1);
+
+namespace Isfett\PhpAnalyzer\Tests\Unit\Node\Representation;
+
+use Isfett\PhpAnalyzer\Node\Representation\NullableType;
+use PhpParser\Node;
+
+/**
+ * Class NullableTypeTest
+ */
+class NullableTypeTest extends AbstractNodeRepresentationTest
+{
+    /**
+     * @return void
+     */
+    protected function setUp(): void
+    {
+        parent::setUp();
+    }
+
+    /**
+     * @return void
+     */
+    public function testGetRepresentation(): void
+    {
+        $node = new Node\NullableType(
+            new Node\Param(
+                new Node\Expr\Variable('x'),
+                null,
+                new Node\Identifier('int'),
+                false,
+                false,
+                $this->getNodeAttributes()
+            ),
+            $this->getNodeAttributes()
+        );
+
+        $this->nodeRepresentationService
+            ->method('representationForNode')
+            ->willReturn('int');
+
+        $representation = new NullableType($this->nodeRepresentationService, $node);
+
+        $this->assertSame('?int', $representation->representation());
+    }
+}
diff --git a/tests/Unit/Node/Representation/ParamTest.php b/tests/Unit/Node/Representation/ParamTest.php
@@ -31,24 +31,25 @@ public function argProvider(): array
             'returnType' => ['int $value', $this->createVariableNode('value'), $this->createIdentifierNode('int'), null, false, false],
             'default' => ['$value = $x', $this->createVariableNode('value'), null, $this->createVariableNode('x'), false, false],
             'default returnType' => ['int $value = $x', $this->createVariableNode('value'), $this->createIdentifierNode('int'), $this->createVariableNode('x'), false, false],
+            'default nullable returnType' => ['?int $value = $x', $this->createVariableNode('value'), $this->createIdentifierNode('int', true), $this->createVariableNode('x'), false, false],
             'variadic' => ['...$value', $this->createVariableNode('value'), null, null, false, true],
             'variadic returnType' => ['int ...$value', $this->createVariableNode('value'), $this->createIdentifierNode('int'), null, false, true],
         ];
     }
 
     /**
-     * @param string               $expectedOutput
-     * @param Node\Expr\Variable   $value
-     * @param Node\Identifier|null $type
-     * @param Node\Expr|null       $default
-     * @param bool                 $byRef
-     * @param bool                 $variadic
+     * @param string             $expectedOutput
+     * @param Node\Expr\Variable $value
+     * @param Node|null          $type
+     * @param Node\Expr|null     $default
+     * @param bool               $byRef
+     * @param bool               $variadic
      *
      * @return void
      *
      * @dataProvider argProvider
      */
-    public function testGetRepresentation(string $expectedOutput, Node\Expr\Variable $value, ?Node\Identifier $type, ?Node\Expr $default, bool $byRef, bool $variadic): void
+    public function testGetRepresentation(string $expectedOutput, Node\Expr\Variable $value, ?Node $type, ?Node\Expr $default, bool $byRef, bool $variadic): void
     {
         $node = new Node\Param(
             $value,
@@ -59,16 +60,28 @@ public function testGetRepresentation(string $expectedOutput, Node\Expr\Variable
             $this->getNodeAttributes()
         );
 
+        $returnValues = [];
+
+        if (null !== $type) {
+            if ($type instanceof Node\NullableType) {
+                $typeRepresentation = '?' . $type->type->name;
+            } else {
+                $typeRepresentation = $type->name;
+            }
+
+            $returnValues[] = $typeRepresentation;
+        }
+
         if (null !== $default) {
-            $this->nodeRepresentationService
-                ->method('representationForNode')
-                ->willReturn('$x', '$value');
-        } else {
-            $this->nodeRepresentationService
-                ->method('representationForNode')
-                ->willReturn('$value');
+            $returnValues[] = '$x';
         }
 
+        $returnValues[] = '$value';
+
+        $this->nodeRepresentationService
+            ->method('representationForNode')
+            ->willReturnOnConsecutiveCalls(...$returnValues);
+
         $representation = new Param($this->nodeRepresentationService, $node);
 
         $this->assertSame($expectedOutput, $representation->representation());

diff --git a/tests/data/magic_number_detector_integrationtest.php b/tests/data/magic_number_detector_integrationtest.php
@@ -10,7 +10,7 @@ class MagicNumberTestClass
 
     private $variable = 6;
 
-    public function test($input = 4) {
+    public function test(?int $input = 4) {
         if ($input > 2) {
             return 15;
         }