Skip to content

Commit d4200bb

Browse files
committed
AssertEqualsToSameRector: skip arrays whose actual value key order is not provably identical
1 parent 088cf1a commit d4200bb

5 files changed

Lines changed: 98 additions & 38 deletions

File tree

rules-tests/CodeQuality/Rector/MethodCall/AssertEqualsToSameRector/Fixture/assert_same_constant_array.php.inc

Lines changed: 0 additions & 33 deletions
This file was deleted.
Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
<?php
2+
3+
namespace Rector\PHPUnit\Tests\CodeQuality\Rector\MethodCall\AssertEqualsToSameRector\Fixture;
4+
5+
use PHPUnit\Framework\TestCase;
6+
7+
final class ConstantArraySameKeyOrder extends TestCase
8+
{
9+
public function test()
10+
{
11+
$expected = ['pack_keys' => 'DEFAULT', 'row_format' => 'REDUNDANT'];
12+
$actual = ['pack_keys' => 'DEFAULT', 'row_format' => 'REDUNDANT'];
13+
$this->assertEquals($expected, $actual);
14+
}
15+
}
16+
17+
?>
18+
-----
19+
<?php
20+
21+
namespace Rector\PHPUnit\Tests\CodeQuality\Rector\MethodCall\AssertEqualsToSameRector\Fixture;
22+
23+
use PHPUnit\Framework\TestCase;
24+
25+
final class ConstantArraySameKeyOrder extends TestCase
26+
{
27+
public function test()
28+
{
29+
$expected = ['pack_keys' => 'DEFAULT', 'row_format' => 'REDUNDANT'];
30+
$actual = ['pack_keys' => 'DEFAULT', 'row_format' => 'REDUNDANT'];
31+
$this->assertSame($expected, $actual);
32+
}
33+
}
34+
35+
?>
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
<?php
2+
3+
namespace Rector\PHPUnit\Tests\CodeQuality\Rector\MethodCall\AssertEqualsToSameRector\Fixture;
4+
5+
use PHPUnit\Framework\TestCase;
6+
7+
final class SkipConstantArrayDifferentKeyOrder extends TestCase
8+
{
9+
public function test()
10+
{
11+
$expected = ['pack_keys' => 'DEFAULT', 'row_format' => 'REDUNDANT'];
12+
$actual = ['row_format' => 'REDUNDANT', 'pack_keys' => 'DEFAULT'];
13+
$this->assertEquals($expected, $actual);
14+
}
15+
}
16+
17+
?>
Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
<?php
2+
3+
namespace Rector\PHPUnit\Tests\CodeQuality\Rector\MethodCall\AssertEqualsToSameRector\Fixture;
4+
5+
use PHPUnit\Framework\TestCase;
6+
7+
final class SkipConstantArrayUnknownActual extends TestCase
8+
{
9+
public function test(array $result)
10+
{
11+
$expected = [1 => 2];
12+
$this->assertEquals($expected, $result);
13+
}
14+
}
15+
16+
?>

rules/CodeQuality/Rector/MethodCall/AssertEqualsToSameRector.php

Lines changed: 30 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,8 @@
1515
use PHPStan\Type\BooleanType;
1616
use PHPStan\Type\Constant\ConstantArrayType;
1717
use PHPStan\Type\Constant\ConstantBooleanType;
18+
use PHPStan\Type\Constant\ConstantIntegerType;
19+
use PHPStan\Type\Constant\ConstantStringType;
1820
use PHPStan\Type\FloatType;
1921
use PHPStan\Type\IntegerType;
2022
use PHPStan\Type\MixedType;
@@ -107,7 +109,7 @@ public function refactor(Node $node): ?Node
107109
return null;
108110
}
109111

110-
if ($this->shouldSkipConstantArrayType($firstArgValue)) {
112+
if ($this->shouldSkipConstantArrayType($firstArgValue, $args[1]->value)) {
111113
return null;
112114
}
113115

@@ -161,15 +163,38 @@ private function shouldSkipLooseComparison(array $args): bool
161163
return $args[0]->value instanceof String_ && is_numeric($args[0]->value->value);
162164
}
163165

164-
private function shouldSkipConstantArrayType(Expr $expr): bool
166+
private function shouldSkipConstantArrayType(Expr $expectedExpr, Expr $actualExpr): bool
165167
{
166-
$type = $this->nodeTypeResolver->getNativeType($expr);
168+
$expectedType = $this->nodeTypeResolver->getNativeType($expectedExpr);
167169

168-
if (! $type instanceof ConstantArrayType) {
170+
if (! $expectedType instanceof ConstantArrayType) {
169171
return false;
170172
}
171173

172-
return $this->hasNonScalarType($type);
174+
if ($this->hasNonScalarType($expectedType)) {
175+
return true;
176+
}
177+
178+
// assertSame() compares arrays strictly, including key order, while assertEquals() ignores it;
179+
// only safe to narrow when the actual value is a constant array with the very same keys in the same order
180+
$actualType = $this->nodeTypeResolver->getNativeType($actualExpr);
181+
if (! $actualType instanceof ConstantArrayType) {
182+
return true;
183+
}
184+
185+
return ! $this->hasSameKeyOrder($expectedType, $actualType);
186+
}
187+
188+
private function hasSameKeyOrder(ConstantArrayType $expectedType, ConstantArrayType $actualType): bool
189+
{
190+
$expectedKeyTypes = $expectedType->getKeyTypes();
191+
$actualKeyTypes = $actualType->getKeyTypes();
192+
193+
if (count($expectedKeyTypes) !== count($actualKeyTypes)) {
194+
return false;
195+
}
196+
197+
return array_all($expectedKeyTypes, fn (ConstantIntegerType|ConstantStringType $expectedKeyType, $position): bool => $expectedKeyType->equals($actualKeyTypes[$position]));
173198
}
174199

175200
private function hasNonScalarType(ConstantArrayType $constantArrayType): bool

0 commit comments

Comments
 (0)