Skip to content

Commit 7bb0b09

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

File tree

8 files changed

+70
-85
lines changed

8 files changed

+70
-85
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-tests/Php70/Rector/StaticCall/StaticCallOnNonStaticToInstanceCallRector/Fixture/skip_test_webmozart.php‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
<?php
2+
3+
namespace Rector\Tests\Php70\Rector\StaticCall\StaticCallOnNonStaticToInstanceCallRector\Fixture;
4+
5+
use Webmozart\Assert\Assert;
6+
7+
final class SkipTestWebmozart
8+
{
9+
public function doWork($values)
10+
{
11+
Assert::allIsAOf('SomeType', $values);
12+
}
13+
}

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

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

77
use PhpParser\Node;
8-
use PhpParser\Node\Arg;
9-
use PhpParser\Node\ArrayItem;
10-
use PhpParser\Node\Expr;
118
use PhpParser\Node\Expr\Assign;
129
use PhpParser\Node\Expr\Cast;
1310
use PhpParser\Node\Expr\Cast\Array_;
@@ -18,7 +15,6 @@
1815
use PhpParser\Node\Expr\Cast\String_;
1916
use PhpParser\Node\Expr\FuncCall;
2017
use PhpParser\Node\Stmt\Expression;
21-
use PhpParser\NodeVisitor;
2218
use Rector\PhpParser\Node\Value\ValueResolver;
2319
use Rector\Rector\AbstractRector;
2420
use Symplify\RuleDocGenerator\ValueObject\CodeSample\CodeSample;
@@ -44,126 +40,99 @@ final class SetTypeToCastRector extends AbstractRector
4440
'string' => String_::class,
4541
];
4642

47-
/**
48-
* @var string
49-
*/
50-
private const IS_ARG_VALUE_ITEM_SET_TYPE = 'is_arg_value_item_set_type';
51-
5243
public function __construct(
5344
private readonly ValueResolver $valueResolver
5445
) {
5546
}
5647

5748
public function getRuleDefinition(): RuleDefinition
5849
{
59-
return new RuleDefinition('Change `settype()` to `(type)` where possible', [
60-
new CodeSample(
61-
<<<'CODE_SAMPLE'
50+
return new RuleDefinition(
51+
'Change `settype()` to `(type)` on standalone line. `settype()` returns always success/failure bool value',
52+
[
53+
new CodeSample(
54+
<<<'CODE_SAMPLE'
6255
class SomeClass
6356
{
6457
public function run($foo)
6558
{
6659
settype($foo, 'string');
67-
68-
return settype($foo, 'integer');
6960
}
7061
}
7162
CODE_SAMPLE
72-
,
73-
<<<'CODE_SAMPLE'
63+
,
64+
<<<'CODE_SAMPLE'
7465
class SomeClass
7566
{
7667
public function run($foo)
7768
{
7869
$foo = (string) $foo;
79-
80-
return (int) $foo;
8170
}
8271
}
8372
CODE_SAMPLE
84-
),
85-
]);
73+
),
74+
75+
]
76+
);
8677
}
8778

8879
/**
8980
* @return array<class-string<Node>>
9081
*/
9182
public function getNodeTypes(): array
9283
{
93-
return [FuncCall::class, Expression::class, Assign::class, ArrayItem::class, Arg::class];
84+
return [Expression::class];
9485
}
9586

9687
/**
97-
* @param FuncCall|Expression|Assign|ArrayItem|Node\Arg $node
98-
* @return null|NodeVisitor::DONT_TRAVERSE_CHILDREN|Expression|Assign|Cast
88+
* @param Expression $node
9989
*/
100-
public function refactor(Node $node): null|int|Expression|Assign|Cast
90+
public function refactor(Node $node): null|Expression
10191
{
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-
92+
// skip expr that are not standalone line, as settype() returns success bool value
93+
// and cannot be casted
94+
if (! $node->expr instanceof FuncCall) {
10795
return null;
10896
}
10997

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);
98+
$assign = $this->refactorFuncCall($node->expr);
99+
if (! $assign instanceof Assign) {
100+
return null;
129101
}
130102

131-
return $this->refactorFuncCall($node, false);
103+
return new Expression($assign);
132104
}
133105

134-
private function refactorFuncCall(FuncCall $funcCall, bool $isStandaloneExpression): Assign|null|Cast
106+
private function refactorFuncCall(FuncCall $funcCall): Assign|null
135107
{
136-
if (! $this->isSetTypeFuncCall($funcCall)) {
108+
if (! $this->isName($funcCall, 'setType')) {
137109
return null;
138110
}
139111

140112
if ($funcCall->isFirstClassCallable()) {
141113
return null;
142114
}
143115

144-
if ($funcCall->getAttribute(self::IS_ARG_VALUE_ITEM_SET_TYPE) === true) {
116+
if (count($funcCall->getArgs()) < 2) {
145117
return null;
146118
}
147119

148-
$typeValue = $this->valueResolver->getValue($funcCall->getArgs()[1]->value);
120+
$secondArg = $funcCall->getArgs()[1];
121+
122+
$typeValue = $this->valueResolver->getValue($secondArg->value);
149123
if (! is_string($typeValue)) {
150124
return null;
151125
}
152126

153127
$typeValue = strtolower($typeValue);
154128

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

158132
if (isset(self::TYPE_TO_CAST[$typeValue])) {
159133
$castClass = self::TYPE_TO_CAST[$typeValue];
160134
$castNode = new $castClass($variable);
161135

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

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

174143
return null;
175144
}
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-
}
186145
}

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

Lines changed: 6 additions & 6 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;
@@ -26,12 +25,13 @@ public function enterNode(Node $node): ?Node
2625
return null;
2726
}
2827

29-
$funcCallName = $node->name->toString();
30-
foreach ($node->args as $arg) {
31-
if (! $arg instanceof Arg) {
32-
continue;
33-
}
28+
// has no args
29+
if ($node->isFirstClassCallable()) {
30+
return null;
31+
}
3432

33+
$funcCallName = $node->name->toString();
34+
foreach ($node->getArgs() as $arg) {
3535
if ($arg->value instanceof Array_) {
3636
$arg->value->setAttribute(AttributeKey::FROM_FUNC_CALL_NAME, $funcCallName);
3737
continue;

0 commit comments

Comments
 (0)