Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 31 additions & 1 deletion packages/NodeTypeResolver/src/StaticTypeMapper.php
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
use PHPStan\Type\ClosureType;
use PHPStan\Type\ConstantType;
use PHPStan\Type\FloatType;
use PHPStan\Type\Generic\GenericObjectType;
use PHPStan\Type\IntegerType;
use PHPStan\Type\IntersectionType;
use PHPStan\Type\IterableType;
Expand Down Expand Up @@ -554,6 +555,13 @@ public function mapStringToPhpParserNode(string $type): ?Node
throw new NotImplementedException(sprintf('%s for "%s"', __METHOD__, $type));
}

public function mapPHPStanPhpDocTypeNodeToPhpDocString(TypeNode $typeNode, Node $node): string
{
$phpStanType = $this->mapPHPStanPhpDocTypeNodeToPHPStanType($typeNode, $node);

return $this->mapPHPStanTypeToDocString($phpStanType);
}

public function mapPHPStanPhpDocTypeNodeToPHPStanType(TypeNode $typeNode, Node $node): Type
{
if ($typeNode instanceof IdentifierTypeNode) {
Expand Down Expand Up @@ -638,7 +646,11 @@ public function mapPHPStanPhpDocTypeNodeToPHPStanType(TypeNode $typeNode, Node $
if ($typeNode instanceof GenericTypeNode) {
if ($typeNode->type instanceof IdentifierTypeNode) {
$typeName = $typeNode->type->name;
if (in_array($typeName, ['array', 'iterable'], true)) {

// remove extra prefix
$typeName = ltrim($typeName, '\\');

if (in_array($typeName, ['array', 'iterable', 'Traversable'], true)) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What exactly is this condition for? There are more types like these - Iterator, Generator, ext-ds types, Doctrine collections, some SPL types... trying to white-list them for some special handling doesn't make much sense to me.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These all need to be listed there then. For phpdoc-parser it's just a string.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is no way to whitelist everything. What is the purpose of this?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only those that have type in object fo PHPStan.

The mapPHPStanPhpDocTypeNodeToPHPStanType() method converts doc type to phpstan type

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you're able to make it simpler and more robust, that would be great

Copy link
Copy Markdown
Contributor

@enumag enumag Dec 30, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only those that have type in object fo PHPStan.

Can you rephrase that? I don't understand...

Would something like === array || === iterable || is_subclass_of(Traversable) work?

Copy link
Copy Markdown
Member Author

@TomasVotruba TomasVotruba Dec 30, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you rephrase that? I don't understand...

Sure, this is phpdoc-paser doc type:

@var int

To use in the Rector/PHPStan, we need to convert it to PHPStan type:

$integerType = new IntegerType();

That's all :)

Would something like === array || === iterable || is_subclass_of(Traversable) work?

Hard to tell, testing needs to be done, as this is 3rd party code we convert

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's address in new PR

$genericTypes = [];
foreach ($typeNode->genericTypes as $genericTypeNode) {
$genericTypes[] = $this->mapPHPStanPhpDocTypeNodeToPHPStanType($genericTypeNode, $node);
Expand All @@ -650,9 +662,27 @@ public function mapPHPStanPhpDocTypeNodeToPHPStanType(TypeNode $typeNode, Node $
return new ArrayType(new MixedType(), $genericType);
}

if ($typeName === 'Traversable') {
return new ObjectType('Traversable');
}

return new IterableType(new MixedType(), $genericType);
}
}

$mainType = $this->mapPHPStanPhpDocTypeNodeToPHPStanType($typeNode->type, $node);
if ($mainType instanceof TypeWithClassName) {
$className = $mainType->getClassName();
} else {
throw new NotImplementedException();
}

$genericTypes = [];
foreach ($typeNode->genericTypes as $genericType) {
$genericTypes[] = $this->mapPHPStanPhpDocTypeNodeToPHPStanType($genericType, $node);
}

return new GenericObjectType($className, $genericTypes);
}

throw new NotImplementedException(__METHOD__ . ' for ' . get_class($typeNode));
Expand Down
59 changes: 56 additions & 3 deletions packages/Php74/src/Rector/Property/TypedPropertyRector.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,9 @@

use PhpParser\Node;
use PhpParser\Node\Stmt\Property;
use PHPStan\PhpDocParser\Ast\PhpDoc\VarTagValueNode;
use PHPStan\PhpDocParser\Ast\Type\ArrayTypeNode;
use PHPStan\PhpDocParser\Ast\Type\GenericTypeNode;
use PHPStan\Type\MixedType;
use Rector\Rector\AbstractRector;
use Rector\RectorDefinition\CodeSample;
Expand Down Expand Up @@ -56,9 +59,6 @@ final class SomeClass
<<<'PHP'
final class SomeClass
{
/**
* @var int
*/
private int count;
}
PHP
Expand Down Expand Up @@ -103,8 +103,61 @@ public function refactor(Node $node): ?Node
return null;
}

$this->removeVarPhpTagValueNodeIfNotComment($node);

$node->type = $propertyTypeNode;

return $node;
}

private function removeVarPhpTagValueNodeIfNotComment(Property $property): void
{
$propertyPhpDocInfo = $this->getPhpDocInfo($property);
// nothing to remove
if ($propertyPhpDocInfo === null) {
return;
}

$varTagValueNode = $propertyPhpDocInfo->getByType(VarTagValueNode::class);
if ($varTagValueNode === null) {
return;
}

// has description? keep it
if ($varTagValueNode->description !== '') {
return;
}

// keep generic types
if ($varTagValueNode->type instanceof GenericTypeNode) {
return;
}

// keep string[] etc.
if ($this->isNonBasicArrayType($property, $varTagValueNode)) {
Comment thread
TomasVotruba marked this conversation as resolved.
return;
}

$propertyPhpDocInfo->removeByType(VarTagValueNode::class);
Comment thread
TomasVotruba marked this conversation as resolved.
$this->docBlockManipulator->updateNodeWithPhpDocInfo($property, $propertyPhpDocInfo);
}

private function isNonBasicArrayType(Property $property, VarTagValueNode $varTagValueNode): bool
{
if (! $this->isArrayTypeNode($varTagValueNode)) {
return false;
}

$varTypeDocString = $this->staticTypeMapper->mapPHPStanPhpDocTypeNodeToPhpDocString(
$varTagValueNode->type,
$property
);

return $varTypeDocString !== 'array';
}

private function isArrayTypeNode(VarTagValueNode $varTagValueNode): bool
{
return $varTagValueNode->type instanceof ArrayTypeNode;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ namespace Rector\Php74\Tests\Rector\Property\TypedPropertyRector\Fixture;
final class BoolProperty
{
/**
* @var bool
* another comment
*/
private bool $isTrue = false;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,6 @@ use Rector\TypeDeclaration\Tests\Rector\Property\TypedPropertyRector\Source\Anot

final class ClassWithClassProperty
{
/**
* @var AnotherClass
*/
private \Rector\TypeDeclaration\Tests\Rector\Property\TypedPropertyRector\Source\AnotherClass $anotherClass;
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
<?php

namespace Rector\Php74\Tests\Rector\Property\TypedPropertyRector\Fixture;

use Rector\Php74\Tests\Rector\Property\TypedPropertyRector\Source\SomeParent;

final class ComplexArray
{
/**
* @var array<int, string>
*/
private $foo;

/**
* @var int[]
*/
private $foos;
}

?>
-----
<?php

namespace Rector\Php74\Tests\Rector\Property\TypedPropertyRector\Fixture;

use Rector\Php74\Tests\Rector\Property\TypedPropertyRector\Source\SomeParent;

final class ComplexArray
{
/**
* @var array<int, string>
*/
private array $foo;

/**
* @var int[]
*/
private array $foos;
}

?>
Original file line number Diff line number Diff line change
Expand Up @@ -68,59 +68,26 @@ namespace Rector\Php74\Tests\Rector\Property\TypedPropertyRector\Fixture;

final class DefaultValues
{
/**
* @var bool
*/
private string $name = 'not_a_bool';

/**
* @var bool
*/
private bool $isItRealName = false;

/**
* @var bool
*/
private ?bool $isItRealNameNull = null;

/**
* @var string
*/
private bool $size = false;

/**
* @var float
*/
private float $a = 42.42;

/**
* @var float
*/
private int $b = 42;

/**
* @var float
*/
private string $c = 'hey';

/**
* @var int
*/
private float $e = 42.42;

/**
* @var int
*/
private int $f = 42;

/**
* @var array
*/
private array $g = [1, 2, 3];

/**
* @var iterable
*/
private array $h = [1, 2, 3];
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,19 +28,10 @@ namespace Rector\Php74\Tests\Rector\Property\TypedPropertyRector\Fixture;

final class DefaultValuesForNullableIterables
{
/**
* @var array
*/
private ?array $items = null;

/**
* @var iterable
*/
private ?iterable $itemsB = null;

/**
* @var array|null
*/
private ?array $nullableItems = null;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,49 +58,22 @@ namespace Rector\Php74\Tests\Rector\Property\TypedPropertyRector\Fixture;

final class MatchTypes
{
/**
* @var bool
*/
private bool $a;

/**
* @var boolean
*/
private bool $b;

/**
* @var int
*/
private int $c;

/**
* @var integer
*/
private int $d;

/**
* @var float
*/
private float $e;

/**
* @var string
*/
private string $f;

/**
* @var object
*/
private object $g;

/**
* @var iterable
*/
private iterable $h;

/**
* @var self
*/
private self $i;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,6 @@ class PropperParent

final class MatchTypesParent extends PropperParent
{
/**
* @var parent
*/
private parent $j;
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
<?php

namespace Rector\Php74\Tests\Rector\Property\TypedPropertyRector\Fixture;

class Promise
{
}

class Foo
{
}

final class NonArrayGenericTypes
{
/**
* @var \Traversable<int, string>
*/
private $foo;

/**
* @var Promise<Foo>
*/
private $bar;
}

?>
-----
<?php

namespace Rector\Php74\Tests\Rector\Property\TypedPropertyRector\Fixture;

class Promise
{
}

class Foo
{
}

final class NonArrayGenericTypes
{
/**
* @var \Traversable<int, string>
*/
private \Traversable $foo;

/**
* @var Promise<Foo>
*/
private \Rector\Php74\Tests\Rector\Property\TypedPropertyRector\Fixture\Promise $bar;
}

?>
Loading