Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Skip null type in ReturnTypeFromStrictScalarReturnExprRector as often not desired #5462

Merged
merged 1 commit into from
Jan 13, 2024
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
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
<?php

namespace Rector\Tests\TypeDeclaration\Rector\ClassMethod\ReturnTypeFromStrictScalarReturnExprRector\Fixture;

class SkipNullAsTemporary
{
public function getValue()
{
return null;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ public function refactor(Node $node): array|If_|null
// only default → basically unwrap
if (! $onlyCase->cond instanceof Expr) {
// remove default clause because it cause syntax error
return array_filter($onlyCase->stmts, static fn(Stmt $stmt): bool => ! $stmt instanceof Break_);
return array_filter($onlyCase->stmts, static fn (Stmt $stmt): bool => ! $stmt instanceof Break_);
}

$if = new If_(new Identical($node->cond, $onlyCase->cond));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
use PhpParser\Node\Stmt\Function_;
use PhpParser\Node\UnionType;
use PHPStan\Analyser\Scope;
use PHPStan\Type\NullType;
use PHPStan\Type\Type;
use Rector\Contract\Rector\ConfigurableRectorInterface;
use Rector\PHPStanStaticTypeMapper\Enum\TypeKind;
Expand Down Expand Up @@ -117,6 +118,11 @@ public function refactorWithScope(Node $node, Scope $scope): ?Node
return null;
}

// skip null as often placeholder value and not an only type
if ($scalarReturnType instanceof NullType) {
Copy link
Member

Choose a reason for hiding this comment

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

this should be fine on Function_, Closure, and when class is final, so that seems need to be checked

I will add new PR for that.

Copy link
Member

Choose a reason for hiding this comment

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

Oh, I see what you mean about placeholder, seems ok then

return null;
}

$returnTypeNode = $this->staticTypeMapper->mapPHPStanTypeToPhpParserNode($scalarReturnType, TypeKind::RETURN);
if (! $returnTypeNode instanceof Node) {
return null;
Expand Down