Skip to content

Commit

Permalink
[php74] Skip conditinal assign in RestoreDefaultNullToNullableTypePro…
Browse files Browse the repository at this point in the history
…pertyRector as most likely desired to assign or fail
  • Loading branch information
TomasVotruba committed Nov 13, 2024
1 parent 7bb5384 commit 541753f
Show file tree
Hide file tree
Showing 6 changed files with 64 additions and 37 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,18 @@

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

class SkipAssignedInConstruct
final class SkipAssignedInConstruct
{
public ?string $name;

public function __construct(?string $name = null)
public function __construct(array $items)
{
$this->name = $name;
foreach ($items as $item) {
$this->name = $item;
}

if (! isset($this->item)) {
throw new \InvalidArgumentException();
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ public function refactor(Node $node): ?Node
$hasChanged = false;

foreach ($node->getProperties() as $property) {
if ($this->shouldSkip($property, $node)) {
if ($this->shouldSkipProperty($property, $node)) {
continue;
}

Expand All @@ -93,7 +93,7 @@ public function provideMinPhpVersion(): int
return PhpVersionFeature::TYPED_PROPERTIES;
}

private function shouldSkip(Property $property, Class_ $class): bool
private function shouldSkipProperty(Property $property, Class_ $class): bool
{
if ($property->type === null) {
return true;
Expand All @@ -103,8 +103,7 @@ private function shouldSkip(Property $property, Class_ $class): bool
return true;
}

$onlyProperty = $property->props[0];
if ($onlyProperty->default instanceof Expr) {
if ($property->props[0]->default instanceof Expr) {
return true;
}

Expand All @@ -118,7 +117,7 @@ private function shouldSkip(Property $property, Class_ $class): bool

// is variable assigned in constructor
$propertyName = $this->getName($property);
return $this->constructorAssignDetector->isPropertyAssigned($class, $propertyName);
return $this->constructorAssignDetector->isPropertyAssignedConditionally($class, $propertyName);
}

private function isReadonlyProperty(Property $property): bool
Expand Down
5 changes: 1 addition & 4 deletions rules/Php80/Rector/FunctionLike/MixedTypeRector.php
Original file line number Diff line number Diff line change
Expand Up @@ -94,11 +94,8 @@ public function refactor(Node $node): ?Node

$this->refactorParamTypes($node, $phpDocInfo);
$hasChanged = $this->paramTagRemover->removeParamTagsIfUseless($phpDocInfo, $node, new MixedType());
if ($this->hasChanged) {
return $node;
}

if ($hasChanged) {
if ($this->hasChanged || $hasChanged) {
return $node;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
use PhpParser\Node\Expr\PropertyFetch;
use PhpParser\Node\Identifier;
use PhpParser\Node\Stmt;
use PhpParser\Node\Stmt\Class_;
use PhpParser\Node\Stmt\ClassLike;
use PhpParser\Node\Stmt\ClassMethod;
use PhpParser\Node\Stmt\Else_;
Expand All @@ -19,6 +20,8 @@
use PhpParser\NodeTraverser;
use PHPStan\Type\ObjectType;
use Rector\NodeAnalyzer\PropertyFetchAnalyzer;
use Rector\NodeDecorator\StatementDepthAttributeDecorator;
use Rector\NodeTypeResolver\Node\AttributeKey;
use Rector\NodeTypeResolver\NodeTypeResolver;
use Rector\PhpDocParser\NodeTraverser\SimpleCallableNodeTraverser;
use Rector\PhpParser\Comparing\NodeComparator;
Expand All @@ -28,11 +31,6 @@

final readonly class ConstructorAssignDetector
{
/**
* @var string
*/
private const IS_FIRST_LEVEL_STATEMENT = 'first_level_stmt';

public function __construct(
private NodeTypeResolver $nodeTypeResolver,
private PropertyAssignMatcher $propertyAssignMatcher,
Expand All @@ -43,7 +41,12 @@ public function __construct(
) {
}

public function isPropertyAssigned(ClassLike $classLike, string $propertyName): bool
public function isPropertyAssignedConditionally(Class_ $class, string $propertyName): bool
{
return $this->isPropertyAssigned($class, $propertyName, true);
}

public function isPropertyAssigned(ClassLike $classLike, string $propertyName, bool $allowConditional = false): bool
{
$initializeClassMethods = $this->matchInitializeClassMethod($classLike);
if ($initializeClassMethods === []) {
Expand All @@ -52,12 +55,12 @@ public function isPropertyAssigned(ClassLike $classLike, string $propertyName):

$isAssignedInConstructor = false;

$this->decorateFirstLevelStatementAttribute($initializeClassMethods);
StatementDepthAttributeDecorator::decorateClassMethods($initializeClassMethods);

foreach ($initializeClassMethods as $initializeClassMethod) {
$this->simpleCallableNodeTraverser->traverseNodesWithCallable((array) $initializeClassMethod->stmts, function (
Node $node
) use ($propertyName, &$isAssignedInConstructor): ?int {
) use ($propertyName, &$isAssignedInConstructor, $allowConditional): ?int {
if ($this->isIfElseAssign($node, $propertyName)) {
$isAssignedInConstructor = true;
return NodeTraverser::STOP_TRAVERSAL;
Expand All @@ -77,10 +80,15 @@ public function isPropertyAssigned(ClassLike $classLike, string $propertyName):
return NodeTraverser::STOP_TRAVERSAL;
}

$isFirstLevelStatement = $assign->getAttribute(self::IS_FIRST_LEVEL_STATEMENT);
$isFirstLevelStatement = $assign->getAttribute(AttributeKey::IS_FIRST_LEVEL_STATEMENT);

// cannot be nested
if ($isFirstLevelStatement !== true) {
if ($allowConditional) {
$isAssignedInConstructor = true;
return NodeTraverser::STOP_TRAVERSAL;
}

return null;
}

Expand Down Expand Up @@ -139,22 +147,6 @@ private function matchAssignExprToPropertyName(Node $node, string $propertyName)
return $this->propertyAssignMatcher->matchPropertyAssignExpr($node, $propertyName);
}

/**
* @param ClassMethod[] $classMethods
*/
private function decorateFirstLevelStatementAttribute(array $classMethods): void
{
foreach ($classMethods as $classMethod) {
foreach ((array) $classMethod->stmts as $methodStmt) {
$methodStmt->setAttribute(self::IS_FIRST_LEVEL_STATEMENT, true);

if ($methodStmt instanceof Expression) {
$methodStmt->expr->setAttribute(self::IS_FIRST_LEVEL_STATEMENT, true);
}
}
}
}

/**
* @return ClassMethod[]
*/
Expand Down
28 changes: 28 additions & 0 deletions src/NodeDecorator/StatementDepthAttributeDecorator.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
<?php

declare(strict_types=1);

namespace Rector\NodeDecorator;

use PhpParser\Node\Stmt\ClassMethod;
use PhpParser\Node\Stmt\Expression;
use Rector\NodeTypeResolver\Node\AttributeKey;

final class StatementDepthAttributeDecorator
{
/**
* @param ClassMethod[] $classMethods
*/
public static function decorateClassMethods(array $classMethods): void
{
foreach ($classMethods as $classMethod) {
foreach ((array) $classMethod->stmts as $methodStmt) {
$methodStmt->setAttribute(AttributeKey::IS_FIRST_LEVEL_STATEMENT, true);

if ($methodStmt instanceof Expression) {
$methodStmt->expr->setAttribute(AttributeKey::IS_FIRST_LEVEL_STATEMENT, true);
}
}
}
}
}
5 changes: 5 additions & 0 deletions src/NodeTypeResolver/Node/AttributeKey.php
Original file line number Diff line number Diff line change
Expand Up @@ -285,4 +285,9 @@ final class AttributeKey
* @var string
*/
public const ATTRIBUTE_COMMENT = 'attribute_comment';

/**
* @var string
*/
public const IS_FIRST_LEVEL_STATEMENT = 'first_level_stmt';
}

0 comments on commit 541753f

Please sign in to comment.