Update property generation for typed properties

lib/Adapter/TolerantParser/Updater/ClassLikeUpdater.php

@@ -3,13 +3,14 @@
 namespace Phpactor\CodeBuilder\Adapter\TolerantParser\Updater;
 
 use Microsoft\PhpParser\Node;
+use Microsoft\PhpParser\Node\ClassConstDeclaration;
 use Microsoft\PhpParser\Node\Expression\AssignmentExpression;
 use Microsoft\PhpParser\Node\Expression\Variable;
 use Microsoft\PhpParser\Node\MethodDeclaration;
 use Microsoft\PhpParser\Node\PropertyDeclaration;
+use Microsoft\PhpParser\Node\TraitUseClause;
 use Phpactor\CodeBuilder\Adapter\TolerantParser\Edits;
 use Phpactor\CodeBuilder\Domain\Prototype\ClassLikePrototype;
-use Phpactor\CodeBuilder\Domain\Prototype\Type;
 use Phpactor\CodeBuilder\Domain\Renderer;
 
 abstract class ClassLikeUpdater
@@ -54,41 +55,48 @@ protected function updateProperties(Edits $edits, ClassLikePrototype $classProto
             return;
         }
 
-        $lastProperty = $classMembers->openBrace;
+        $previousMember = $classMembers->openBrace;
         $memberDeclarations = $this->memberDeclarations($classMembers);
 
-        $nextMember = null;
+        $nextMember = reset($memberDeclarations) ?: null;
         $existingPropertyNames = [];
 
         foreach ($memberDeclarations as $memberNode) {
-            if (null === $nextMember) {
-                $nextMember = $memberNode;
+            // Property goes after traits and constants
+            if ($memberNode instanceof TraitUseClause ||
+                $memberNode instanceof ClassConstDeclaration
+            ) {
+                $previousMember = $memberNode;
             }
 
             if ($memberNode instanceof PropertyDeclaration) {
                 foreach ($memberNode->propertyElements->getElements() as $property) {
                     $existingPropertyNames[] = $this->resolvePropertyName($property);
                 }
-                $lastProperty = $memberNode;
+                $previousMember = $memberNode;
                 $nextMember = next($memberDeclarations) ?: $nextMember;
                 prev($memberDeclarations);
             }
         }
 
+        // If the previous member is neither the open brace of class nor a property
+        // Then we must add a blank line before the new properties
+        if ($previousMember !== $classMembers->openBrace
+            && !($previousMember instanceof PropertyDeclaration)
+        ) {
+            $edits->after($previousMember, PHP_EOL);
+        }
+
         foreach ($classPrototype->properties()->notIn($existingPropertyNames) as $property) {
-            // if property type exists then the last property has a docblock - add a line break
-            if ($lastProperty instanceof PropertyDeclaration && $property->type() != Type::none()) {
-                $edits->after($lastProperty, PHP_EOL);
-            }
+            $renderedProperty = $this->renderer->render($property);
 
-            $edits->after(
-                $lastProperty,
-                PHP_EOL . $edits->indent($this->renderer->render($property), 1)
-            );
+            $edits->after($previousMember, PHP_EOL . $edits->indent($renderedProperty, 1));
+        }
 
-            if ($classPrototype->properties()->isLast($property) && $nextMember instanceof MethodDeclaration) {
-                $edits->after($lastProperty, PHP_EOL);
-            }
+        if ($previousMember === $classMembers->openBrace &&
+            $nextMember instanceof MethodDeclaration
+        ) {
+            $edits->after($previousMember, PHP_EOL);
         }
     }
 }

lib/Adapter/WorseReflection/WorseBuilderFactory.php

@@ -87,7 +87,9 @@ private function buildProperty(ClassLikeBuilder $classBuilder, ReflectionPropert
         $type = $property->inferredTypes()->best();
         if ($type->isDefined()) {
             $this->resolveClassMemberType($classBuilder, $property->class()->name(), $type);
-            $propertyBuilder->type((string) $type->short());
+            $imports = $property->scope()->nameImports();
+            $typeName = $this->resolveTypeNameFromNameImports($type, $imports);
+            $propertyBuilder->type($typeName);
         }
     }
 
@@ -99,7 +101,9 @@ private function buildMethod(ClassLikeBuilder $classBuilder, ReflectionMethod $m
         if ($method->returnType()->isDefined()) {
             $type = $method->returnType();
             $this->resolveClassMemberType($classBuilder, $method->class()->name(), $type);
-            $methodBuilder->returnType($type->short());
+            $imports = $method->scope()->nameImports();
+            $typeName = $this->resolveTypeNameFromNameImports($type, $imports);
+            $methodBuilder->returnType($typeName);
         }
 
         if ($method->isStatic()) {
@@ -150,6 +154,11 @@ private function resolveTypeNameFromNameImports(Type $type, NameImports $imports
                 $typeName = $alias;
             }
         }
+
+        if ($type->isNullable()) {
+            return "?$typeName";
+        }
+
         return $typeName;
     }
 }

lib/Util/TextFormat.php

@@ -18,6 +18,10 @@ public function indent(string $string, int $level = 0)
     {
         $lines = explode(PHP_EOL, $string);
         $lines = array_map(function ($line) use ($level) {
+            if (empty($line)) {
+                return $line;
+            }
+
             return str_repeat($this->indentation, $level) . $line;
         }, $lines);
 

templates/Property.php.twig

@@ -1,6 +1,7 @@
 {% if prototype.type.notNone %}
+
 /**
- * @var {{ prototype.type }}
+ * @var {{ prototype.type|trim('?', 'left') }}{{ prototype.type.nullable ? '|null' }}
  */
 {% endif %}
 {{ prototype.visibility }} ${{ prototype.name}}{% if prototype.defaultValue.notNone %} = {{ prototype.defaultValue.export }}{% endif %};

tests/Adapter/GeneratorTestCase.php

@@ -359,6 +359,7 @@ class Kitten implements Feline, Infant
                     Type::fromString('PlaneCollection')
                 ),
                 <<<'EOT'
+
 /**
  * @var PlaneCollection
  */
@@ -487,6 +488,7 @@ public function sleep();
 
 trait Oryctolagus
 {
+
     /**
      * @var bool
      */
@@ -499,6 +501,7 @@ public function burrow(Depth $depth = 'deep')
 
 class Rabbits extends Leporidae implements Animal
 {
+
     /**
      * @var int
      */
@@ -584,6 +587,7 @@ class Rabbits implements Animal
     const LEGS = 4;
     const SKIN = 'soft';
 
+
     /**
      * @var int
      */

tests/Adapter/UpdaterTestCase.php

@@ -7,6 +7,7 @@
 use Phpactor\CodeBuilder\Domain\Code;
 use Phpactor\CodeBuilder\Domain\Builder\SourceCodeBuilder;
 use Phpactor\CodeBuilder\Domain\Prototype\SourceCode;
+use Phpactor\WorseReflection\Core\Type;
 
 abstract class UpdaterTestCase extends TestCase
 {
@@ -883,7 +884,7 @@ class Aardvark
 EOT
             ];
 
-        yield 'It adds a documented properties' => [
+        yield 'It adds a typed property' => [
                 <<<'EOT'
 class Aardvark
 {
@@ -905,6 +906,33 @@ class Aardvark
      */
     public $propertyOne;
 }
+EOT
+            ];
+
+        yield 'It adds a nullable typed property' => [
+                <<<'EOT'
+class Aardvark
+{
+    public $eyes
+}
+EOT
+                , SourceCodeBuilder::create()
+                    ->class('Aardvark')
+                        ->property('propertyOne')->type(
+                            Type::fromString('Hello')->asNullable()
+                        )->end()
+                    ->end()
+                    ->build(),
+                <<<'EOT'
+class Aardvark
+{
+    public $eyes
+
+    /**
+     * @var Hello|null
+     */
+    public $propertyOne;
+}
 EOT
             ];
 
@@ -925,6 +953,7 @@ public function crawl()
                 <<<'EOT'
 class Aardvark
 {
+
     /**
      * @var Hello
      */
@@ -1092,6 +1121,7 @@ public function crawl()
                 <<<'EOT'
 trait Aardvark
 {
+
     /**
      * @var Hello
      */
@@ -1267,6 +1297,35 @@ public function methodOne(Barf $sniff)
     {
     }
 }
+EOT
+            ];
+
+        yield 'It adds nullable typed parameters' => [
+                <<<'EOT'
+class Aardvark
+{
+    public function methodOne()
+    {
+    }
+}
+EOT
+                , SourceCodeBuilder::create()
+                    ->class('Aardvark')
+                        ->method('methodOne')
+                            ->parameter('sniff')->type(
+                                Type::fromString('Barf')->asNullable()
+                            )
+                            ->end()
+                        ->end()
+                    ->end()
+                    ->build(),
+                <<<'EOT'
+class Aardvark
+{
+    public function methodOne(?Barf $sniff)
+    {
+    }
+}
 EOT
             ];
 

tests/Adapter/WorseReflection/WorseBuilderFactoryTest.php

@@ -72,6 +72,16 @@ public function testClassWithPropertyImportedType()
         $this->assertEquals('Bar\Foobar', (string) $source->useStatements()->first());
     }
 
+    public function testClassWithPropertyImportedNullableType()
+    {
+        $source = $this->build('<?php namespace Test; use Bar\Foobar; class Foobar { private ?Foobar $foo = "foobar"; }');
+        $this->assertEquals(
+            '?Foobar',
+            (string) $source->classes()->first()->properties()->first()->type()
+        );
+        $this->assertEquals('Bar\Foobar', (string) $source->useStatements()->first());
+    }
+
     public function testSimpleTrait()
     {
         $source = $this->build('<?php trait Foobar {}');
@@ -119,6 +129,19 @@ public function testMethodWithReturnType()
         $this->assertEquals('string', $source->classes()->first()->methods()->first()->returnType());
     }
 
+    public function testMethodWithNullableReturnType()
+    {
+        $source = $this->build('<?php class Foobar { public function method(): ?string {} }');
+        $this->assertEquals('?string', $source->classes()->first()->methods()->first()->returnType());
+    }
+
+    public function testMethodWithImportedNullableReturnType()
+    {
+        $source = $this->build('<?php namespace Test; use Bar\Baz; class Foobar { public function method(): ?Baz {} }');
+        $this->assertEquals('?Baz', $source->classes()->first()->methods()->first()->returnType());
+        $this->assertEquals('Bar\Baz', (string) $source->useStatements()->first());
+    }
+
     public function testMethodProtected()
     {
         $source = $this->build('<?php class Foobar { protected function method() {} }');
@@ -143,6 +166,12 @@ public function testMethodWithTypedParameter()
         $this->assertEquals('string', (string) $source->classes()->first()->methods()->first()->parameters()->first()->type());
     }
 
+    public function testMethodWithNullableTypedParameter()
+    {
+        $source = $this->build('<?php class Foobar { public function method(?string $param) {} }');
+        $this->assertEquals('?string', (string) $source->classes()->first()->methods()->first()->parameters()->first()->type());
+    }
+
     public function testMethodWithAliasedParameter()
     {
         $source = $this->build('<?php use Foobar as Barfoo; class Foobar { public function method(Barfoo $param) {} }');

tests/Unit/Util/TextFormatTest.php

@@ -0,0 +1,43 @@
+<?php
+
+namespace Phpactor\CodeBuilder\Tests\Unit\Util;
+
+use PHPUnit\Framework\TestCase;
+use Phpactor\CodeBuilder\Util\TextFormat;
+
+class TextFormatTest extends TestCase
+{
+    public function testThatItDoesNotIndentBlankLines()
+    {
+        $input = <<<EOT
+Non blank line
+
+Non blank line
+EOT;
+        $expectedOutput = <<<EOT
+    Non blank line
+
+    Non blank line
+EOT;
+
+        $formater = new TextFormat('    ');
+        $this->assertSame($expectedOutput, $formater->indent($input, 1));
+    }
+
+    public function testThatItIndentOnMoreThanOneLevel()
+    {
+        $input = <<<EOT
+Non blank line
+
+Non blank line
+EOT;
+        $expectedOutput = <<<EOT
+        Non blank line
+
+        Non blank line
+EOT;
+
+        $formater = new TextFormat('    ');
+        $this->assertSame($expectedOutput, $formater->indent($input, 2));
+    }
+}