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 ); }