Skip to content

Commit 9ce6c8b

Browse files
TomasVotrubaTomasVotruba
committed
[fix] skip all-but standalone assign on SetTypeToCastRector
1 parent 8cd3cc6 commit 9ce6c8b

File tree

8 files changed

+55
-83
lines changed

8 files changed

+55
-83
lines changed

‎phpstan.neon‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -392,7 +392,6 @@ parameters:
392392
-
393393
identifier: rector.noIntegerRefactorReturn
394394
paths:
395-
- rules/CodeQuality/Rector/FuncCall/SetTypeToCastRector.php
396395
- rules/Php55/Rector/String_/StringClassNameToClassConstantRector.php
397396
- rules/Php80/Rector/Switch_/ChangeSwitchToMatchRector.php
398397
- tests/Issues/InfiniteLoop/Rector/MethodCall/InfinityLoopRector.php

‎rules-tests/CodeQuality/Rector/FuncCall/SetTypeToCastRector/Fixture/fixture.php.inc‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,6 @@ class Fixture
1313
settype($foo, 'double');
1414
settype($foo, 'bool');
1515
settype($foo, 'boolean');
16-
17-
return settype($foo, 'integer');
1816
}
1917

2018
public function null()
@@ -40,8 +38,6 @@ class Fixture
4038
$foo = (double) $foo;
4139
$foo = (bool) $foo;
4240
$foo = (bool) $foo;
43-
44-
return (int) $foo;
4541
}
4642

4743
public function null()

‎rules-tests/CodeQuality/Rector/FuncCall/SetTypeToCastRector/Fixture/skip_args.php.inc‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,5 @@ final class SkipArgs
77
public function run($foo)
88
{
99
is_bool(settype($foo, 'string'));
10-
11-
settype($foo, settype($foo, 'string'));
1210
}
1311
}

‎rules-tests/CodeQuality/Rector/FuncCall/SetTypeToCastRector/Fixture/skip_assign_and_return.php.inc‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace Rector\Tests\CodeQuality\Rector\FuncCall\SetTypeToCastRector\Fixture;
6+
7+
final class SkipAssignAndrReturn
8+
{
9+
public function run($foo)
10+
{
11+
// $result is always bool, as settype() returns true on success, false on failure
12+
$result = settype($foo, 'string');
13+
}
14+
15+
public function retype($foo)
16+
{
17+
// $result is always bool, as settype() returns true on success, false on failure
18+
return settype($foo, 'string');
19+
}
20+
}

rules-tests/CodeQuality/Rector/FuncCall/SetTypeToCastRector/Fixture/skip_assign.php.inc renamed to rules-tests/CodeQuality/Rector/FuncCall/SetTypeToCastRector/Fixture/skip_invalid_cast_type.php.inc

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,10 @@
22

33
namespace Rector\Tests\CodeQuality\Rector\FuncCall\SetTypeToCastRector\Fixture;
44

5-
class SkipAssign
5+
final class SkipInvalidCastType
66
{
77
public function run($foo)
88
{
9-
$result = settype($foo, 'string');
9+
is_bool(settype($foo, 'super-element'));
1010
}
1111
}

‎rules/CodeQuality/Rector/FuncCall/SetTypeToCastRector.php‎

Lines changed: 29 additions & 69 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,6 @@
55
namespace Rector\CodeQuality\Rector\FuncCall;
66

77
use PhpParser\Node;
8-
use PhpParser\Node\Arg;
9-
use PhpParser\Node\ArrayItem;
108
use PhpParser\Node\Expr;
119
use PhpParser\Node\Expr\Assign;
1210
use PhpParser\Node\Expr\Cast;
@@ -18,7 +16,6 @@
1816
use PhpParser\Node\Expr\Cast\String_;
1917
use PhpParser\Node\Expr\FuncCall;
2018
use PhpParser\Node\Stmt\Expression;
21-
use PhpParser\NodeVisitor;
2219
use Rector\PhpParser\Node\Value\ValueResolver;
2320
use Rector\Rector\AbstractRector;
2421
use Symplify\RuleDocGenerator\ValueObject\CodeSample\CodeSample;
@@ -44,126 +41,99 @@ final class SetTypeToCastRector extends AbstractRector
4441
'string' => String_::class,
4542
];
4643

47-
/**
48-
* @var string
49-
*/
50-
private const IS_ARG_VALUE_ITEM_SET_TYPE = 'is_arg_value_item_set_type';
51-
5244
public function __construct(
5345
private readonly ValueResolver $valueResolver
5446
) {
5547
}
5648

5749
public function getRuleDefinition(): RuleDefinition
5850
{
59-
return new RuleDefinition('Change `settype()` to `(type)` where possible', [
60-
new CodeSample(
61-
<<<'CODE_SAMPLE'
51+
return new RuleDefinition(
52+
'Change `settype()` to `(type)` on standalone line. `settype()` returns always success/failure bool value',
53+
[
54+
new CodeSample(
55+
<<<'CODE_SAMPLE'
6256
class SomeClass
6357
{
6458
public function run($foo)
6559
{
6660
settype($foo, 'string');
67-
68-
return settype($foo, 'integer');
6961
}
7062
}
7163
CODE_SAMPLE
72-
,
73-
<<<'CODE_SAMPLE'
64+
,
65+
<<<'CODE_SAMPLE'
7466
class SomeClass
7567
{
7668
public function run($foo)
7769
{
7870
$foo = (string) $foo;
79-
80-
return (int) $foo;
8171
}
8272
}
8373
CODE_SAMPLE
84-
),
85-
]);
74+
),
75+
76+
]
77+
);
8678
}
8779

8880
/**
8981
* @return array<class-string<Node>>
9082
*/
9183
public function getNodeTypes(): array
9284
{
93-
return [FuncCall::class, Expression::class, Assign::class, ArrayItem::class, Arg::class];
85+
return [Expression::class];
9486
}
9587

9688
/**
97-
* @param FuncCall|Expression|Assign|ArrayItem|Node\Arg $node
98-
* @return null|NodeVisitor::DONT_TRAVERSE_CHILDREN|Expression|Assign|Cast
89+
* @param Expression $node
9990
*/
100-
public function refactor(Node $node): null|int|Expression|Assign|Cast
91+
public function refactor(Node $node): null|Expression
10192
{
102-
if ($node instanceof Arg || $node instanceof ArrayItem) {
103-
if ($this->isSetTypeFuncCall($node->value)) {
104-
$node->value->setAttribute(self::IS_ARG_VALUE_ITEM_SET_TYPE, true);
105-
}
106-
93+
// skip expr that are not standalone line, as settype() returns success bool value
94+
// and cannot be casted
95+
if (! $node->expr instanceof FuncCall) {
10796
return null;
10897
}
10998

110-
if ($node instanceof Assign) {
111-
if (! $this->isSetTypeFuncCall($node->expr)) {
112-
return null;
113-
}
114-
115-
return NodeVisitor::DONT_TRAVERSE_CHILDREN;
116-
}
117-
118-
if ($node instanceof Expression) {
119-
if (! $node->expr instanceof FuncCall) {
120-
return null;
121-
}
122-
123-
$assignOrCast = $this->refactorFuncCall($node->expr, true);
124-
if (! $assignOrCast instanceof Expr) {
125-
return null;
126-
}
127-
128-
return new Expression($assignOrCast);
99+
$assign = $this->refactorFuncCall($node->expr);
100+
if (! $assign instanceof Assign) {
101+
return null;
129102
}
130103

131-
return $this->refactorFuncCall($node, false);
104+
return new Expression($assign);
132105
}
133106

134-
private function refactorFuncCall(FuncCall $funcCall, bool $isStandaloneExpression): Assign|null|Cast
107+
private function refactorFuncCall(FuncCall $funcCall): Assign|null
135108
{
136-
if (! $this->isSetTypeFuncCall($funcCall)) {
109+
if (! $this->isName($funcCall, 'setType')) {
137110
return null;
138111
}
139112

140113
if ($funcCall->isFirstClassCallable()) {
141114
return null;
142115
}
143116

144-
if ($funcCall->getAttribute(self::IS_ARG_VALUE_ITEM_SET_TYPE) === true) {
117+
if (count($funcCall->getArgs()) < 2) {
145118
return null;
146119
}
147120

148-
$typeValue = $this->valueResolver->getValue($funcCall->getArgs()[1]->value);
121+
$secondArg = $funcCall->getArgs()[1];
122+
123+
$typeValue = $this->valueResolver->getValue($secondArg->value);
149124
if (! is_string($typeValue)) {
150125
return null;
151126
}
152127

153128
$typeValue = strtolower($typeValue);
154129

155-
$variable = $funcCall->getArgs()[0]
156-
->value;
130+
$firstArg = $funcCall->getArgs()[0];
131+
$variable = $firstArg->value;
157132

158133
if (isset(self::TYPE_TO_CAST[$typeValue])) {
159134
$castClass = self::TYPE_TO_CAST[$typeValue];
160135
$castNode = new $castClass($variable);
161136

162-
if (! $isStandaloneExpression) {
163-
return $castNode;
164-
}
165-
166-
// bare expression? → assign
167137
return new Assign($variable, $castNode);
168138
}
169139

@@ -173,14 +143,4 @@ private function refactorFuncCall(FuncCall $funcCall, bool $isStandaloneExpressi
173143

174144
return null;
175145
}
176-
177-
private function isSetTypeFuncCall(Expr $expr): bool
178-
{
179-
// skip assign of settype() calls
180-
if (! $expr instanceof FuncCall) {
181-
return false;
182-
}
183-
184-
return $this->isName($expr, 'settype');
185-
}
186146
}

‎src/NodeTypeResolver/Node/AttributeKey.php‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -292,4 +292,6 @@ final class AttributeKey
292292
public const IS_CLASS_CONST_VALUE = 'is_default_class_const_value';
293293

294294
public const IS_INSIDE_SYMFONY_PHP_CLOSURE = 'is_inside_symfony_php_closure';
295+
296+
public const IS_ARG_EXPR = 'is_arg_expr';
295297
}

‎src/NodeTypeResolver/PHPStan/Scope/NodeVisitor/ArgNodeVisitor.php‎

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@
55
namespace Rector\NodeTypeResolver\PHPStan\Scope\NodeVisitor;
66

77
use PhpParser\Node;
8-
use PhpParser\Node\Arg;
98
use PhpParser\Node\Expr\Array_;
109
use PhpParser\Node\Expr\ArrayDimFetch;
1110
use PhpParser\Node\Expr\FuncCall;
@@ -27,10 +26,8 @@ public function enterNode(Node $node): ?Node
2726
}
2827

2928
$funcCallName = $node->name->toString();
30-
foreach ($node->args as $arg) {
31-
if (! $arg instanceof Arg) {
32-
continue;
33-
}
29+
foreach ($node->getArgs() as $arg) {
30+
$arg->value->setAttribute(AttributeKey::IS_ARG_EXPR, true);
3431

3532
if ($arg->value instanceof Array_) {
3633
$arg->value->setAttribute(AttributeKey::FROM_FUNC_CALL_NAME, $funcCallName);

0 commit comments

Comments
 (0)