From d225589576613a67d204775bb024718610f04705 Mon Sep 17 00:00:00 2001 From: Abdul Malik Ikhsan Date: Tue, 18 Jun 2024 19:41:10 +0700 Subject: [PATCH] [Rules] Skip Property fetch on NoIssetOnObjectRule --- config/extension.neon | 1 + src/Guard/EmptyIssetGuard.php | 37 +++++++++++++++++++ src/Rules/NoEmptyOnObjectRule.php | 36 ++++-------------- src/Rules/NoIssetOnObjectRule.php | 35 ++++-------------- .../Fixture/SkipIssetOnPropertyFetch.php | 27 ++++++++++++++ .../NoIssetOnObjectRuleTest.php | 1 + .../Rules/NoIssetOnObjectRule/Source/Foo.php | 13 +++++++ 7 files changed, 95 insertions(+), 55 deletions(-) create mode 100644 src/Guard/EmptyIssetGuard.php create mode 100644 tests/Rules/NoIssetOnObjectRule/Fixture/SkipIssetOnPropertyFetch.php create mode 100644 tests/Rules/NoIssetOnObjectRule/Source/Foo.php diff --git a/config/extension.neon b/config/extension.neon index 00d762aa..ff5f379e 100644 --- a/config/extension.neon +++ b/config/extension.neon @@ -54,6 +54,7 @@ services: - Rector\TypePerfect\Matcher\Collector\PublicClassMethodMatcher - Rector\TypePerfect\Matcher\ClassMethodCallReferenceResolver - Rector\TypePerfect\Printer\CollectorMetadataPrinter + - Rector\TypePerfect\Guard\EmptyIssetGuard # for NarrowPublicClassMethodParamTypeByCallerTypeRule - diff --git a/src/Guard/EmptyIssetGuard.php b/src/Guard/EmptyIssetGuard.php new file mode 100644 index 00000000..a80cbb6b --- /dev/null +++ b/src/Guard/EmptyIssetGuard.php @@ -0,0 +1,37 @@ +name instanceof Expr) { + return true; + } + + if (! $scope->hasVariableType($expr->name)->yes()) { + return true; + } + + $varType = $scope->getType($expr); + $varType = TypeCombinator::removeNull($varType); + return $varType->getObjectClassNames() === []; + } +} diff --git a/src/Rules/NoEmptyOnObjectRule.php b/src/Rules/NoEmptyOnObjectRule.php index 4f0f5441..7e40d732 100644 --- a/src/Rules/NoEmptyOnObjectRule.php +++ b/src/Rules/NoEmptyOnObjectRule.php @@ -5,24 +5,26 @@ namespace Rector\TypePerfect\Rules; use PhpParser\Node; -use PhpParser\Node\Expr; -use PhpParser\Node\Expr\ArrayDimFetch; use PhpParser\Node\Expr\Empty_; -use PhpParser\Node\Expr\Variable; use PHPStan\Analyser\Scope; use PHPStan\Rules\Rule; -use PHPStan\Type\TypeCombinator; +use Rector\TypePerfect\Guard\EmptyIssetGuard; /** * @implements Rule */ -final class NoEmptyOnObjectRule implements Rule +final readonly class NoEmptyOnObjectRule implements Rule { /** * @var string */ public const ERROR_MESSAGE = 'Use instanceof instead of empty() on object'; + public function __construct( + private EmptyIssetGuard $emptyIssetGuard + ) { + } + public function getNodeType(): string { return Empty_::class; @@ -34,8 +36,7 @@ public function getNodeType(): string */ public function processNode(Node $node, Scope $scope): array { - $expr = $node->expr; - if ($this->shouldSkipVariable($expr, $scope)) { + if ($this->emptyIssetGuard->isLegal($node->expr, $scope)) { return []; } @@ -43,25 +44,4 @@ public function processNode(Node $node, Scope $scope): array self::ERROR_MESSAGE, ]; } - - private function shouldSkipVariable(Expr $expr, Scope $scope): bool - { - if ($expr instanceof ArrayDimFetch) { - return true; - } - - if ($expr instanceof Variable) { - if ($expr->name instanceof Expr) { - return true; - } - - if (! $scope->hasVariableType($expr->name)->yes()) { - return true; - } - } - - $varType = $scope->getType($expr); - $varType = TypeCombinator::removeNull($varType); - return $varType->getObjectClassNames() === []; - } } diff --git a/src/Rules/NoIssetOnObjectRule.php b/src/Rules/NoIssetOnObjectRule.php index c397d178..e4b55c0e 100644 --- a/src/Rules/NoIssetOnObjectRule.php +++ b/src/Rules/NoIssetOnObjectRule.php @@ -5,25 +5,27 @@ namespace Rector\TypePerfect\Rules; use PhpParser\Node; -use PhpParser\Node\Expr; -use PhpParser\Node\Expr\ArrayDimFetch; use PhpParser\Node\Expr\Isset_; -use PhpParser\Node\Expr\Variable; use PHPStan\Analyser\Scope; use PHPStan\Rules\Rule; -use PHPStan\Type\TypeCombinator; +use Rector\TypePerfect\Guard\EmptyIssetGuard; /** * @see \Rector\TypePerfect\Tests\Rules\NoIssetOnObjectRule\NoIssetOnObjectRuleTest * @implements Rule */ -final class NoIssetOnObjectRule implements Rule +final readonly class NoIssetOnObjectRule implements Rule { /** * @var string */ public const ERROR_MESSAGE = 'Use instanceof instead of isset() on object'; + public function __construct( + private EmptyIssetGuard $emptyIssetGuard + ) { + } + public function getNodeType(): string { return Isset_::class; @@ -37,7 +39,7 @@ public function getNodeType(): string public function processNode(Node $node, Scope $scope): array { foreach ($node->vars as $var) { - if ($this->shouldSkipVariable($var, $scope)) { + if ($this->emptyIssetGuard->isLegal($var, $scope)) { continue; } @@ -48,25 +50,4 @@ public function processNode(Node $node, Scope $scope): array return []; } - - private function shouldSkipVariable(Expr $expr, Scope $scope): bool - { - if ($expr instanceof ArrayDimFetch) { - return true; - } - - if ($expr instanceof Variable) { - if ($expr->name instanceof Expr) { - return true; - } - - if (! $scope->hasVariableType($expr->name)->yes()) { - return true; - } - } - - $varType = $scope->getType($expr); - $varType = TypeCombinator::removeNull($varType); - return $varType->getObjectClassNames() === []; - } } diff --git a/tests/Rules/NoIssetOnObjectRule/Fixture/SkipIssetOnPropertyFetch.php b/tests/Rules/NoIssetOnObjectRule/Fixture/SkipIssetOnPropertyFetch.php new file mode 100644 index 00000000..66f1bd5c --- /dev/null +++ b/tests/Rules/NoIssetOnObjectRule/Fixture/SkipIssetOnPropertyFetch.php @@ -0,0 +1,27 @@ +initializeFoo(); + + $this->foo->sayHello(); + } + + private function initializeFoo(): void + { + if (!isset($this->foo)) + { + $this->foo = new Foo(); + } + } +} \ No newline at end of file diff --git a/tests/Rules/NoIssetOnObjectRule/NoIssetOnObjectRuleTest.php b/tests/Rules/NoIssetOnObjectRule/NoIssetOnObjectRuleTest.php index ee53cd0e..c99753b7 100644 --- a/tests/Rules/NoIssetOnObjectRule/NoIssetOnObjectRuleTest.php +++ b/tests/Rules/NoIssetOnObjectRule/NoIssetOnObjectRuleTest.php @@ -28,6 +28,7 @@ public static function provideData(): Iterator yield [__DIR__ . '/Fixture/SkipIssetOnArray.php', []]; yield [__DIR__ . '/Fixture/SkipIssetOnArrayNestedOnObject.php', []]; yield [__DIR__ . '/Fixture/SkipPossibleUndefinedVariable.php', []]; + yield [__DIR__ . '/Fixture/SkipIssetOnPropertyFetch.php', []]; } /** diff --git a/tests/Rules/NoIssetOnObjectRule/Source/Foo.php b/tests/Rules/NoIssetOnObjectRule/Source/Foo.php new file mode 100644 index 00000000..d69a712f --- /dev/null +++ b/tests/Rules/NoIssetOnObjectRule/Source/Foo.php @@ -0,0 +1,13 @@ +