Skip to content

Commit 4c52b71

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

7 files changed

Lines changed: 57 additions & 89 deletions

File tree

‎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.php.inc‎

Lines changed: 0 additions & 11 deletions
This file was deleted.

‎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/CodeQuality/Rector/FuncCall/SetTypeToCastRector.php‎

Lines changed: 33 additions & 67 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@
66

77
use PhpParser\Node;
88
use PhpParser\Node\Arg;
9-
use PhpParser\Node\ArrayItem;
109
use PhpParser\Node\Expr;
1110
use PhpParser\Node\Expr\Assign;
1211
use PhpParser\Node\Expr\Cast;
@@ -18,7 +17,6 @@
1817
use PhpParser\Node\Expr\Cast\String_;
1918
use PhpParser\Node\Expr\FuncCall;
2019
use PhpParser\Node\Stmt\Expression;
21-
use PhpParser\NodeVisitor;
2220
use Rector\PhpParser\Node\Value\ValueResolver;
2321
use Rector\Rector\AbstractRector;
2422
use Symplify\RuleDocGenerator\ValueObject\CodeSample\CodeSample;
@@ -44,10 +42,10 @@ final class SetTypeToCastRector extends AbstractRector
4442
'string' => String_::class,
4543
];
4644

47-
/**
48-
* @var string
49-
*/
50-
private const IS_ARG_VALUE_ITEM_SET_TYPE = 'is_arg_value_item_set_type';
45+
// /**
46+
// * @var string
47+
// */
48+
// private const IS_ARG_VALUE_ITEM_SET_TYPE = 'is_arg_value_item_set_type';
5149

5250
public function __construct(
5351
private readonly ValueResolver $valueResolver
@@ -56,114 +54,92 @@ public function __construct(
5654

5755
public function getRuleDefinition(): RuleDefinition
5856
{
59-
return new RuleDefinition('Change `settype()` to `(type)` where possible', [
60-
new CodeSample(
61-
<<<'CODE_SAMPLE'
57+
return new RuleDefinition(
58+
'Change `settype()` to `(type)` on standalone line. `settype()` returns always success/failure bool value',
59+
[
60+
new CodeSample(
61+
<<<'CODE_SAMPLE'
6262
class SomeClass
6363
{
6464
public function run($foo)
6565
{
6666
settype($foo, 'string');
67-
68-
return settype($foo, 'integer');
6967
}
7068
}
7169
CODE_SAMPLE
72-
,
73-
<<<'CODE_SAMPLE'
70+
,
71+
<<<'CODE_SAMPLE'
7472
class SomeClass
7573
{
7674
public function run($foo)
7775
{
7876
$foo = (string) $foo;
79-
80-
return (int) $foo;
8177
}
8278
}
8379
CODE_SAMPLE
84-
),
85-
]);
80+
),
81+
82+
]
83+
);
8684
}
8785

8886
/**
8987
* @return array<class-string<Node>>
9088
*/
9189
public function getNodeTypes(): array
9290
{
93-
return [FuncCall::class, Expression::class, Assign::class, ArrayItem::class, Arg::class];
91+
return [Expression::class];
9492
}
9593

9694
/**
97-
* @param FuncCall|Expression|Assign|ArrayItem|Node\Arg $node
98-
* @return null|NodeVisitor::DONT_TRAVERSE_CHILDREN|Expression|Assign|Cast
95+
* @param Expression $node
9996
*/
100-
public function refactor(Node $node): null|int|Expression|Assign|Cast
97+
public function refactor(Node $node): null|Expression
10198
{
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-
99+
// skip expr that are part of Arg, as settype() returns success bool value
100+
// and cannot be casted
101+
if (! $node->expr instanceof FuncCall) {
107102
return null;
108103
}
109104

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);
105+
$assignOrCast = $this->refactorFuncCall($node->expr);
106+
if (! $assignOrCast instanceof Expr) {
107+
return null;
129108
}
130109

131-
return $this->refactorFuncCall($node, false);
110+
return new Expression($assignOrCast);
132111
}
133112

134-
private function refactorFuncCall(FuncCall $funcCall, bool $isStandaloneExpression): Assign|null|Cast
113+
private function refactorFuncCall(FuncCall $funcCall): Assign|null
135114
{
136-
if (! $this->isSetTypeFuncCall($funcCall)) {
115+
if (! $this->isName($funcCall, 'setType')) {
137116
return null;
138117
}
139118

140119
if ($funcCall->isFirstClassCallable()) {
141120
return null;
142121
}
143122

144-
if ($funcCall->getAttribute(self::IS_ARG_VALUE_ITEM_SET_TYPE) === true) {
123+
if (count($funcCall->getArgs()) < 2) {
145124
return null;
146125
}
147126

148-
$typeValue = $this->valueResolver->getValue($funcCall->getArgs()[1]->value);
127+
$secondArg = $funcCall->getArgs()[1];
128+
129+
$typeValue = $this->valueResolver->getValue($secondArg->value);
149130
if (! is_string($typeValue)) {
150131
return null;
151132
}
152133

153134
$typeValue = strtolower($typeValue);
154135

155-
$variable = $funcCall->getArgs()[0]
156-
->value;
136+
$firstARg = $funcCall->getArgs()[0];
137+
$variable = $firstARg->value;
157138

158139
if (isset(self::TYPE_TO_CAST[$typeValue])) {
159140
$castClass = self::TYPE_TO_CAST[$typeValue];
160141
$castNode = new $castClass($variable);
161142

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

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

174150
return null;
175151
}
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-
}
186152
}

‎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)