Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions config/sets/phpunit-code-quality.php
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
use Rector\PHPUnit\CodeQuality\Rector\MethodCall\AssertSameTrueFalseToAssertTrueFalseRector;
use Rector\PHPUnit\CodeQuality\Rector\MethodCall\AssertTrueFalseToSpecificMethodRector;
use Rector\PHPUnit\CodeQuality\Rector\MethodCall\FlipAssertRector;
use Rector\PHPUnit\CodeQuality\Rector\MethodCall\MatchAssertSameExpectedTypeRector;
use Rector\PHPUnit\CodeQuality\Rector\MethodCall\NarrowIdenticalWithConsecutiveRector;
use Rector\PHPUnit\CodeQuality\Rector\MethodCall\NarrowSingleWillReturnCallbackRector;
use Rector\PHPUnit\CodeQuality\Rector\MethodCall\RemoveExpectAnyFromMockRector;
Expand All @@ -43,7 +44,10 @@
return static function (RectorConfig $rectorConfig): void {
$rectorConfig->rules([
ConstructClassMethodToSetUpTestCaseRector::class,

AssertSameTrueFalseToAssertTrueFalseRector::class,
MatchAssertSameExpectedTypeRector::class,

AssertEqualsToSameRector::class,
PreferPHPUnitThisCallRector::class,
YieldDataProviderRector::class,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
<?php

namespace Rector\PHPUnit\Tests\CodeQuality\Rector\MethodCall\MatchAssertSameExpectedTypeRector\Fixture;

use PHPUnit\Framework\TestCase;

final class MatchAssertType extends TestCase
{
public function test()
{
$this->assertSame('123', $this->getOrderId());
}

private function getOrderId(): int
{
return 123;
}
}

?>
-----
<?php

namespace Rector\PHPUnit\Tests\CodeQuality\Rector\MethodCall\MatchAssertSameExpectedTypeRector\Fixture;

use PHPUnit\Framework\TestCase;

final class MatchAssertType extends TestCase
{
public function test()
{
$this->assertSame(123, $this->getOrderId());
}

private function getOrderId(): int
{
return 123;
}
}

?>
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
<?php

namespace Rector\PHPUnit\Tests\CodeQuality\Rector\MethodCall\MatchAssertSameExpectedTypeRector\Fixture;

use PHPUnit\Framework\TestCase;

final class MatchIntegerToString extends TestCase
{
public function test()
{
$this->assertSame(123, $this->getOrderId());
}

private function getOrderId(): string
{
return '123';
}
}

?>
-----
<?php

namespace Rector\PHPUnit\Tests\CodeQuality\Rector\MethodCall\MatchAssertSameExpectedTypeRector\Fixture;

use PHPUnit\Framework\TestCase;

final class MatchIntegerToString extends TestCase
{
public function test()
{
$this->assertSame('123', $this->getOrderId());
}

private function getOrderId(): string
{
return '123';
}
}

?>
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
<?php

namespace Rector\PHPUnit\Tests\CodeQuality\Rector\MethodCall\MatchAssertSameExpectedTypeRector\Fixture;

use PHPUnit\Framework\TestCase;

final class NullableMatchAssertType extends TestCase
{
public function test()
{
$this->assertSame('123', $this->getOrderId());
}

private function getOrderId(): ?int
{
return 123;
}
}

?>
-----
<?php

namespace Rector\PHPUnit\Tests\CodeQuality\Rector\MethodCall\MatchAssertSameExpectedTypeRector\Fixture;

use PHPUnit\Framework\TestCase;

final class NullableMatchAssertType extends TestCase
{
public function test()
{
$this->assertSame(123, $this->getOrderId());
}

private function getOrderId(): ?int
{
return 123;
}
}

?>
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
<?php

declare(strict_types=1);

namespace Rector\PHPUnit\Tests\CodeQuality\Rector\MethodCall\MatchAssertSameExpectedTypeRector;

use Iterator;
use PHPUnit\Framework\Attributes\DataProvider;
use Rector\Testing\PHPUnit\AbstractRectorTestCase;

final class MatchAssertSameExpectedTypeRectorTest extends AbstractRectorTestCase
{
#[DataProvider('provideData')]
public function test(string $filePath): void
{
$this->doTestFile($filePath);
}

public static function provideData(): Iterator
{
return self::yieldFilesFromDirectory(__DIR__ . '/Fixture');
}

public function provideConfigFilePath(): string
{
return __DIR__ . '/config/configured_rule.php';
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
<?php

declare(strict_types=1);

use Rector\Config\RectorConfig;
use Rector\PHPUnit\CodeQuality\Rector\MethodCall\MatchAssertSameExpectedTypeRector;

return static function (RectorConfig $rectorConfig): void {
$rectorConfig->rule(MatchAssertSameExpectedTypeRector::class);
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,118 @@
<?php

declare(strict_types=1);

namespace Rector\PHPUnit\CodeQuality\Rector\MethodCall;

use PhpParser\Node;
use PhpParser\Node\Expr\MethodCall;
use PhpParser\Node\Expr\StaticCall;
use PhpParser\Node\Scalar\LNumber;
use PhpParser\Node\Scalar\String_;
use PHPStan\Type\TypeCombinator;
use Rector\PHPUnit\NodeAnalyzer\TestsNodeAnalyzer;
use Rector\Rector\AbstractRector;
use Symplify\RuleDocGenerator\ValueObject\CodeSample\CodeSample;
use Symplify\RuleDocGenerator\ValueObject\RuleDefinition;

/**
* @see \Rector\PHPUnit\Tests\CodeQuality\Rector\MethodCall\MatchAssertSameExpectedTypeRector\MatchAssertSameExpectedTypeRectorTest
*/
final class MatchAssertSameExpectedTypeRector extends AbstractRector
{
public function __construct(
private readonly TestsNodeAnalyzer $testsNodeAnalyzer
) {
}

public function getRuleDefinition(): RuleDefinition
{
return new RuleDefinition(
'Correct expected type in assertSame() method to match strict type of passed variable',
[
new CodeSample(
<<<'CODE_SAMPLE'
use PHPUnit\Framework\TestCase;

class SomeTest extends TestCase
{
public function run()
{
$this->assertSame('123', $this->getOrderId());
}

private function getOrderId(): int
{
return 123;
}
Comment on lines +39 to +47
Copy link
Member

@samsonasik samsonasik Aug 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@TomasVotruba on assertSame, it actually already error on the first usage before its transformation, that means the test already invalid on very beginning:

There was 1 failure:

1) ArrayLookup\Tests\CollectorTest::test
Failed asserting that 123 is identical to '123'.

so it probably should only apply on assertEquals ?

https://github.com/rectorphp/rector-phpunit/pull/510/files#diff-1d628c9179459ab3a6e79445318e1c19281e70418d547ad433a05f35d83cf8be

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@TomasVotruba I tested in our project, it cause error:

-        $this->assertEquals(1, $this->getOrderId());
+        $this->assertSame('1', $this->getOrderId);

that compare:

-'1'
+'00000000000000000001'

this should indeed only on assertEquals, assertSame is apply on actual same type, I will create a patch to only apply on assertEquals and rename the rule to:

-MatchAssertSameExpectedTypeRector
+MatchAssertEqualsExpectedTypeRector

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's add test fixture here

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

}
CODE_SAMPLE
,
<<<'CODE_SAMPLE'
use PHPUnit\Framework\TestCase;

class SomeTest extends TestCase
{
public function run()
{
$this->assertSame(123, $this->getOrderId());
}

private function getOrderId(): int
{
return 123;
}
}
CODE_SAMPLE
),
]
);
}

/**
* @return array<class-string<Node>>
*/
public function getNodeTypes(): array
{
return [MethodCall::class, StaticCall::class];
}

/**
* @param MethodCall|StaticCall $node
*/
public function refactor(Node $node): ?Node
{
if (! $this->testsNodeAnalyzer->isPHPUnitMethodCallNames($node, ['assertSame', 'assertEquals'])) {
return null;
}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should add check:

  • isFirstClassCallable()
  • count($node->getArgs()) >= 2

I will apply patch for it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

$expectedArg = $node->getArgs()[0];
if (! $expectedArg->value instanceof String_ && ! $expectedArg->value instanceof LNumber) {
return null;
}

$expectedType = $this->getType($expectedArg->value);

$variableExpr = $node->getArgs()[1]
->value;
$variableType = $this->getType($variableExpr);
Copy link
Member

@samsonasik samsonasik Aug 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this should be getNativeType(), avoid compare invalid doc:

   public function run()
    {
        $this->assertSame('123', $this->getOrderId());
    }

    /**
     * @return int
     */
    private function getOrderId()
    {
        return '123';
    }

I will apply patch for it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.


$directVariableType = TypeCombinator::removeNull($variableType);

if ($expectedType->isLiteralString()->yes() && $directVariableType->isInteger()->yes()) {
// update expected type to provided type
$expectedArg->value = new LNumber((int) $expectedArg->value->value);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LNumber is deprecated, use Int_ instead, I will create new PR for it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.


return $node;
}

if ($expectedType->isInteger()->yes() && $directVariableType->isString()->yes()) {
// update expected type to provided type
$expectedArg->value = new String_((string) $expectedArg->value->value);

return $node;
}

return null;
}
}