From 26ab509d7a5092f3e67304163407c2c16bea9107 Mon Sep 17 00:00:00 2001 From: Abdul Malik Ikhsan Date: Mon, 19 Oct 2020 01:42:49 +0700 Subject: [PATCH] [CodeQuality] Register IssetOnPropertyObjectToPropertyExistsRector to code-quality config set (#4441) Co-authored-by: rector-bot --- config/set/code-quality.php | 3 ++ .../Foreach_/ForeachToInArrayRector.php | 2 +- ...OnPropertyObjectToPropertyExistsRector.php | 53 +++++++++++++++---- .../Fixture/property.php.inc | 6 +-- ...ter_instantiation_if_always_exists.php.inc | 39 ++++++++++++++ ..._maybe_exists_after_instantiation.php.inc} | 24 ++++++--- rules/symfony/src/ServiceMapProvider.php | 10 ++-- 7 files changed, 112 insertions(+), 25 deletions(-) create mode 100644 rules/code-quality/tests/Rector/Isset_/IssetOnPropertyObjectToPropertyExistsRector/Fixture/property_after_instantiation_if_always_exists.php.inc rename rules/code-quality/tests/Rector/Isset_/IssetOnPropertyObjectToPropertyExistsRector/Fixture/{property_after_instantiation.php.inc => property_maybe_exists_after_instantiation.php.inc} (72%) diff --git a/config/set/code-quality.php b/config/set/code-quality.php index 8fb1bdea4f5..3545b6f8c54 100644 --- a/config/set/code-quality.php +++ b/config/set/code-quality.php @@ -51,6 +51,7 @@ use Rector\CodeQuality\Rector\If_\SimplifyIfIssetToNullCoalescingRector; use Rector\CodeQuality\Rector\If_\SimplifyIfNotNullReturnRector; use Rector\CodeQuality\Rector\If_\SimplifyIfReturnBoolRector; use Rector\CodeQuality\Rector\Include_\AbsolutizeRequireAndIncludePathRector; +use Rector\CodeQuality\Rector\Isset_\IssetOnPropertyObjectToPropertyExistsRector; use Rector\CodeQuality\Rector\LogicalAnd\AndAssignsToSeparateLinesRector; use Rector\CodeQuality\Rector\LogicalAnd\LogicalToBooleanRector; use Rector\CodeQuality\Rector\Name\FixClassCaseSensitivityNameRector; @@ -219,4 +220,6 @@ return static function (ContainerConfigurator $containerConfigurator): void { $services->set(VarToPublicPropertyRector::class); $services->set(FixClassCaseSensitivityNameRector::class); + + $services->set(IssetOnPropertyObjectToPropertyExistsRector::class); }; diff --git a/rules/code-quality/src/Rector/Foreach_/ForeachToInArrayRector.php b/rules/code-quality/src/Rector/Foreach_/ForeachToInArrayRector.php index 551a6a61b39..4b1f0413c33 100644 --- a/rules/code-quality/src/Rector/Foreach_/ForeachToInArrayRector.php +++ b/rules/code-quality/src/Rector/Foreach_/ForeachToInArrayRector.php @@ -139,7 +139,7 @@ CODE_SAMPLE private function shouldSkipForeach(Foreach_ $foreach): bool { - if (isset($foreach->keyVar)) { + if ($foreach->keyVar !== null) { return true; } diff --git a/rules/code-quality/src/Rector/Isset_/IssetOnPropertyObjectToPropertyExistsRector.php b/rules/code-quality/src/Rector/Isset_/IssetOnPropertyObjectToPropertyExistsRector.php index b702c3e61a0..bb1aa2780ba 100644 --- a/rules/code-quality/src/Rector/Isset_/IssetOnPropertyObjectToPropertyExistsRector.php +++ b/rules/code-quality/src/Rector/Isset_/IssetOnPropertyObjectToPropertyExistsRector.php @@ -15,6 +15,9 @@ use PhpParser\Node\Expr\PropertyFetch; use PhpParser\Node\Identifier; use PhpParser\Node\Name; use PhpParser\Node\Scalar\String_; +use PHPStan\Analyser\Scope; +use PHPStan\Type\ObjectType; +use PHPStan\Type\ThisType; use Rector\Core\Rector\AbstractRector; use Rector\Core\RectorDefinition\CodeSample; use Rector\Core\RectorDefinition\RectorDefinition; @@ -79,29 +82,61 @@ CODE_SAMPLE } $previous = $issetVar->getAttribute(AttributeKey::PREVIOUS_NODE); - $current = $issetVar->getAttribute(AttributeKey::PARENT_NODE); $next = $issetVar->getAttribute(AttributeKey::NEXT_NODE); - if ($previous && $previous->getAttribute(AttributeKey::PARENT_NODE) === $current) { - continue; - } - - if ($next && $next->getAttribute(AttributeKey::PARENT_NODE) === $current) { + if ($this->isFoundInPreviuosOrNext($previous, $next, $node)) { continue; } /** @var Expr $object */ $object = $issetVar->var->getAttribute(AttributeKey::ORIGINAL_NODE); + + /** @var Scope $scope */ + $scope = $object->getAttribute(AttributeKey::SCOPE); + /** @var ThisType|ObjectType $type */ + $type = $scope->getType($object); + + if ($type instanceof ThisType) { + return new NotIdentical($issetVar, $this->createNull()); + } + /** @var Identifier $name */ $name = $issetVar->name; $property = $name->toString(); - $args = [new Arg($object), new Arg(new String_($property))]; - $propertyExistsFuncCall = new FuncCall(new Name('property_exists'), $args); + if ($type instanceof ObjectType) { + /** @var string $className */ + $className = $type->getClassName(); - return new BooleanAnd($propertyExistsFuncCall, new NotIdentical($issetVar, $this->createNull())); + $isPropertyAlwaysExists = property_exists($className, $property); + if ($isPropertyAlwaysExists) { + return new NotIdentical($issetVar, $this->createNull()); + } + } + + return $this->replaceToPropertyExistsWithNullCheck($object, $property, $issetVar); } return null; } + + /** + * @param Node $previous + * @param Node $next + */ + private function isFoundInPreviuosOrNext(?Node $previous = null, ?Node $next = null, Isset_ $isset): bool + { + if ($previous && $previous->getAttribute(AttributeKey::PARENT_NODE) === $isset) { + return true; + } + return $next && $next->getAttribute(AttributeKey::PARENT_NODE) === $isset; + } + + private function replaceToPropertyExistsWithNullCheck(Expr $expr, string $property, Expr $issetVar): BooleanAnd + { + $args = [new Arg($expr), new Arg(new String_($property))]; + $propertyExistsFuncCall = new FuncCall(new Name('property_exists'), $args); + + return new BooleanAnd($propertyExistsFuncCall, new NotIdentical($issetVar, $this->createNull())); + } } diff --git a/rules/code-quality/tests/Rector/Isset_/IssetOnPropertyObjectToPropertyExistsRector/Fixture/property.php.inc b/rules/code-quality/tests/Rector/Isset_/IssetOnPropertyObjectToPropertyExistsRector/Fixture/property.php.inc index 74e2e9e3bda..2e012793ea4 100644 --- a/rules/code-quality/tests/Rector/Isset_/IssetOnPropertyObjectToPropertyExistsRector/Fixture/property.php.inc +++ b/rules/code-quality/tests/Rector/Isset_/IssetOnPropertyObjectToPropertyExistsRector/Fixture/property.php.inc @@ -28,9 +28,9 @@ class SomeClass public function run() { - property_exists($this, 'x') && $this->x !== null; - property_exists($this, 'y') && $this->y !== null; - property_exists($this, 'x') && $this->x !== null && (property_exists($this, 'y') && $this->y !== null); + $this->x !== null; + $this->y !== null; + $this->x !== null && $this->y !== null; } } diff --git a/rules/code-quality/tests/Rector/Isset_/IssetOnPropertyObjectToPropertyExistsRector/Fixture/property_after_instantiation_if_always_exists.php.inc b/rules/code-quality/tests/Rector/Isset_/IssetOnPropertyObjectToPropertyExistsRector/Fixture/property_after_instantiation_if_always_exists.php.inc new file mode 100644 index 00000000000..536a8b022a4 --- /dev/null +++ b/rules/code-quality/tests/Rector/Isset_/IssetOnPropertyObjectToPropertyExistsRector/Fixture/property_after_instantiation_if_always_exists.php.inc @@ -0,0 +1,39 @@ +x); + isset($obj->y); + isset($obj->x) && isset($obj->y); + } +} + +?> +----- +x !== null; + $obj->y !== null; + $obj->x !== null && $obj->y !== null; + } +} + +?> diff --git a/rules/code-quality/tests/Rector/Isset_/IssetOnPropertyObjectToPropertyExistsRector/Fixture/property_after_instantiation.php.inc b/rules/code-quality/tests/Rector/Isset_/IssetOnPropertyObjectToPropertyExistsRector/Fixture/property_maybe_exists_after_instantiation.php.inc similarity index 72% rename from rules/code-quality/tests/Rector/Isset_/IssetOnPropertyObjectToPropertyExistsRector/Fixture/property_after_instantiation.php.inc rename to rules/code-quality/tests/Rector/Isset_/IssetOnPropertyObjectToPropertyExistsRector/Fixture/property_maybe_exists_after_instantiation.php.inc index 94397c183f0..1501b086a71 100644 --- a/rules/code-quality/tests/Rector/Isset_/IssetOnPropertyObjectToPropertyExistsRector/Fixture/property_after_instantiation.php.inc +++ b/rules/code-quality/tests/Rector/Isset_/IssetOnPropertyObjectToPropertyExistsRector/Fixture/property_maybe_exists_after_instantiation.php.inc @@ -2,14 +2,19 @@ namespace Rector\CodeQuality\Tests\Rector\Isset_\IssetOnPropertyObjectToPropertyExistsRector\Fixture; -class SomeClass2 +class SomeClass4 { - public $x; - public $y; + public function init() + { + $this->x = 'a'; + $this->y = 'b'; + } public function run() { $obj = new self(); + $obj->init(); + isset($obj->x); isset($obj->y); isset($obj->x) && isset($obj->y); @@ -22,18 +27,23 @@ class SomeClass2 namespace Rector\CodeQuality\Tests\Rector\Isset_\IssetOnPropertyObjectToPropertyExistsRector\Fixture; -class SomeClass2 +class SomeClass4 { - public $x; - public $y; + public function init() + { + $this->x = 'a'; + $this->y = 'b'; + } public function run() { $obj = new self(); + $obj->init(); + property_exists($obj, 'x') && $obj->x !== null; property_exists($obj, 'y') && $obj->y !== null; property_exists($obj, 'x') && $obj->x !== null && (property_exists($obj, 'y') && $obj->y !== null); } } -?> +?> \ No newline at end of file diff --git a/rules/symfony/src/ServiceMapProvider.php b/rules/symfony/src/ServiceMapProvider.php index 2dafcc0e694..437331fada5 100644 --- a/rules/symfony/src/ServiceMapProvider.php +++ b/rules/symfony/src/ServiceMapProvider.php @@ -81,7 +81,7 @@ final class ServiceMapProvider foreach ($xml->services->service as $def) { /** @var SimpleXMLElement $attrs */ $attrs = $def->attributes(); - if (! isset($attrs->id)) { + if (! (property_exists($attrs, 'id') && $attrs->id !== null)) { continue; } @@ -152,10 +152,10 @@ final class ServiceMapProvider return new ServiceDefinition( strpos((string) $attrs->id, '.') === 0 ? Strings::substring((string) $attrs->id, 1) : (string) $attrs->id, - isset($attrs->class) ? (string) $attrs->class : null, - ! isset($attrs->public) || (string) $attrs->public !== 'false', - isset($attrs->synthetic) && (string) $attrs->synthetic === 'true', - isset($attrs->alias) ? (string) $attrs->alias : null, + property_exists($attrs, 'class') && $attrs->class !== null ? (string) $attrs->class : null, + ! (property_exists($attrs, 'public') && $attrs->public !== null) || (string) $attrs->public !== 'false', + property_exists($attrs, 'synthetic') && $attrs->synthetic !== null && (string) $attrs->synthetic === 'true', + property_exists($attrs, 'alias') && $attrs->alias !== null ? (string) $attrs->alias : null, $tags ); }