diff --git a/Makefile b/Makefile index 6d79b27..02c6d35 100644 --- a/Makefile +++ b/Makefile @@ -7,7 +7,7 @@ tests: .PHONY: lint lint: - php vendor/bin/parallel-lint --colors \ + php vendor/bin/parallel-lint --colors -e php,php3,php4,php5,phtml,phpt,fixed \ src tests .PHONY: cs-install diff --git a/src/Rules/Cast/UselessCastRule.php b/src/Rules/Cast/UselessCastRule.php index 4e0be95..78507b3 100644 --- a/src/Rules/Cast/UselessCastRule.php +++ b/src/Rules/Cast/UselessCastRule.php @@ -55,28 +55,39 @@ public function processNode(Node $node, Scope $scope): array $expressionType = $scope->getNativeType($node->expr); } if ($castType->isSuperTypeOf($expressionType)->yes()) { - $addTip = function (RuleErrorBuilder $ruleErrorBuilder) use ($scope, $node, $castType): RuleErrorBuilder { - if (!$this->treatPhpDocTypesAsCertain) { - return $ruleErrorBuilder; - } + $ruleErrorBuilder = RuleErrorBuilder::message(sprintf( + 'Casting to %s something that\'s already %s.', + $castType->describe(VerbosityLevel::typeOnly()), + $expressionType->describe(VerbosityLevel::typeOnly()), + ))->identifier('cast.useless'); - if (!$this->treatPhpDocTypesAsCertainTip) { - return $ruleErrorBuilder; - } + $nativeType = $scope->getNativeType($node->expr); + if ($castType->isSuperTypeOf($nativeType)->yes()) { + $ruleErrorBuilder->fixNode($node, static function (Cast $node) use ($nativeType): Node { + if ($node instanceof Cast\Bool_) { + if (!$nativeType->isBoolean()->yes()) { + return $node; + } + } elseif ($node instanceof Cast\Int_) { + if ( + $node->expr instanceof Node\Expr\UnaryMinus + || !$nativeType->isInteger()->yes() + ) { + return $node; + } + } else { + // skip all other casts + return $node; + } - $expressionTypeWithoutPhpDoc = $scope->getNativeType($node->expr); - if ($castType->isSuperTypeOf($expressionTypeWithoutPhpDoc)->yes()) { - return $ruleErrorBuilder; - } + return $node->expr; + }); + } elseif ($this->treatPhpDocTypesAsCertain && $this->treatPhpDocTypesAsCertainTip) { + $ruleErrorBuilder->treatPhpDocTypesAsCertainTip(); + } - return $ruleErrorBuilder->treatPhpDocTypesAsCertainTip(); - }; return [ - $addTip(RuleErrorBuilder::message(sprintf( - 'Casting to %s something that\'s already %s.', - $castType->describe(VerbosityLevel::typeOnly()), - $expressionType->describe(VerbosityLevel::typeOnly()), - )))->identifier('cast.useless')->build(), + $ruleErrorBuilder->build(), ]; } diff --git a/tests/Rules/Cast/UselessCastRuleTest.php b/tests/Rules/Cast/UselessCastRuleTest.php index e0297e8..7fbd1da 100644 --- a/tests/Rules/Cast/UselessCastRuleTest.php +++ b/tests/Rules/Cast/UselessCastRuleTest.php @@ -13,11 +13,13 @@ class UselessCastRuleTest extends RuleTestCase private bool $treatPhpDocTypesAsCertain; + private bool $treatPhpDocTypesAsCertainTip = true; + protected function getRule(): Rule { return new UselessCastRule( $this->treatPhpDocTypesAsCertain, - true, + $this->treatPhpDocTypesAsCertainTip, ); } @@ -95,4 +97,31 @@ public function testBug14565(): void $this->analyse([__DIR__ . '/data/bug-14565.php'], []); } + public function testVoidCastIsNotFixed(): void + { + $this->treatPhpDocTypesAsCertain = true; + $this->fix(__DIR__ . '/data/bug-14565.php', __DIR__ . '/data/bug-14565.php'); + } + + /** + * @return iterable + */ + public function dataFix(): iterable + { + yield [true, true]; + yield [true, false]; + yield [false, true]; + yield [false, false]; + } + + /** + * @dataProvider dataFix + */ + public function testFix(bool $treatPhpDocTypesAsCertain, bool $treatPhpDocTypesAsCertainTip): void + { + $this->treatPhpDocTypesAsCertain = $treatPhpDocTypesAsCertain; + $this->treatPhpDocTypesAsCertainTip = $treatPhpDocTypesAsCertainTip; + $this->fix(__DIR__ . '/data/useless-cast-fixable.php', __DIR__ . '/data/useless-cast-fixable.php.fixed'); + } + } diff --git a/tests/Rules/Cast/data/useless-cast-fixable.php b/tests/Rules/Cast/data/useless-cast-fixable.php new file mode 100644 index 0000000..4617dd4 --- /dev/null +++ b/tests/Rules/Cast/data/useless-cast-fixable.php @@ -0,0 +1,115 @@ + $phpDocArray + * @param stdClass $phpDocObject + * @param non-empty-string $string + */ +function casts( + bool $boolean, + int $integer, + float $float, + string $string, + array $array, + stdClass $object, + ?int $nullableInteger, + $phpDocBoolean, + $phpDocInteger, + $phpDocFloat, + $phpDocString, + $phpDocArray, + $phpDocObject, + $mixed +): void +{ + (bool) $boolean; + (int) $integer; + (float) $float; + (string) $string; + (array) $array; + (object) $object; + + (bool) $phpDocBoolean; + (int) $phpDocInteger; + (float) $phpDocFloat; + (string) $phpDocString; + (array) $phpDocArray; + (object) $phpDocObject; + + (string) $integer; + (int) $nullableInteger; + (int) $mixed; + + if ($nullableInteger !== null) { + (int) $nullableInteger; + } + if (is_int($mixed)) { + (int) $mixed; + } + + (int) 42; + (bool) true; + (string) 'value'; + (array) [1, 2]; + (object) new stdClass(); + (int) ($integer + 1) * 2; + 2 - (int) ($integer + 1); + (bool) ($boolean || true) && false; + (int) ($integer = 1) + 2; + (int) ($integer + 1) ** 2; + (int) ($boolean ? $integer : 0); + (int) ++$integer; + (int) strlen($string); + $b = $integer-(int)-$integer; +} + +class Foo +{ + public static function bug(string $key): void + { + $a = [$key => 1]; + + foreach ($a as $k => $v) { + $l = strlen((string)$k); // type cast is needed for auto-coerced non-decimal-int-string + } + } +} + +// https://3v4l.org/krTct#v +function doFoo() { + $array = [0, 1]; + foreach ((array) $array as &$v) { + $v=2; + } + var_dump($array); +} +function doBar() { + $array = [0, 1]; + foreach ($array as &$v) { + $v=2; + } + var_dump($array); +} +doFoo(); +doBar(); + +class X { + public int $i; + public string $s; +} + +function doBaz(X $x) { + echo (int) $x->i; + echo (int) $x->s; + + echo (string) $x->i; + echo (string) $x->s; +} diff --git a/tests/Rules/Cast/data/useless-cast-fixable.php.fixed b/tests/Rules/Cast/data/useless-cast-fixable.php.fixed new file mode 100644 index 0000000..9482f11 --- /dev/null +++ b/tests/Rules/Cast/data/useless-cast-fixable.php.fixed @@ -0,0 +1,115 @@ + $phpDocArray + * @param stdClass $phpDocObject + * @param non-empty-string $string + */ +function casts( + bool $boolean, + int $integer, + float $float, + string $string, + array $array, + stdClass $object, + ?int $nullableInteger, + $phpDocBoolean, + $phpDocInteger, + $phpDocFloat, + $phpDocString, + $phpDocArray, + $phpDocObject, + $mixed +): void +{ + $boolean; + $integer; + (float) $float; + (string) $string; + (array) $array; + (object) $object; + + (bool) $phpDocBoolean; + (int) $phpDocInteger; + (float) $phpDocFloat; + (string) $phpDocString; + (array) $phpDocArray; + (object) $phpDocObject; + + (string) $integer; + (int) $nullableInteger; + (int) $mixed; + + if ($nullableInteger !== null) { + $nullableInteger; + } + if (is_int($mixed)) { + $mixed; + } + + 42; + true; + (string) 'value'; + (array) [1, 2]; + (object) new stdClass(); + $integer + 1 * 2; + 2 - $integer + 1; + $boolean || true && false; + $integer = 1 + 2; + ($integer + 1) ** 2; + $boolean ? $integer : 0; + ++$integer; + strlen($string); + $b = $integer-(int)-$integer; +} + +class Foo +{ + public static function bug(string $key): void + { + $a = [$key => 1]; + + foreach ($a as $k => $v) { + $l = strlen((string)$k); // type cast is needed for auto-coerced non-decimal-int-string + } + } +} + +// https://3v4l.org/krTct#v +function doFoo() { + $array = [0, 1]; + foreach ((array) $array as &$v) { + $v=2; + } + var_dump($array); +} +function doBar() { + $array = [0, 1]; + foreach ($array as &$v) { + $v=2; + } + var_dump($array); +} +doFoo(); +doBar(); + +class X { + public int $i; + public string $s; +} + +function doBaz(X $x) { + echo $x->i; + echo (int) $x->s; + + echo (string) $x->i; + echo (string) $x->s; +}