Skip to content

Commit 0e1d5a3

Browse files
mnastalskimnastalski
authored
Merge branch 'driftingly:main' into main
2 parents d5e875a + 06b5c1d commit 0e1d5a3

17 files changed

+198
-65
lines changed

‎build/composer-php-74.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
"description": "Rector upgrades rules for Laravel Framework",
66
"require": {
77
"php": "^7.4 || ^8.0",
8-
"rector/rector": "^2.0"
8+
"rector/rector": "^2.2.7"
99
},
1010
"autoload": {
1111
"psr-4": {

‎composer.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
"description": "Rector upgrades rules for Laravel Framework",
66
"require": {
77
"php": ">=8.3",
8-
"rector/rector": "^2.1.3",
8+
"rector/rector": "^2.2.7",
99
"webmozart/assert": "^1.11",
1010
"symplify/rule-doc-generator-contracts": "^11.2"
1111
},

‎config/sets/laravel60.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,6 @@
7070

7171
$rectorConfig
7272
->ruleWithConfiguration(ArgumentAdderRector::class, [ // https://github.com/laravel/framework/commit/6c1e014943a508afb2c10869c3175f7783a004e1
73-
new ArgumentAdder('Illuminate\Database\Capsule\Manager', 'table', 1, 'as', null),
73+
new ArgumentAdder('Illuminate\Database\Capsule\Manager', 'table', 1, 'as'),
7474
]);
7575
};

‎config/sets/laravel70.php‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -51,8 +51,7 @@
5151
'Illuminate\Contracts\Routing\UrlRoutable',
5252
'resolveRouteBinding',
5353
1,
54-
'field',
55-
null
54+
'field'
5655
),
5756
]);
5857

‎docs/rector_rules_overview.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
# 87 Rules Overview
1+
# 88 Rules Overview
22

33
## AbortIfRector
44

‎src/Rector/ClassMethod/MakeModelAttributesAndScopesProtectedRector.php‎

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@
1212
use PHPStan\Reflection\ClassReflection;
1313
use PHPStan\Type\ObjectType;
1414
use Rector\Php80\NodeAnalyzer\PhpAttributeAnalyzer;
15-
use Rector\Php81\NodeManipulator\AttributeGroupNewLiner;
1615
use Rector\PHPStan\ScopeFetcher;
1716
use Rector\Privatization\NodeManipulator\VisibilityManipulator;
1817
use RectorLaravel\AbstractRector;
@@ -26,8 +25,7 @@ class MakeModelAttributesAndScopesProtectedRector extends AbstractRector
2625
{
2726
public function __construct(
2827
private readonly VisibilityManipulator $visibilityManipulator,
29-
private readonly PhpAttributeAnalyzer $phpAttributeAnalyzer,
30-
private readonly AttributeGroupNewLiner $attributeGroupNewLiner,
28+
private readonly PhpAttributeAnalyzer $phpAttributeAnalyzer
3129
) {}
3230

3331
public function getRuleDefinition(): RuleDefinition
@@ -92,10 +90,6 @@ public function refactor(Node $node): ?Node
9290

9391
$this->visibilityManipulator->makeProtected($node);
9492

95-
if ($node->attrGroups !== []) {
96-
$this->attributeGroupNewLiner->newLine($this->file, $node);
97-
}
98-
9993
return $node;
10094
}
10195

‎src/Rector/Expr/AppEnvironmentComparisonToParameterRector.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -113,7 +113,7 @@ private function handleBinaryOp(BinaryOp $binaryOp): array
113113
$methodCall = array_values(
114114
array_filter(
115115
[$binaryOp->left, $binaryOp->right],
116-
fn ($node) => $this->validMethodCall($node),
116+
$this->validMethodCall(...),
117117
)
118118
)[0] ?? null;
119119

‎src/Rector/If_/ThrowIfRector.php‎

Lines changed: 64 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,16 @@
55
use PhpParser\Node;
66
use PhpParser\Node\Arg;
77
use PhpParser\Node\Expr;
8+
use PhpParser\Node\Expr\ArrayDimFetch;
89
use PhpParser\Node\Expr\Assign;
10+
use PhpParser\Node\Expr\BinaryOp\BooleanAnd;
11+
use PhpParser\Node\Expr\BinaryOp\BooleanOr;
912
use PhpParser\Node\Expr\BooleanNot;
1013
use PhpParser\Node\Expr\FuncCall;
14+
use PhpParser\Node\Expr\MethodCall;
15+
use PhpParser\Node\Expr\PropertyFetch;
16+
use PhpParser\Node\Expr\StaticCall;
17+
use PhpParser\Node\Expr\StaticPropertyFetch;
1118
use PhpParser\Node\Expr\Throw_;
1219
use PhpParser\Node\Expr\Variable;
1320
use PhpParser\Node\Name;
@@ -16,6 +23,7 @@
1623
use PhpParser\Node\Stmt\If_;
1724
use PhpParser\NodeVisitor;
1825
use Rector\NodeTypeResolver\Node\AttributeKey;
26+
use Rector\PhpParser\Node\BetterNodeFinder;
1927
use RectorLaravel\AbstractRector;
2028
use Symplify\RuleDocGenerator\ValueObject\CodeSample\CodeSample;
2129
use Symplify\RuleDocGenerator\ValueObject\RuleDefinition;
@@ -25,6 +33,8 @@
2533
*/
2634
class ThrowIfRector extends AbstractRector
2735
{
36+
public function __construct(private readonly BetterNodeFinder $betterNodeFinder) {}
37+
2838
public function getRuleDefinition(): RuleDefinition
2939
{
3040
return new RuleDefinition('Change if throw to throw_if', [
@@ -46,51 +56,45 @@ public function getRuleDefinition(): RuleDefinition
4656
]);
4757
}
4858

59+
/**
60+
* @return array<class-string<Node>>
61+
*/
4962
public function getNodeTypes(): array
5063
{
5164
return [If_::class];
5265
}
5366

67+
/**
68+
* @param If_ $node
69+
*/
5470
public function refactor(Node $node): ?Node
5571
{
56-
if (! $node instanceof If_) {
57-
return null;
58-
}
59-
72+
/**
73+
* This is too conservative, can work with cases with elseif and else execution branches.
74+
* Skip for now for simplicity.
75+
*/
6076
if ($node->else instanceof Else_ || $node->elseifs !== []) {
6177
return null;
6278
}
6379

6480
$ifStmts = $node->stmts;
65-
66-
// Check if there's a single throw statement inside the if
67-
if (count($ifStmts) !== 1 || ! $ifStmts[0] instanceof Expression || ! $ifStmts[0]->expr instanceof Throw_) {
81+
// Check if there's a single throw expression inside the if-statement
82+
if (! (count($ifStmts) === 1 && $ifStmts[0] instanceof Expression && $ifStmts[0]->expr instanceof Throw_)) {
6883
return null;
6984
}
7085

71-
$condition = $node->cond;
86+
$ifCondition = $node->cond;
7287
$throwExpr = $ifStmts[0]->expr;
7388

74-
if ($this->exceptionUsesVariablesAssignedByCondition($throwExpr, $condition)) {
89+
if (! $this->isSafeToTransform($throwExpr, $ifCondition)) {
7590
return null;
7691
}
7792

78-
// Check if the condition is a negation
79-
if ($condition instanceof BooleanNot) {
80-
// Create a new throw_unless function call
81-
$funcCall = new FuncCall(new Name('throw_unless'), [
82-
new Arg($condition->expr),
83-
new Arg($throwExpr->expr),
84-
]);
85-
} else {
86-
// Create a new throw_if function call
87-
$funcCall = new FuncCall(new Name('throw_if'), [
88-
new Arg($condition),
89-
new Arg($throwExpr->expr),
90-
]);
91-
}
92-
93-
$expression = new Expression($funcCall);
93+
$expression = new Expression(
94+
$ifCondition instanceof BooleanNot
95+
? new FuncCall(new Name('throw_unless'), [new Arg($ifCondition->expr), new Arg($throwExpr->expr)])
96+
: new FuncCall(new Name('throw_if'), [new Arg($ifCondition), new Arg($throwExpr->expr)])
97+
);
9498

9599
$comments = array_merge($node->getComments(), $ifStmts[0]->getComments());
96100
if ($comments !== []) {
@@ -100,33 +104,50 @@ public function refactor(Node $node): ?Node
100104
return $expression;
101105
}
102106

103-
/**
104-
* Make sure the exception doesn't use variables assigned by the condition or this
105-
* will cause broken code to be generated
106-
*/
107-
private function exceptionUsesVariablesAssignedByCondition(Expr $throwExpr, Expr $condition): bool
107+
private function isSafeToTransform(Throw_ $throw, Expr $expr): bool
108108
{
109-
$conditionVariables = [];
110-
$returnValue = false;
109+
$shouldTransform = true;
110+
$bannedNodeTypes = [MethodCall::class, StaticCall::class, FuncCall::class, ArrayDimFetch::class, PropertyFetch::class, StaticPropertyFetch::class];
111+
$this->traverseNodesWithCallable($throw->expr, function (Node $node) use (&$shouldTransform, $bannedNodeTypes, $expr): ?int {
112+
if (
113+
in_array($node::class, $bannedNodeTypes, true)
114+
|| $node instanceof Variable && ! $this->isSafeToTransformWithVariableAccess($node, $expr)
115+
) {
116+
$shouldTransform = false;
111117

112-
$this->traverseNodesWithCallable($condition, function (Node $node) use (&$conditionVariables): null {
113-
if ($node instanceof Assign) {
114-
$conditionVariables[] = $this->getName($node->var);
118+
return NodeVisitor::STOP_TRAVERSAL;
115119
}
116120

117121
return null;
118122
});
119123

120-
$this->traverseNodesWithCallable($throwExpr, function (Node $node) use ($conditionVariables, &$returnValue): ?int {
121-
if ($node instanceof Variable && in_array($this->getName($node), $conditionVariables, true)) {
122-
$returnValue = true;
124+
return $shouldTransform;
125+
}
123126

124-
return NodeVisitor::STOP_TRAVERSAL;
125-
}
127+
/**
128+
* Not safe to transform when throw expression contains variables that are assigned in the if-condition because of the short-circuit logical operators issue.
129+
* This method checks if the variable was assigned on the right side of a short-circuit logical operator (conjunction and disjunction).
130+
* Note: The check is a little too strict, because such a variable may be initialized before the if-statement, and in such case it doesn't matter if it was assigned somewhere in the condition.
131+
*/
132+
private function isSafeToTransformWithVariableAccess(Variable $variable, Expr $expr): bool
133+
{
134+
$firstShortCircuitOperator = $this->betterNodeFinder->findFirst(
135+
$expr,
136+
fn (Node $node): bool => $node instanceof BooleanAnd || $node instanceof BooleanOr
137+
);
138+
if (! $firstShortCircuitOperator instanceof Node) {
139+
return true;
140+
}
141+
assert($firstShortCircuitOperator instanceof BooleanAnd || $firstShortCircuitOperator instanceof BooleanOr);
126142

127-
return null;
128-
});
143+
$varName = $this->getName($variable);
144+
$hasUnsafeAssignment = $this->betterNodeFinder->findFirst(
145+
$firstShortCircuitOperator->right, // only here short-circuit problem can happen
146+
fn (Node $node): bool => $node instanceof Assign
147+
&& $node->var instanceof Variable
148+
&& $this->getName($node->var) === $varName
149+
);
129150

130-
return $returnValue;
151+
return ! $hasUnsafeAssignment instanceof Node;
131152
}
132153
}

‎src/Rector/PropertyFetch/ReplaceFakerInstanceWithHelperRector.php‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -137,7 +137,6 @@ private function refactorInterpolatedString(InterpolatedString $interpolatedStri
137137
return array_reduce(
138138
$parts,
139139
fn (?Expr $carry, Expr $part) => $carry === null ? $part : new Concat($carry, $part),
140-
null,
141140
);
142141
}
143142

‎src/Rector/StaticCall/DispatchToHelperFunctionsRector.php‎

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@
33
namespace RectorLaravel\Rector\StaticCall;
44

55
use PhpParser\Node;
6-
use PhpParser\Node\Arg;
76
use PhpParser\Node\Expr\FuncCall;
87
use PhpParser\Node\Expr\New_;
98
use PhpParser\Node\Expr\StaticCall;
@@ -137,11 +136,13 @@ private function createDispatchableCall(StaticCall $staticCall, string $method):
137136
return null;
138137
}
139138

140-
return new FuncCall(
141-
new Name($method),
142-
[
143-
new Arg(new New_(new FullyQualified($class), $staticCall->args)),
144-
],
139+
$className = $class->isSpecialClassName()
140+
? $class
141+
: new FullyQualified($class);
142+
143+
return $this->nodeFactory->createFuncCall(
144+
$method,
145+
[new New_($className, $staticCall->args)],
145146
);
146147
}
147148
}

0 commit comments

Comments
 (0)