Skip to content

Commit

Permalink
Add ReturnTypeFromStrictBoolReturnExprRector support for if/else retu…
Browse files Browse the repository at this point in the history
…rns (#5384)

* Add fixture

* Add ReturnTypeFromStrictBoolReturnExprRector support for if/else returns
  • Loading branch information
TomasVotruba committed Dec 23, 2023
1 parent 16f8d19 commit 620321b
Show file tree
Hide file tree
Showing 16 changed files with 194 additions and 70 deletions.
1 change: 0 additions & 1 deletion phpstan.neon
Expand Up @@ -79,7 +79,6 @@ parameters:
- "#^Cognitive complexity for \"Rector\\\\Php70\\\\EregToPcreTransformer\\:\\:(.*?)\" is (.*?), keep it under 11$#"
- '#Cognitive complexity for "Rector\\Core\\PhpParser\\Node\\Value\\ValueResolver\:\:getValue\(\)" is \d+, keep it under 11#'
- '#Cognitive complexity for "Rector\\DeadCode\\NodeManipulator\\LivingCodeManipulator\:\:keepLivingCodeFromExpr\(\)" is \d+, keep it under 11#'
- '#Cognitive complexity for "Rector\\DeadCode\\PhpDoc\\DeadReturnTagValueNodeAnalyzer\:\:isDead\(\)" is 13, keep it under 11#'

# is nested expr
-
Expand Down
@@ -0,0 +1,35 @@
<?php

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

final class IfElseBools
{
public function run($value)
{
if ($value) {
return true;
} else {
return false;
}
}
}

?>
-----
<?php

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

final class IfElseBools
{
public function run($value): bool
{
if ($value) {
return true;
} else {
return false;
}
}
}

?>
@@ -0,0 +1,39 @@
<?php

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

final class PreContentIfElseBools
{
public function run($value)
{
$value += 100;

if ($value) {
return true;
} else {
return false;
}
}
}

?>
-----
<?php

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

final class PreContentIfElseBools
{
public function run($value): bool
{
$value += 100;

if ($value) {
return true;
} else {
return false;
}
}
}

?>
@@ -0,0 +1,35 @@
<?php

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

final class IfElseBools
{
public function run($value)
{
if ($value) {
return true;
} else {
return false;
}
}
}

?>
-----
<?php

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

final class IfElseBools
{
public function run($value): bool
{
if ($value) {
return true;
} else {
return false;
}
}
}

?>

This file was deleted.

This file was deleted.

6 changes: 4 additions & 2 deletions rules/Arguments/NodeAnalyzer/ArgumentAddingScope.php
Expand Up @@ -37,8 +37,10 @@ public function __construct(
) {
}

public function isInCorrectScope(MethodCall | StaticCall $expr, ArgumentAdder|ArgumentAdderWithoutDefaultValue $argumentAdder): bool
{
public function isInCorrectScope(
MethodCall | StaticCall $expr,
ArgumentAdder|ArgumentAdderWithoutDefaultValue $argumentAdder
): bool {
if ($argumentAdder->getScope() === null) {
return true;
}
Expand Down
23 changes: 17 additions & 6 deletions rules/Php55/Rector/FuncCall/GetCalledClassToSelfClassRector.php
Expand Up @@ -8,8 +8,10 @@
use PhpParser\Node\Expr\FuncCall;
use PHPStan\Analyser\Scope;
use Rector\Core\Enum\ObjectReference;
use Rector\Core\Rector\AbstractScopeAwareRector;
use Rector\Core\Rector\AbstractRector;
use Rector\Core\Reflection\ClassModifierChecker;
use Rector\Core\ValueObject\PhpVersionFeature;
use Rector\NodeTypeResolver\Node\AttributeKey;
use Rector\VersionBonding\Contract\MinPhpVersionInterface;
use Symplify\RuleDocGenerator\ValueObject\CodeSample\CodeSample;
use Symplify\RuleDocGenerator\ValueObject\RuleDefinition;
Expand All @@ -19,8 +21,13 @@
* @changelog https://3v4l.org/GU9dP
* @see \Rector\Tests\Php55\Rector\FuncCall\GetCalledClassToSelfClassRector\GetCalledClassToSelfClassRectorTest
*/
final class GetCalledClassToSelfClassRector extends AbstractScopeAwareRector implements MinPhpVersionInterface
final class GetCalledClassToSelfClassRector extends AbstractRector implements MinPhpVersionInterface
{
public function __construct(
private readonly ClassModifierChecker $classModifierChecker
) {
}

public function getRuleDefinition(): RuleDefinition
{
return new RuleDefinition('Change get_called_class() to self::class on final class', [
Expand Down Expand Up @@ -59,22 +66,26 @@ public function getNodeTypes(): array
/**
* @param FuncCall $node
*/
public function refactorWithScope(Node $node, Scope $scope): ?Node
public function refactor(Node $node): ?Node
{
if (! $this->isName($node, 'get_called_class')) {
return null;
}

$scope = $node->getAttribute(AttributeKey::SCOPE);
if (! $scope instanceof Scope) {
return null;
}

if (! $scope->isInClass()) {
return null;
}

$classReflection = $scope->getClassReflection();
if ($classReflection->isFinalByKeyword()) {
if ($this->classModifierChecker->isInsideFinalClass($node)) {
return $this->nodeFactory->createClassConstFetch(ObjectReference::SELF, 'class');
}

if ($classReflection->isAnonymous()) {
if ($scope->isInAnonymousFunction()) {
return $this->nodeFactory->createClassConstFetch(ObjectReference::SELF, 'class');
}

Expand Down
Expand Up @@ -7,7 +7,9 @@
use PhpParser\Node\Expr\Closure;
use PhpParser\Node\Expr\Yield_;
use PhpParser\Node\Stmt\ClassMethod;
use PhpParser\Node\Stmt\Else_;
use PhpParser\Node\Stmt\Function_;
use PhpParser\Node\Stmt\If_;
use PhpParser\Node\Stmt\Return_;
use Rector\Core\PhpParser\Node\BetterNodeFinder;
use Rector\TypeDeclaration\NodeAnalyzer\ReturnAnalyzer;
Expand All @@ -21,34 +23,77 @@ public function __construct(
}

/**
* @return Return_[]|null
* @return Return_[]
*/
public function matchAlwaysStrictReturns(ClassMethod|Closure|Function_ $functionLike): ?array
public function matchAlwaysStrictReturns(ClassMethod|Closure|Function_ $functionLike): array
{
if ($functionLike->stmts === null) {
return null;
return [];
}

if ($this->betterNodeFinder->hasInstancesOfInFunctionLikeScoped($functionLike, [Yield_::class])) {
return null;
return [];
}

/** @var Return_[] $returns */
$returns = $this->betterNodeFinder->findInstancesOfInFunctionLikeScoped($functionLike, Return_::class);
if ($returns === []) {
return null;
return [];
}

// is one statement depth 3?
if (! $this->returnAnalyzer->areExclusiveExprReturns($returns)) {
return null;
return [];
}

// is one in ifOrElse, other in else?
if ($this->hasOnlyStmtWithIfAndElse($functionLike)) {
return $returns;
}

// has root return?
if (! $this->returnAnalyzer->hasClassMethodRootReturn($functionLike)) {
return null;
return [];
}

return $returns;
}

private function hasFirstLevelReturn(If_|Else_ $ifOrElse): bool
{
foreach ($ifOrElse->stmts as $stmt) {
if ($stmt instanceof Return_) {
return true;
}
}

return false;
}

private function hasOnlyStmtWithIfAndElse(ClassMethod|Function_|Closure $functionLike): bool
{
foreach ((array) $functionLike->stmts as $functionLikeStmt) {
if (! $functionLikeStmt instanceof If_) {
continue;
}

$if = $functionLikeStmt;

if ($if->elseifs !== []) {
return false;
}

if (! $if->else instanceof Else_) {
return false;
}

if (! $this->hasFirstLevelReturn($if)) {
return false;
}

return $this->hasFirstLevelReturn($if->else);
}

return false;
}
}
Expand Up @@ -21,7 +21,7 @@ public function __construct(
public function hasAlwaysStrictBoolReturn(ClassMethod|Closure|Function_ $functionLike): bool
{
$returns = $this->alwaysStrictReturnAnalyzer->matchAlwaysStrictReturns($functionLike);
if ($returns === null) {
if ($returns === []) {
return false;
}

Expand Down
Expand Up @@ -8,7 +8,6 @@
use PhpParser\Node\Expr\Closure;
use PhpParser\Node\Stmt\ClassMethod;
use PhpParser\Node\Stmt\Function_;
use PHPStan\Analyser\Scope;
use PHPStan\Type\Type;
use Rector\NodeTypeResolver\PHPStan\Type\TypeFactory;
use Rector\TypeDeclaration\TypeAnalyzer\AlwaysStrictScalarExprAnalyzer;
Expand All @@ -22,10 +21,10 @@ public function __construct(
) {
}

public function matchAlwaysScalarReturnType(ClassMethod|Closure|Function_ $functionLike, Scope $scope): ?Type
public function matchAlwaysScalarReturnType(ClassMethod|Closure|Function_ $functionLike): ?Type
{
$returns = $this->alwaysStrictReturnAnalyzer->matchAlwaysStrictReturns($functionLike);
if ($returns === null) {
if ($returns === []) {
return null;
}

Expand Down
Expand Up @@ -13,9 +13,9 @@
use PhpParser\Node\Stmt\Return_;
use PhpParser\Node\Stmt\Throw_;
use PHPStan\Analyser\Scope;
use PHPStan\Reflection\ClassReflection;
use Rector\Core\PhpParser\Node\BetterNodeFinder;
use Rector\Core\Rector\AbstractScopeAwareRector;
use Rector\Core\Reflection\ClassModifierChecker;
use Rector\Core\ValueObject\PhpVersionFeature;
use Rector\NodeNestingScope\ValueObject\ControlStructure;
use Rector\TypeDeclaration\NodeAnalyzer\NeverFuncCallAnalyzer;
Expand All @@ -35,6 +35,7 @@ public function __construct(
private readonly ClassMethodReturnTypeOverrideGuard $classMethodReturnTypeOverrideGuard,
private readonly BetterNodeFinder $betterNodeFinder,
private readonly NeverFuncCallAnalyzer $neverFuncCallAnalyzer,
private readonly ClassModifierChecker $classModifierChecker,
) {
}

Expand Down Expand Up @@ -119,11 +120,7 @@ private function shouldSkip(ClassMethod | Function_ | Closure $node, Scope $scop
}

// skip as most likely intentional
$classReflection = $scope->getClassReflection();
if ($classReflection instanceof ClassReflection && ! $classReflection->isFinalByKeyword() && $this->isName(
$node->returnType,
'void'
)) {
if (! $this->classModifierChecker->isInsideFinalClass($node) && $this->isName($node->returnType, 'void')) {
return true;
}

Expand Down
Expand Up @@ -70,7 +70,7 @@ public function getNodeTypes(): array
*/
public function refactorWithScope(Node $node, Scope $scope): ?Node
{
if ($node->returnType !== null) {
if ($node->returnType instanceof Node) {
return null;
}

Expand Down

0 comments on commit 620321b

Please sign in to comment.