[code-quality] Add MatchAssertSameExpectedTypeRector #510
Conversation
| if (! $this->testsNodeAnalyzer->isPHPUnitMethodCallNames($node, ['assertSame', 'assertEquals'])) { | ||
| return null; | ||
| } | ||
|
|
There was a problem hiding this comment.
should add check:
isFirstClassCallable()count($node->getArgs()) >= 2
I will apply patch for it.
|
|
||
| $variableExpr = $node->getArgs()[1] | ||
| ->value; | ||
| $variableType = $this->getType($variableExpr); |
There was a problem hiding this comment.
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.
| public function run() | ||
| { | ||
| $this->assertSame('123', $this->getOrderId()); | ||
| } | ||
|
|
||
| private function getOrderId(): int | ||
| { | ||
| return 123; | ||
| } |
There was a problem hiding this comment.
@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 ?
There was a problem hiding this comment.
@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
+MatchAssertEqualsExpectedTypeRectorThere was a problem hiding this comment.
Let's add test fixture here
There was a problem hiding this comment.
|
|
||
| if ($expectedType->isLiteralString()->yes() && $directVariableType->isInteger()->yes()) { | ||
| // update expected type to provided type | ||
| $expectedArg->value = new LNumber((int) $expectedArg->value->value); |
There was a problem hiding this comment.
LNumber is deprecated, use Int_ instead, I will create new PR for it.
No description provided.