Skip to content

Commit 23397d8

Browse files
samsonasiksamsonasik
authored
[CodeQuality] Skip no code parameter on custom Throwable instance on ThrowWithPreviousExceptionRector (#7912)
* [CodeQuality] Skip no code parameter on custom Throwable instance on ThrowWithPreviousExceptionRector * Fix * Fix * fix * fix phpstan * fix * clean flag
1 parent e4081fe commit 23397d8

File tree

11 files changed

+106
-40
lines changed

11 files changed

+106
-40
lines changed

‎rules-tests/CodeQuality/Rector/Catch_/ThrowWithPreviousExceptionRector/Fixture/empty_brackets.php.inc‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,8 @@ class EmptyBrackets
88
{
99
try {
1010
$someCode = 1;
11-
} catch (Throwable $throwable) {
12-
throw new AnotherException;
11+
} catch (\Throwable $throwable) {
12+
throw new \RuntimeException;
1313
}
1414
}
1515
}
@@ -26,8 +26,8 @@ class EmptyBrackets
2626
{
2727
try {
2828
$someCode = 1;
29-
} catch (Throwable $throwable) {
30-
throw new AnotherException($throwable->getMessage(), $throwable->getCode(), $throwable);
29+
} catch (\Throwable $throwable) {
30+
throw new \RuntimeException($throwable->getMessage(), $throwable->getCode(), $throwable);
3131
}
3232
}
3333
}

‎rules-tests/CodeQuality/Rector/Catch_/ThrowWithPreviousExceptionRector/Fixture/fixture.php.inc‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,8 @@ class Fixture
88
{
99
try {
1010
$someCode = 1;
11-
} catch (Throwable $throwable) {
12-
throw new AnotherException('ups');
11+
} catch (\Throwable $throwable) {
12+
throw new \RuntimeException('ups');
1313
}
1414
}
1515
}
@@ -26,8 +26,8 @@ class Fixture
2626
{
2727
try {
2828
$someCode = 1;
29-
} catch (Throwable $throwable) {
30-
throw new AnotherException('ups', $throwable->getCode(), $throwable);
29+
} catch (\Throwable $throwable) {
30+
throw new \RuntimeException('ups', $throwable->getCode(), $throwable);
3131
}
3232
}
3333
}

‎rules-tests/CodeQuality/Rector/Catch_/ThrowWithPreviousExceptionRector/Fixture/named_argument.php.inc‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ class NamedArgument
99
try {
1010
$this->run();
1111
}catch(\Throwable $throwable) {
12-
throw new LogicException('Some exception', previous: $throwable);
12+
throw new \LogicException('Some exception', previous: $throwable);
1313
}
1414
}
1515
}
@@ -27,7 +27,7 @@ class NamedArgument
2727
try {
2828
$this->run();
2929
}catch(\Throwable $throwable) {
30-
throw new LogicException('Some exception', $throwable->getCode(), previous: $throwable);
30+
throw new \LogicException('Some exception', $throwable->getCode(), previous: $throwable);
3131
}
3232
}
3333
}

‎rules-tests/CodeQuality/Rector/Catch_/ThrowWithPreviousExceptionRector/Fixture/named_argument_for_message.php.inc‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ class NamedArgumentForMessage
99
try {
1010
$this->run();
1111
}catch(\Throwable $throwable) {
12-
throw new LogicException(message: 'Some exception');
12+
throw new \LogicException(message: 'Some exception');
1313
}
1414
}
1515
}
@@ -27,7 +27,7 @@ class NamedArgumentForMessage
2727
try {
2828
$this->run();
2929
}catch(\Throwable $throwable) {
30-
throw new LogicException(message: 'Some exception', code: $throwable->getCode(), previous: $throwable);
30+
throw new \LogicException(message: 'Some exception', code: $throwable->getCode(), previous: $throwable);
3131
}
3232
}
3333
}

‎rules-tests/CodeQuality/Rector/Catch_/ThrowWithPreviousExceptionRector/Fixture/skip_custom_exception_no_code_param.php.inc‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
<?php
2+
3+
namespace Rector\Tests\CodeQuality\Rector\Catch_\ThrowWithPreviousExceptionRector\Fixture;
4+
5+
use stdClass;
6+
7+
class CustomError extends \Exception
8+
{
9+
public function __construct(
10+
string $message = '',
11+
$nodes = null,
12+
?stdClass $source = null,
13+
?array $positions = null,
14+
?array $path = null,
15+
?\Throwable $previous = null,
16+
?array $extensions = null,
17+
?array $unaliasedPath = null
18+
) {
19+
parent::__construct($message, 0, $previous);
20+
}
21+
}
22+
23+
try {
24+
throw new \InvalidArgumentException('foo');
25+
} catch (\InvalidArgumentException $e) {
26+
throw new CustomError(message: 'Some error message', previous: $e);
27+
}

‎rules-tests/CodeQuality/Rector/Catch_/ThrowWithPreviousExceptionRector/Fixture/skip_custom_exception_param_order_flipped_no_namespace.php.inc‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ class HttpException extends \RuntimeException
1919

2020
class BadGatewayHttpException extends HttpException
2121
{
22-
public function __construct(string $message = '', ?Throwable $previous = null, array $headers = [], int $code = 0)
22+
public function __construct(string $message = '', ?\Throwable $previous = null, array $headers = [], int $code = 0)
2323
{
2424
parent::__construct(Response::HTTP_BAD_GATEWAY, $message, $previous, $headers, $code);
2525
}

‎rules-tests/CodeQuality/Rector/Catch_/ThrowWithPreviousExceptionRector/Fixture/skip_filled_exception_with_different_location.php.inc‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,15 +10,15 @@ class SkipFilledExceptionWithDifferentLocation
1010
public function run()
1111
{
1212
try {
13-
} catch (Throwable $throwable) {
13+
} catch (\Throwable $throwable) {
1414
throw new BadRequestHttpException('message some', $throwable);
1515
}
1616
}
1717
}
1818

1919
class BadRequestHttpException extends Exception
2020
{
21-
public function __construct(string $message = null, \Throwable $previous = null, int $code = 0, array $headers = [])
21+
public function __construct(string $message = null, ?\Throwable $previous = null, int $code = 0, array $headers = [])
2222
{
2323
parent::__construct('message', 400, $previous);
2424
}

‎rules-tests/CodeQuality/Rector/Catch_/ThrowWithPreviousExceptionRector/Fixture/skip_missing_location.php.inc‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ class SkipMissingLocation
1010
public function run()
1111
{
1212
try {
13-
} catch (Throwable $throwable) {
13+
} catch (\Throwable $throwable) {
1414
throw new MissingPreviousException('message some');
1515
}
1616
}

‎rules/CodeQuality/Rector/Catch_/ThrowWithPreviousExceptionRector.php‎

Lines changed: 53 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ public function run()
5151
try {
5252
$someCode = 1;
5353
} catch (Throwable $throwable) {
54-
throw new AnotherException('ups');
54+
throw new \RuntimeException('ups');
5555
}
5656
}
5757
}
@@ -65,7 +65,7 @@ public function run()
6565
try {
6666
$someCode = 1;
6767
} catch (Throwable $throwable) {
68-
throw new AnotherException('ups', $throwable->getCode(), $throwable);
68+
throw new \RuntimeException('ups', $throwable->getCode(), $throwable);
6969
}
7070
}
7171
}
@@ -142,34 +142,47 @@ private function refactorThrow(Throw_ $throw, Variable $caughtThrowableVariable)
142142
$messageArgument = $new->args[0] ?? null;
143143
$shouldUseNamedArguments = $messageArgument instanceof Arg && $messageArgument->name instanceof Identifier;
144144

145+
$hasChanged = false;
145146
if (! isset($new->args[0])) {
146147
// get previous message
147148
$getMessageMethodCall = new MethodCall($caughtThrowableVariable, 'getMessage');
148149
$new->args[0] = new Arg($getMessageMethodCall);
150+
$hasChanged = true;
149151
} elseif ($new->args[0] instanceof Arg && $new->args[0]->name instanceof Identifier && $new->args[0]->name->toString() === 'previous' && $this->nodeComparator->areNodesEqual(
150152
$new->args[0]->value,
151153
$caughtThrowableVariable
152154
)) {
153155
$new->args[0]->name->name = 'message';
154156
$new->args[0]->value = new MethodCall($caughtThrowableVariable, 'getMessage');
157+
$hasChanged = true;
155158
}
156159

157160
if (! isset($new->getArgs()[1])) {
158-
// get previous code
159-
$new->args[1] = new Arg(
160-
new MethodCall($caughtThrowableVariable, 'getCode'),
161-
name: $shouldUseNamedArguments ? new Identifier('code') : null
162-
);
161+
if ($this->hasCodeParameter($new->class)) {
162+
// get previous code
163+
$new->args[1] = new Arg(
164+
new MethodCall($caughtThrowableVariable, 'getCode'),
165+
name: $shouldUseNamedArguments ? new Identifier('code') : null
166+
);
167+
$hasChanged = true;
168+
} else {
169+
return null;
170+
}
163171
}
164172

165173
/** @var Arg $arg1 */
166174
$arg1 = $new->args[1];
167175
if ($arg1->name instanceof Identifier && $arg1->name->toString() === 'previous') {
168-
$new->args[1] = new Arg(
169-
new MethodCall($caughtThrowableVariable, 'getCode'),
170-
name: $shouldUseNamedArguments ? new Identifier('code') : null
171-
);
172-
$new->args[$exceptionArgumentPosition] = $arg1;
176+
if ($this->hasCodeParameter($new->class)) {
177+
$new->args[1] = new Arg(
178+
new MethodCall($caughtThrowableVariable, 'getCode'),
179+
name: $shouldUseNamedArguments ? new Identifier('code') : null
180+
);
181+
$new->args[$exceptionArgumentPosition] = $arg1;
182+
$hasChanged = true;
183+
} elseif (! $hasChanged) {
184+
return null;
185+
}
173186
} else {
174187
$new->args[$exceptionArgumentPosition] = new Arg(
175188
$caughtThrowableVariable,
@@ -184,6 +197,34 @@ private function refactorThrow(Throw_ $throw, Variable $caughtThrowableVariable)
184197
return NodeVisitor::DONT_TRAVERSE_CURRENT_AND_CHILDREN;
185198
}
186199

200+
private function hasCodeParameter(Name $exceptionName): bool
201+
{
202+
$className = $this->getName($exceptionName);
203+
if (! $this->reflectionProvider->hasClass($className)) {
204+
return false;
205+
}
206+
207+
$classReflection = $this->reflectionProvider->getClass($className);
208+
$construct = $classReflection->hasMethod(MethodName::CONSTRUCT);
209+
210+
if (! $construct) {
211+
return false;
212+
}
213+
214+
$extendedMethodReflection = $classReflection->getConstructor();
215+
$extendedParametersAcceptor = ParametersAcceptorSelector::combineAcceptors(
216+
$extendedMethodReflection->getVariants()
217+
);
218+
219+
foreach ($extendedParametersAcceptor->getParameters() as $extendedParameterReflection) {
220+
if ($extendedParameterReflection->getName() === 'code') {
221+
return true;
222+
}
223+
}
224+
225+
return false;
226+
}
227+
187228
private function resolveExceptionArgumentPosition(Name $exceptionName): ?int
188229
{
189230
$className = $this->getName($exceptionName);

‎rules/Php73/Rector/FuncCall/JsonThrowOnErrorRector.php‎

Lines changed: 9 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -29,8 +29,6 @@ final class JsonThrowOnErrorRector extends AbstractRector implements MinPhpVersi
2929
{
3030
private const array FLAGS = ['JSON_THROW_ON_ERROR'];
3131

32-
private bool $hasChanged = false;
33-
3432
public function __construct(
3533
private readonly ValueResolver $valueResolver,
3634
private readonly BetterNodeFinder $betterNodeFinder
@@ -77,9 +75,9 @@ public function refactor(Node $node): ?Node
7775
return null;
7876
}
7977

80-
$this->hasChanged = false;
78+
$hasChanged = false;
8179

82-
$this->traverseNodesWithCallable($node, function (Node $currentNode): ?FuncCall {
80+
$this->traverseNodesWithCallable($node, function (Node $currentNode) use (&$hasChanged): ?FuncCall {
8381
if (! $currentNode instanceof FuncCall) {
8482
return null;
8583
}
@@ -89,17 +87,17 @@ public function refactor(Node $node): ?Node
8987
}
9088

9189
if ($this->isName($currentNode, 'json_encode')) {
92-
return $this->processJsonEncode($currentNode);
90+
return $this->processJsonEncode($currentNode, $hasChanged);
9391
}
9492

9593
if ($this->isName($currentNode, 'json_decode')) {
96-
return $this->processJsonDecode($currentNode);
94+
return $this->processJsonDecode($currentNode, $hasChanged);
9795
}
9896

9997
return null;
10098
});
10199

102-
if ($this->hasChanged) {
100+
if ($hasChanged) {
103101
return $node;
104102
}
105103

@@ -134,7 +132,7 @@ private function shouldSkipFuncCall(FuncCall $funcCall): bool
134132
return $this->isFirstValueStringOrArray($funcCall);
135133
}
136134

137-
private function processJsonEncode(FuncCall $funcCall): FuncCall
135+
private function processJsonEncode(FuncCall $funcCall, bool &$hasChanged): FuncCall
138136
{
139137
$flags = [];
140138
if (isset($funcCall->args[1])) {
@@ -145,14 +143,14 @@ private function processJsonEncode(FuncCall $funcCall): FuncCall
145143

146144
$newArg = $this->getArgWithFlags($flags);
147145
if ($newArg instanceof Arg) {
148-
$this->hasChanged = true;
146+
$hasChanged = true;
149147
$funcCall->args[1] = $newArg;
150148
}
151149

152150
return $funcCall;
153151
}
154152

155-
private function processJsonDecode(FuncCall $funcCall): FuncCall
153+
private function processJsonDecode(FuncCall $funcCall, bool &$hasChanged): FuncCall
156154
{
157155
$flags = [];
158156
if (isset($funcCall->args[3])) {
@@ -172,7 +170,7 @@ private function processJsonDecode(FuncCall $funcCall): FuncCall
172170

173171
$newArg = $this->getArgWithFlags($flags);
174172
if ($newArg instanceof Arg) {
175-
$this->hasChanged = true;
173+
$hasChanged = true;
176174
$funcCall->args[3] = $newArg;
177175
}
178176

0 commit comments

Comments
 (0)