Skip to content

Commit 8a896f7

Browse files
SoeanTomasVotruba
Soean
and
TomasVotruba
authored
[CodeQuality] Narrow assertTrue(is_<type>()) to assertIs<Type>() (#770)
Co-authored-by: Tomas Votruba <tomas.vot@gmail.com>
1 parent e2d2cfb commit 8a896f7

5 files changed

Lines changed: 165 additions & 0 deletions

File tree

‎rules-tests/CodeQuality/Rector/MethodCall/AssertTrueFalseToSpecificMethodRector/Fixture/is_type_functions.php.inc‎

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
<?php
2+
3+
namespace Rector\PHPUnit\Tests\CodeQuality\Rector\MethodCall\AssertTrueFalseToSpecificMethodRector\Fixture;
4+
5+
use PHPUnit\Framework\TestCase;
6+
7+
final class IsTypeFunctions extends TestCase
8+
{
9+
public function test()
10+
{
11+
$this->assertTrue(is_array($value));
12+
$this->assertTrue(is_bool($value));
13+
$this->assertTrue(is_callable($value));
14+
$this->assertTrue(is_float($value));
15+
$this->assertTrue(is_int($value));
16+
$this->assertTrue(is_iterable($value));
17+
$this->assertTrue(is_numeric($value));
18+
$this->assertTrue(is_object($value));
19+
$this->assertTrue(is_scalar($value));
20+
$this->assertTrue(is_string($value), 'the value must be a string');
21+
}
22+
}
23+
24+
?>
25+
-----
26+
<?php
27+
28+
namespace Rector\PHPUnit\Tests\CodeQuality\Rector\MethodCall\AssertTrueFalseToSpecificMethodRector\Fixture;
29+
30+
use PHPUnit\Framework\TestCase;
31+
32+
final class IsTypeFunctions extends TestCase
33+
{
34+
public function test()
35+
{
36+
$this->assertIsArray($value);
37+
$this->assertIsBool($value);
38+
$this->assertIsCallable($value);
39+
$this->assertIsFloat($value);
40+
$this->assertIsInt($value);
41+
$this->assertIsIterable($value);
42+
$this->assertIsNumeric($value);
43+
$this->assertIsObject($value);
44+
$this->assertIsScalar($value);
45+
$this->assertIsString($value, 'the value must be a string');
46+
}
47+
}
48+
49+
?>

‎rules-tests/CodeQuality/Rector/MethodCall/AssertTrueFalseToSpecificMethodRector/Fixture/is_type_functions_negated.php.inc‎

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
<?php
2+
3+
namespace Rector\PHPUnit\Tests\CodeQuality\Rector\MethodCall\AssertTrueFalseToSpecificMethodRector\Fixture;
4+
5+
use PHPUnit\Framework\TestCase;
6+
7+
final class IsTypeFunctionsNegated extends TestCase
8+
{
9+
public function test()
10+
{
11+
$this->assertFalse(is_callable($value));
12+
$this->assertNotTrue(is_array($value));
13+
$this->assertNotFalse(is_string($value));
14+
self::assertFalse(is_object($value), 'the value must not be an object');
15+
}
16+
}
17+
18+
?>
19+
-----
20+
<?php
21+
22+
namespace Rector\PHPUnit\Tests\CodeQuality\Rector\MethodCall\AssertTrueFalseToSpecificMethodRector\Fixture;
23+
24+
use PHPUnit\Framework\TestCase;
25+
26+
final class IsTypeFunctionsNegated extends TestCase
27+
{
28+
public function test()
29+
{
30+
$this->assertIsNotCallable($value);
31+
$this->assertIsNotArray($value);
32+
$this->assertIsString($value);
33+
self::assertIsNotObject($value, 'the value must not be an object');
34+
}
35+
}
36+
37+
?>

‎rules-tests/CodeQuality/Rector/MethodCall/AssertTrueFalseToSpecificMethodRector/Fixture/skip_is_callable_with_syntax_only.php.inc‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
<?php
2+
3+
namespace Rector\PHPUnit\Tests\CodeQuality\Rector\MethodCall\AssertTrueFalseToSpecificMethodRector\Fixture;
4+
5+
use PHPUnit\Framework\TestCase;
6+
7+
final class SkipIsCallableWithSyntaxOnly extends TestCase
8+
{
9+
public function test()
10+
{
11+
$this->assertTrue(is_callable($value, true));
12+
$this->assertTrue(is_callable($value, true, $callableName));
13+
}
14+
}

‎rules-tests/CodeQuality/Rector/MethodCall/AssertTrueFalseToSpecificMethodRector/Fixture/skip_named_and_spread_args.php.inc‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
<?php
2+
3+
namespace Rector\PHPUnit\Tests\CodeQuality\Rector\MethodCall\AssertTrueFalseToSpecificMethodRector\Fixture;
4+
5+
use PHPUnit\Framework\TestCase;
6+
7+
final class SkipNamedAndSpreadArgs extends TestCase
8+
{
9+
public function test()
10+
{
11+
$this->assertTrue(is_string(value: $value));
12+
$this->assertTrue(is_readable(filename: $file));
13+
$this->assertTrue(is_string(...$args));
14+
$this->assertTrue(is_string(...));
15+
$this->assertTrue(in_array(needle: '...', haystack: ['...'], strict: true));
16+
}
17+
}

‎rules/CodeQuality/Rector/MethodCall/AssertTrueFalseToSpecificMethodRector.php‎

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,16 @@ final class AssertTrueFalseToSpecificMethodRector extends AbstractRector
4242
'is_nan' => ['is_nan', 'assertNan', ''],
4343
'is_a' => ['is_a', 'assertInstanceOf', 'assertNotInstanceOf'],
4444
'str_contains' => ['str_contains', 'assertStringContainsString', 'assertStringNotContainsString'],
45+
'is_array' => ['is_array', 'assertIsArray', 'assertIsNotArray'],
46+
'is_bool' => ['is_bool', 'assertIsBool', 'assertIsNotBool'],
47+
'is_callable' => ['is_callable', 'assertIsCallable', 'assertIsNotCallable'],
48+
'is_float' => ['is_float', 'assertIsFloat', 'assertIsNotFloat'],
49+
'is_int' => ['is_int', 'assertIsInt', 'assertIsNotInt'],
50+
'is_iterable' => ['is_iterable', 'assertIsIterable', 'assertIsNotIterable'],
51+
'is_numeric' => ['is_numeric', 'assertIsNumeric', 'assertIsNotNumeric'],
52+
'is_object' => ['is_object', 'assertIsObject', 'assertIsNotObject'],
53+
'is_scalar' => ['is_scalar', 'assertIsScalar', 'assertIsNotScalar'],
54+
'is_string' => ['is_string', 'assertIsString', 'assertIsNotString'],
4555
];
4656

4757
/**
@@ -115,6 +125,10 @@ public function refactor(Node $node): ?Node
115125
return null;
116126
}
117127

128+
if ($firstArgumentValue instanceof FuncCall && $this->hasUnmovableArgs($firstArgumentValue)) {
129+
return null;
130+
}
131+
118132
if ($firstArgumentName === 'is_a') {
119133
/** @var FuncCall $firstArgumentValue */
120134
$args = $firstArgumentValue->getArgs();
@@ -130,6 +144,14 @@ public function refactor(Node $node): ?Node
130144
}
131145
}
132146

147+
// the is_callable() $syntax_only and $callable_name arguments have no counterpart in assertIsCallable()
148+
if ($firstArgumentName === 'is_callable') {
149+
/** @var FuncCall $firstArgumentValue */
150+
if (count($firstArgumentValue->getArgs()) > 1) {
151+
return null;
152+
}
153+
}
154+
133155
[$functionName, $assetMethodName, $notAssertMethodName] = self::FUNCTION_NAME_WITH_ASSERT_METHOD_NAMES[$firstArgumentName];
134156

135157
$functionNameWithAssertMethods = new FunctionNameWithAssertMethods($assetMethodName, $notAssertMethodName);
@@ -148,6 +170,32 @@ private function resolveFirstArgument(FuncCall|Empty_ $firstArgumentValue): ?str
148170
: $this->getName($firstArgumentValue);
149171
}
150172

173+
/**
174+
* Args are moved up by position, so a named arg only survives when the assert method happens to
175+
* use the same parameter name: is_readable(filename:) fits assertIsReadable(), while
176+
* is_string(value:) does not fit assertIsString(), whose parameter is $actual. Bail out instead
177+
* of tracking a parameter name per mapped function. Spread args cannot be moved up either,
178+
* as their count is unknown.
179+
*/
180+
private function hasUnmovableArgs(FuncCall $funcCall): bool
181+
{
182+
if ($funcCall->isFirstClassCallable()) {
183+
return true;
184+
}
185+
186+
foreach ($funcCall->getArgs() as $arg) {
187+
if ($arg->name instanceof Identifier) {
188+
return true;
189+
}
190+
191+
if ($arg->unpack) {
192+
return true;
193+
}
194+
}
195+
196+
return false;
197+
}
198+
151199
private function renameMethod(
152200
MethodCall|StaticCall $node,
153201
FunctionNameWithAssertMethods $functionNameWithAssertMethods

0 commit comments

Comments
 (0)