From 5a3f4c80513b03adfcb6a6a3c610c03567d1178e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=B6ren=20W=C3=BCnsch?= Date: Thu, 13 Aug 2026 11:39:40 +0200 Subject: [PATCH 1/2] [CodeQuality] Narrow assertTrue(is_()) to assertIs() in AssertTrueFalseToSpecificMethodRector Adds the is_() family to the function map: is_array, is_bool, is_callable, is_float, is_int, is_iterable, is_numeric, is_object, is_scalar and is_string. $this->assertTrue(is_callable($value)) -> $this->assertIsCallable($value) $this->assertFalse(is_callable($value)) -> $this->assertIsNotCallable($value) is_callable() is skipped when it carries more than one argument, as its $syntax_only and $callable_name parameters have no counterpart in assertIsCallable() and would end up in the $message slot. is_resource() is left out on purpose: the IsType constraint counts "resource (closed)" as a resource, while is_resource() returns false for closed resources, so assertIsResource() is not an equivalent. Also bail out on named and spread args. The rule moves the inner function arguments up into the assert call, but the assert methods name their parameters differently, so a named arg produces a call that cannot be resolved: $this->assertTrue(is_readable(filename: $file)); // became assertIsReadable(filename: $file) -> Unknown named parameter Spread args have the same problem, as their count is unknown at compile time. This part is pre-existing and affects the whole map, not just the new entries. --- .../Fixture/is_type_functions.php.inc | 49 +++++++++++++++++++ .../Fixture/is_type_functions_negated.php.inc | 37 ++++++++++++++ .../skip_is_callable_with_syntax_only.php.inc | 14 ++++++ .../skip_named_and_spread_args.php.inc | 16 ++++++ .../AssertTrueFalseToSpecificMethodRector.php | 41 ++++++++++++++++ 5 files changed, 157 insertions(+) create mode 100644 rules-tests/CodeQuality/Rector/MethodCall/AssertTrueFalseToSpecificMethodRector/Fixture/is_type_functions.php.inc create mode 100644 rules-tests/CodeQuality/Rector/MethodCall/AssertTrueFalseToSpecificMethodRector/Fixture/is_type_functions_negated.php.inc create mode 100644 rules-tests/CodeQuality/Rector/MethodCall/AssertTrueFalseToSpecificMethodRector/Fixture/skip_is_callable_with_syntax_only.php.inc create mode 100644 rules-tests/CodeQuality/Rector/MethodCall/AssertTrueFalseToSpecificMethodRector/Fixture/skip_named_and_spread_args.php.inc diff --git a/rules-tests/CodeQuality/Rector/MethodCall/AssertTrueFalseToSpecificMethodRector/Fixture/is_type_functions.php.inc b/rules-tests/CodeQuality/Rector/MethodCall/AssertTrueFalseToSpecificMethodRector/Fixture/is_type_functions.php.inc new file mode 100644 index 00000000..a9145f91 --- /dev/null +++ b/rules-tests/CodeQuality/Rector/MethodCall/AssertTrueFalseToSpecificMethodRector/Fixture/is_type_functions.php.inc @@ -0,0 +1,49 @@ +assertTrue(is_array($value)); + $this->assertTrue(is_bool($value)); + $this->assertTrue(is_callable($value)); + $this->assertTrue(is_float($value)); + $this->assertTrue(is_int($value)); + $this->assertTrue(is_iterable($value)); + $this->assertTrue(is_numeric($value)); + $this->assertTrue(is_object($value)); + $this->assertTrue(is_scalar($value)); + $this->assertTrue(is_string($value), 'the value must be a string'); + } +} + +?> +----- +assertIsArray($value); + $this->assertIsBool($value); + $this->assertIsCallable($value); + $this->assertIsFloat($value); + $this->assertIsInt($value); + $this->assertIsIterable($value); + $this->assertIsNumeric($value); + $this->assertIsObject($value); + $this->assertIsScalar($value); + $this->assertIsString($value, 'the value must be a string'); + } +} + +?> diff --git a/rules-tests/CodeQuality/Rector/MethodCall/AssertTrueFalseToSpecificMethodRector/Fixture/is_type_functions_negated.php.inc b/rules-tests/CodeQuality/Rector/MethodCall/AssertTrueFalseToSpecificMethodRector/Fixture/is_type_functions_negated.php.inc new file mode 100644 index 00000000..c6bed2cb --- /dev/null +++ b/rules-tests/CodeQuality/Rector/MethodCall/AssertTrueFalseToSpecificMethodRector/Fixture/is_type_functions_negated.php.inc @@ -0,0 +1,37 @@ +assertFalse(is_callable($value)); + $this->assertNotTrue(is_array($value)); + $this->assertNotFalse(is_string($value)); + self::assertFalse(is_object($value), 'the value must not be an object'); + } +} + +?> +----- +assertIsNotCallable($value); + $this->assertIsNotArray($value); + $this->assertIsString($value); + self::assertIsNotObject($value, 'the value must not be an object'); + } +} + +?> diff --git a/rules-tests/CodeQuality/Rector/MethodCall/AssertTrueFalseToSpecificMethodRector/Fixture/skip_is_callable_with_syntax_only.php.inc b/rules-tests/CodeQuality/Rector/MethodCall/AssertTrueFalseToSpecificMethodRector/Fixture/skip_is_callable_with_syntax_only.php.inc new file mode 100644 index 00000000..f7f22b88 --- /dev/null +++ b/rules-tests/CodeQuality/Rector/MethodCall/AssertTrueFalseToSpecificMethodRector/Fixture/skip_is_callable_with_syntax_only.php.inc @@ -0,0 +1,14 @@ +assertTrue(is_callable($value, true)); + $this->assertTrue(is_callable($value, true, $callableName)); + } +} diff --git a/rules-tests/CodeQuality/Rector/MethodCall/AssertTrueFalseToSpecificMethodRector/Fixture/skip_named_and_spread_args.php.inc b/rules-tests/CodeQuality/Rector/MethodCall/AssertTrueFalseToSpecificMethodRector/Fixture/skip_named_and_spread_args.php.inc new file mode 100644 index 00000000..527a8109 --- /dev/null +++ b/rules-tests/CodeQuality/Rector/MethodCall/AssertTrueFalseToSpecificMethodRector/Fixture/skip_named_and_spread_args.php.inc @@ -0,0 +1,16 @@ +assertTrue(is_string(value: $value)); + $this->assertTrue(is_readable(filename: $file)); + $this->assertTrue(is_string(...$args)); + $this->assertTrue(in_array(needle: '...', haystack: ['...'], strict: true)); + } +} diff --git a/rules/CodeQuality/Rector/MethodCall/AssertTrueFalseToSpecificMethodRector.php b/rules/CodeQuality/Rector/MethodCall/AssertTrueFalseToSpecificMethodRector.php index 8d79c653..eaccf22e 100644 --- a/rules/CodeQuality/Rector/MethodCall/AssertTrueFalseToSpecificMethodRector.php +++ b/rules/CodeQuality/Rector/MethodCall/AssertTrueFalseToSpecificMethodRector.php @@ -42,6 +42,16 @@ final class AssertTrueFalseToSpecificMethodRector extends AbstractRector 'is_nan' => ['is_nan', 'assertNan', ''], 'is_a' => ['is_a', 'assertInstanceOf', 'assertNotInstanceOf'], 'str_contains' => ['str_contains', 'assertStringContainsString', 'assertStringNotContainsString'], + 'is_array' => ['is_array', 'assertIsArray', 'assertIsNotArray'], + 'is_bool' => ['is_bool', 'assertIsBool', 'assertIsNotBool'], + 'is_callable' => ['is_callable', 'assertIsCallable', 'assertIsNotCallable'], + 'is_float' => ['is_float', 'assertIsFloat', 'assertIsNotFloat'], + 'is_int' => ['is_int', 'assertIsInt', 'assertIsNotInt'], + 'is_iterable' => ['is_iterable', 'assertIsIterable', 'assertIsNotIterable'], + 'is_numeric' => ['is_numeric', 'assertIsNumeric', 'assertIsNotNumeric'], + 'is_object' => ['is_object', 'assertIsObject', 'assertIsNotObject'], + 'is_scalar' => ['is_scalar', 'assertIsScalar', 'assertIsNotScalar'], + 'is_string' => ['is_string', 'assertIsString', 'assertIsNotString'], ]; /** @@ -115,6 +125,10 @@ public function refactor(Node $node): ?Node return null; } + if ($firstArgumentValue instanceof FuncCall && $this->hasUnmovableArgs($firstArgumentValue)) { + return null; + } + if ($firstArgumentName === 'is_a') { /** @var FuncCall $firstArgumentValue */ $args = $firstArgumentValue->getArgs(); @@ -130,6 +144,14 @@ public function refactor(Node $node): ?Node } } + // the is_callable() $syntax_only and $callable_name arguments have no counterpart in assertIsCallable() + if ($firstArgumentName === 'is_callable') { + /** @var FuncCall $firstArgumentValue */ + if (count($firstArgumentValue->getArgs()) > 1) { + return null; + } + } + [$functionName, $assetMethodName, $notAssertMethodName] = self::FUNCTION_NAME_WITH_ASSERT_METHOD_NAMES[$firstArgumentName]; $functionNameWithAssertMethods = new FunctionNameWithAssertMethods($assetMethodName, $notAssertMethodName); @@ -148,6 +170,25 @@ private function resolveFirstArgument(FuncCall|Empty_ $firstArgumentValue): ?str : $this->getName($firstArgumentValue); } + /** + * The assert methods use different parameter names than the functions they replace, + * so named args cannot be moved up. The same goes for spread args, as their count is unknown. + */ + private function hasUnmovableArgs(FuncCall $funcCall): bool + { + foreach ($funcCall->getArgs() as $arg) { + if ($arg->name instanceof Identifier) { + return true; + } + + if ($arg->unpack) { + return true; + } + } + + return false; + } + private function renameMethod( MethodCall|StaticCall $node, FunctionNameWithAssertMethods $functionNameWithAssertMethods From 3ae1aa24a2dfe78ef0cbfcae7cc90e403fbf40c9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=B6ren=20W=C3=BCnsch?= Date: Thu, 13 Aug 2026 12:01:14 +0200 Subject: [PATCH 2/2] Update comment --- .../MethodCall/AssertTrueFalseToSpecificMethodRector.php | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/rules/CodeQuality/Rector/MethodCall/AssertTrueFalseToSpecificMethodRector.php b/rules/CodeQuality/Rector/MethodCall/AssertTrueFalseToSpecificMethodRector.php index eaccf22e..b480eacf 100644 --- a/rules/CodeQuality/Rector/MethodCall/AssertTrueFalseToSpecificMethodRector.php +++ b/rules/CodeQuality/Rector/MethodCall/AssertTrueFalseToSpecificMethodRector.php @@ -171,8 +171,11 @@ private function resolveFirstArgument(FuncCall|Empty_ $firstArgumentValue): ?str } /** - * The assert methods use different parameter names than the functions they replace, - * so named args cannot be moved up. The same goes for spread args, as their count is unknown. + * Args are moved up by position, so a named arg only survives when the assert method happens to + * use the same parameter name: is_readable(filename:) fits assertIsReadable(), while + * is_string(value:) does not fit assertIsString(), whose parameter is $actual. Bail out instead + * of tracking a parameter name per mapped function. Spread args cannot be moved up either, + * as their count is unknown. */ private function hasUnmovableArgs(FuncCall $funcCall): bool {