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

[CodeQuality] Handle default value on CallableThisArrayToAnonymousFunctionRector #1900

Merged
merged 6 commits into from
Mar 3, 2022
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,41 @@
<?php

namespace Rector\Tests\CodeQuality\Rector\Array_\CallableThisArrayToAnonymousFunctionRector\Fixture;

use Rector\Tests\CodeQuality\Rector\Array_\CallableThisArrayToAnonymousFunctionRector\Source\SomeClassWithDefaultParamValue;
use Twig\Extension\AbstractExtension;
use Twig\TwigFilter;

class HasDefaultValue extends AbstractExtension
{
public function getFilters()
{
return [
new TwigFilter('test', [SomeClassWithDefaultParamValue::class, "run"]),
];
}
}

?>
-----
<?php

namespace Rector\Tests\CodeQuality\Rector\Array_\CallableThisArrayToAnonymousFunctionRector\Fixture;

use Rector\Tests\CodeQuality\Rector\Array_\CallableThisArrayToAnonymousFunctionRector\Source\SomeClassWithDefaultParamValue;
use Twig\Extension\AbstractExtension;
use Twig\TwigFilter;

class HasDefaultValue extends AbstractExtension
{
public function getFilters()
{
return [
new TwigFilter('test', function ($a, $b = ['test']) {
return (new \Rector\Tests\CodeQuality\Rector\Array_\CallableThisArrayToAnonymousFunctionRector\Source\SomeClassWithDefaultParamValue())->run($a, $b);
}),
];
}
}

?>
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
<?php

namespace Rector\Tests\CodeQuality\Rector\Array_\CallableThisArrayToAnonymousFunctionRector\Fixture;

use Rector\Tests\CodeQuality\Rector\Array_\CallableThisArrayToAnonymousFunctionRector\Source\SomeClassWithDefaultParamValue;
use Twig\Extension\AbstractExtension;
use Twig\TwigFilter;

class HasDefaultValue2 extends AbstractExtension
{
public function getFilters()
{
return [
new TwigFilter('test', [$this, "run"]),
];
}

public function run($a, $b = "test")
{
return $a . $b;
}
}

?>
-----
<?php

namespace Rector\Tests\CodeQuality\Rector\Array_\CallableThisArrayToAnonymousFunctionRector\Fixture;

use Rector\Tests\CodeQuality\Rector\Array_\CallableThisArrayToAnonymousFunctionRector\Source\SomeClassWithDefaultParamValue;
use Twig\Extension\AbstractExtension;
use Twig\TwigFilter;

class HasDefaultValue2 extends AbstractExtension
{
public function getFilters()
{
return [
new TwigFilter('test', function ($a, $b = "test") {
return $this->run($a, $b);
}),
];
}

public function run($a, $b = "test")
{
return $a . $b;
}
}

?>
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
<?php

namespace Rector\Tests\CodeQuality\Rector\Array_\CallableThisArrayToAnonymousFunctionRector\Source;

class SomeClassWithDefaultParamValue
{
public function run($a, $b = ['test'])
{
return $a . $b;
}
}
112 changes: 74 additions & 38 deletions rules/Php72/NodeFactory/AnonymousFunctionFactory.php
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
use PhpParser\Node\Expr\ClassConstFetch;
use PhpParser\Node\Expr\Closure;
use PhpParser\Node\Expr\ClosureUse;
use PhpParser\Node\Expr\ConstFetch;
use PhpParser\Node\Expr\MethodCall;
use PhpParser\Node\Expr\New_;
use PhpParser\Node\Expr\StaticCall;
Expand All @@ -25,6 +26,7 @@
use PhpParser\Node\Scalar\LNumber;
use PhpParser\Node\Scalar\String_;
use PhpParser\Node\Stmt;
use PhpParser\Node\Stmt\ClassMethod;
use PhpParser\Node\Stmt\Expression;
use PhpParser\Node\Stmt\Foreach_;
use PhpParser\Node\Stmt\Return_;
Expand All @@ -34,12 +36,15 @@
use PHPStan\Reflection\ParametersAcceptorSelector;
use PHPStan\Reflection\Php\PhpMethodReflection;
use PHPStan\Type\MixedType;
use PHPStan\Type\Type;
use PHPStan\Type\VoidType;
use Rector\Core\Exception\ShouldNotHappenException;
use Rector\Core\PhpParser\AstResolver;
use Rector\Core\PhpParser\Comparing\NodeComparator;
use Rector\Core\PhpParser\Node\BetterNodeFinder;
use Rector\Core\PhpParser\Node\NodeFactory;
use Rector\Core\PhpParser\Parser\SimplePhpParser;
use Rector\Core\PhpParser\Printer\BetterStandardPrinter;
use Rector\NodeNameResolver\NodeNameResolver;
use Rector\NodeTypeResolver\Node\AttributeKey;
use Rector\PHPStanStaticTypeMapper\Enum\TypeKind;
Expand All @@ -61,7 +66,9 @@ public function __construct(
private readonly StaticTypeMapper $staticTypeMapper,
private readonly SimpleCallableNodeTraverser $simpleCallableNodeTraverser,
private readonly SimplePhpParser $simplePhpParser,
private readonly NodeComparator $nodeComparator
private readonly NodeComparator $nodeComparator,
private readonly AstResolver $astResolver,
private readonly BetterStandardPrinter $betterStandardPrinter
) {
}

Expand Down Expand Up @@ -103,7 +110,7 @@ public function createFromPhpMethodReflection(PhpMethodReflection $phpMethodRefl
$functionVariantWithPhpDoc = ParametersAcceptorSelector::selectSingle($phpMethodReflection->getVariants());

$anonymousFunction = new Closure();
$newParams = $this->createParams($functionVariantWithPhpDoc->getParameters());
$newParams = $this->createParams($phpMethodReflection, $functionVariantWithPhpDoc->getParameters());

$anonymousFunction->params = $newParams;

Expand All @@ -121,11 +128,9 @@ public function createFromPhpMethodReflection(PhpMethodReflection $phpMethodRefl
}

// does method return something?
if (! $functionVariantWithPhpDoc->getReturnType() instanceof VoidType) {
$anonymousFunction->stmts[] = new Return_($innerMethodCall);
} else {
$anonymousFunction->stmts[] = new Expression($innerMethodCall);
}
$anonymousFunction->stmts[] = $functionVariantWithPhpDoc->getReturnType() instanceof VoidType
? new Expression($innerMethodCall)
: new Return_($innerMethodCall);

if ($expr instanceof Variable && ! $this->nodeNameResolver->isName($expr, 'this')) {
$anonymousFunction->uses[] = new ClosureUse($expr);
Expand Down Expand Up @@ -247,17 +252,27 @@ private function collectUsesEqual(Closure $closure, array $uses, Variable $useVa
}

/**
* @param Node[] $nodes
* @param Param[] $paramNodes
* @return Variable[]
* @return string[]
*/
private function createUseVariablesFromParams(array $nodes, array $paramNodes): array
private function collectParamNames(array $paramNodes): array
{
$paramNames = [];
foreach ($paramNodes as $paramNode) {
$paramNames[] = $this->nodeNameResolver->getName($paramNode);
}

return $paramNames;
}

/**
* @param Node[] $nodes
* @param Param[] $paramNodes
* @return Variable[]
*/
private function createUseVariablesFromParams(array $nodes, array $paramNodes): array
{
$paramNames = $this->collectParamNames($paramNodes);
$variableNodes = $this->betterNodeFinder->findInstanceOf($nodes, Variable::class);

/** @var Variable[] $filteredVariables */
Expand All @@ -279,19 +294,17 @@ private function createUseVariablesFromParams(array $nodes, array $paramNodes):
}

$parentNode = $variableNode->getAttribute(AttributeKey::PARENT_NODE);
if (
$parentNode instanceof Assign
|| $parentNode instanceof Foreach_
|| $parentNode instanceof Param
) {
if ($parentNode instanceof Node && in_array(
$parentNode::class,
[Assign::class, Foreach_::class, Param::class],
true
)) {
$alreadyAssignedVariables[] = $variableName;
}

if ($this->nodeNameResolver->isNames($variableNode, $alreadyAssignedVariables)) {
continue;
if (! $this->nodeNameResolver->isNames($variableNode, $alreadyAssignedVariables)) {
$filteredVariables[$variableName] = $variableNode;
}

$filteredVariables[$variableName] = $variableNode;
}

return $filteredVariables;
Expand All @@ -301,25 +314,52 @@ private function createUseVariablesFromParams(array $nodes, array $paramNodes):
* @param ParameterReflection[] $parameterReflections
* @return Param[]
*/
private function createParams(array $parameterReflections): array
private function createParams(PhpMethodReflection $phpMethodReflection, array $parameterReflections): array
{
$classReflection = $phpMethodReflection->getDeclaringClass();
$className = $classReflection->getName();
$methodName = $phpMethodReflection->getName();
/** @var ClassMethod $classMethod */
$classMethod = $this->astResolver->resolveClassMethod($className, $methodName);

$params = [];
foreach ($parameterReflections as $parameterReflection) {
foreach ($parameterReflections as $key => $parameterReflection) {
$param = new Param(new Variable($parameterReflection->getName()));

if (! $parameterReflection->getType() instanceof MixedType) {
$param->type = $this->staticTypeMapper->mapPHPStanTypeToPhpParserNode(
$parameterReflection->getType(),
TypeKind::PARAM()
);
}
$this->applyParamType($param, $parameterReflection);
$this->applyParamDefaultValue($param, $parameterReflection, $key, $classMethod);

$params[] = $param;
}

return $params;
}

private function applyParamType(Param $param, ParameterReflection $parameterReflection): void
{
if ($parameterReflection->getType() instanceof MixedType) {
return;
}

$param->type = $this->staticTypeMapper->mapPHPStanTypeToPhpParserNode(
$parameterReflection->getType(),
TypeKind::PARAM()
);
}

private function applyParamDefaultValue(
Param $param,
ParameterReflection $parameterReflection,
int $key,
ClassMethod $classMethod
): void {
if (! $parameterReflection->getDefaultValue() instanceof Type) {
return;
}

$printDefaultValue = $this->betterStandardPrinter->print($classMethod->params[$key]->default);
$param->default = new ConstFetch(new Name($printDefaultValue));
}

/**
* @param Param[] $params
*/
Expand Down Expand Up @@ -366,11 +406,9 @@ private function normalizeClassConstFetchForStatic(Expr $expr): null | Name | Fu
}

$name = new Name($className);
if ($name->isSpecialClassName()) {
return $name;
}

return new FullyQualified($className);
return $name->isSpecialClassName()
? $name
: new FullyQualified($className);
}

private function resolveExpr(Expr $expr): New_ | Expr | null
Expand All @@ -385,10 +423,8 @@ private function resolveExpr(Expr $expr): New_ | Expr | null

// dynamic name, nothing we can do
$className = $this->nodeNameResolver->getName($expr->class);
if ($className === null) {
return null;
}

return new New_(new FullyQualified($className));
return $className === null
? null
: new New_(new FullyQualified($className));
}
}