diff --git a/rules-tests/TypeDeclaration/Rector/ClassMethod/BoolReturnTypeFromBooleanConstAndStrictReturnsRector/BoolReturnTypeFromBooleanConstAndStrictReturnsRectorTest.php b/rules-tests/TypeDeclaration/Rector/ClassMethod/BoolReturnTypeFromBooleanConstAndStrictReturnsRector/BoolReturnTypeFromBooleanConstAndStrictReturnsRectorTest.php new file mode 100644 index 00000000000..5962b3b6ca5 --- /dev/null +++ b/rules-tests/TypeDeclaration/Rector/ClassMethod/BoolReturnTypeFromBooleanConstAndStrictReturnsRector/BoolReturnTypeFromBooleanConstAndStrictReturnsRectorTest.php @@ -0,0 +1,28 @@ +doTestFile($filePath); + } + + public static function provideData(): Iterator + { + return self::yieldFilesFromDirectory(__DIR__ . '/Fixture'); + } + + public function provideConfigFilePath(): string + { + return __DIR__ . '/config/configured_rule.php'; + } +} diff --git a/rules-tests/TypeDeclaration/Rector/ClassMethod/BoolReturnTypeFromBooleanConstAndStrictReturnsRector/Fixture/mixed_const_and_strict.php.inc b/rules-tests/TypeDeclaration/Rector/ClassMethod/BoolReturnTypeFromBooleanConstAndStrictReturnsRector/Fixture/mixed_const_and_strict.php.inc new file mode 100644 index 00000000000..51ad316f000 --- /dev/null +++ b/rules-tests/TypeDeclaration/Rector/ClassMethod/BoolReturnTypeFromBooleanConstAndStrictReturnsRector/Fixture/mixed_const_and_strict.php.inc @@ -0,0 +1,35 @@ + 0; + } +} + +?> +----- + 0; + } +} + +?> diff --git a/rules-tests/TypeDeclaration/Rector/ClassMethod/BoolReturnTypeFromBooleanConstAndStrictReturnsRector/Fixture/mixed_multiple_returns.php.inc b/rules-tests/TypeDeclaration/Rector/ClassMethod/BoolReturnTypeFromBooleanConstAndStrictReturnsRector/Fixture/mixed_multiple_returns.php.inc new file mode 100644 index 00000000000..0df21abe5fa --- /dev/null +++ b/rules-tests/TypeDeclaration/Rector/ClassMethod/BoolReturnTypeFromBooleanConstAndStrictReturnsRector/Fixture/mixed_multiple_returns.php.inc @@ -0,0 +1,47 @@ +context)) { + return true; + } + + if ($this->context == $context) { + return true; + } + + return 0 === stripos($this->context, (string) $context); + } +} + +?> +----- +context)) { + return true; + } + + if ($this->context == $context) { + return true; + } + + return 0 === stripos($this->context, (string) $context); + } +} + +?> diff --git a/rules-tests/TypeDeclaration/Rector/ClassMethod/BoolReturnTypeFromBooleanConstAndStrictReturnsRector/Fixture/skip_non_bool_mix.php.inc b/rules-tests/TypeDeclaration/Rector/ClassMethod/BoolReturnTypeFromBooleanConstAndStrictReturnsRector/Fixture/skip_non_bool_mix.php.inc new file mode 100644 index 00000000000..890ffbfe246 --- /dev/null +++ b/rules-tests/TypeDeclaration/Rector/ClassMethod/BoolReturnTypeFromBooleanConstAndStrictReturnsRector/Fixture/skip_non_bool_mix.php.inc @@ -0,0 +1,15 @@ + $second) { + return $first === $second; + } + + return $first < $second; + } +} diff --git a/rules-tests/TypeDeclaration/Rector/ClassMethod/BoolReturnTypeFromBooleanConstAndStrictReturnsRector/config/configured_rule.php b/rules-tests/TypeDeclaration/Rector/ClassMethod/BoolReturnTypeFromBooleanConstAndStrictReturnsRector/config/configured_rule.php new file mode 100644 index 00000000000..395813096eb --- /dev/null +++ b/rules-tests/TypeDeclaration/Rector/ClassMethod/BoolReturnTypeFromBooleanConstAndStrictReturnsRector/config/configured_rule.php @@ -0,0 +1,9 @@ +withRules([BoolReturnTypeFromBooleanConstAndStrictReturnsRector::class]); diff --git a/rules/TypeDeclaration/Rector/ClassMethod/BoolReturnTypeFromBooleanConstAndStrictReturnsRector.php b/rules/TypeDeclaration/Rector/ClassMethod/BoolReturnTypeFromBooleanConstAndStrictReturnsRector.php new file mode 100644 index 00000000000..0aa0bcf9fa6 --- /dev/null +++ b/rules/TypeDeclaration/Rector/ClassMethod/BoolReturnTypeFromBooleanConstAndStrictReturnsRector.php @@ -0,0 +1,174 @@ + 0; + } +} +CODE_SAMPLE + + , + <<<'CODE_SAMPLE' +class SomeClass +{ + public function resolve($value): bool + { + if ($value === []) { + return true; + } + + return count($value) > 0; + } +} +CODE_SAMPLE + ), + ] + ); + } + + /** + * @return array> + */ + public function getNodeTypes(): array + { + return [ClassMethod::class, Function_::class]; + } + + /** + * @param ClassMethod|Function_ $node + */ + public function refactor(Node $node): ?Node + { + $scope = ScopeFetcher::fetch($node); + if ($this->shouldSkip($node, $scope)) { + return null; + } + + $returns = $this->betterNodeFinder->findReturnsScoped($node); + if (! $this->returnAnalyzer->hasOnlyReturnWithExpr($node, $returns)) { + return null; + } + + if (! $this->hasMixedBoolConstAndStrictReturns($returns)) { + return null; + } + + $node->returnType = new Identifier('bool'); + + return $node; + } + + public function provideMinPhpVersion(): int + { + return PhpVersionFeature::SCALAR_TYPES; + } + + /** + * @param ClassMethod|Function_|Closure $node + */ + private function shouldSkip(Node $node, Scope $scope): bool + { + // already has the type, skip + if ($node->returnType instanceof Node) { + return true; + } + + return $node instanceof ClassMethod + && $this->classMethodReturnTypeOverrideGuard->shouldSkipClassMethod($node, $scope); + } + + /** + * Every return must be bool, mixing at least one true/false const with at least one strict bool expression. + * Pure-const returns are handled by BoolReturnTypeFromBooleanConstReturnsRector, pure-strict by + * BoolReturnTypeFromBooleanStrictReturnsRector, so this rule stays disjoint from both. + * + * @param Return_[] $returns + */ + private function hasMixedBoolConstAndStrictReturns(array $returns): bool + { + $hasConstReturn = false; + $hasStrictReturn = false; + + foreach ($returns as $return) { + if (! $return->expr instanceof Expr) { + return false; + } + + if ($this->isBoolConst($return->expr)) { + $hasConstReturn = true; + continue; + } + + if ($this->isStrictBool($return->expr)) { + $hasStrictReturn = true; + continue; + } + + return false; + } + + return $hasConstReturn && $hasStrictReturn; + } + + private function isBoolConst(Expr $expr): bool + { + return $expr instanceof ConstFetch && $this->valueResolver->isTrueOrFalse($expr); + } + + private function isStrictBool(Expr $expr): bool + { + return $this->exprAnalyzer->isBoolExpr($expr) || $this->exprAnalyzer->isCallLikeReturnNativeBool($expr); + } +} diff --git a/src/Config/Level/TypeDeclarationLevel.php b/src/Config/Level/TypeDeclarationLevel.php index 1deb2643fcc..97f02a56430 100644 --- a/src/Config/Level/TypeDeclarationLevel.php +++ b/src/Config/Level/TypeDeclarationLevel.php @@ -30,6 +30,7 @@ use Rector\TypeDeclaration\Rector\ClassMethod\AddReturnTypeFromTryCatchTypeRector; use Rector\TypeDeclaration\Rector\ClassMethod\AddVoidReturnTypeWhereNoReturnRector; use Rector\TypeDeclaration\Rector\ClassMethod\ArrayParamTypeByMethodCallTypeRector; +use Rector\TypeDeclaration\Rector\ClassMethod\BoolReturnTypeFromBooleanConstAndStrictReturnsRector; use Rector\TypeDeclaration\Rector\ClassMethod\BoolReturnTypeFromBooleanConstReturnsRector; use Rector\TypeDeclaration\Rector\ClassMethod\BoolReturnTypeFromBooleanStrictReturnsRector; use Rector\TypeDeclaration\Rector\ClassMethod\KnownMagicClassMethodTypeRector; @@ -111,6 +112,7 @@ final class TypeDeclarationLevel NumericReturnTypeFromStrictScalarReturnsRector::class, BoolReturnTypeFromBooleanStrictReturnsRector::class, + BoolReturnTypeFromBooleanConstAndStrictReturnsRector::class, StringReturnTypeFromStrictStringReturnsRector::class, NumericReturnTypeFromStrictReturnsRector::class,