From 4a273221443fb7d9bdf806fe2e48b323e3a06777 Mon Sep 17 00:00:00 2001 From: Vincent Langlet Date: Thu, 3 Nov 2022 11:25:24 +0100 Subject: [PATCH 1/5] serialize is not pure for array of object --- src/Psalm/Internal/Codebase/Functions.php | 8 ++--- .../TypeVisitor/ContainsObjectTypeVisitor.php | 36 +++++++++++++++++++ src/Psalm/Type/Union.php | 10 ++++++ tests/UnusedCodeTest.php | 1 + 4 files changed, 49 insertions(+), 6 deletions(-) create mode 100644 src/Psalm/Internal/TypeVisitor/ContainsObjectTypeVisitor.php diff --git a/src/Psalm/Internal/Codebase/Functions.php b/src/Psalm/Internal/Codebase/Functions.php index eb73c2932be..54e1bdca1f0 100644 --- a/src/Psalm/Internal/Codebase/Functions.php +++ b/src/Psalm/Internal/Codebase/Functions.php @@ -530,12 +530,8 @@ public function isCallMapFunctionPure( if ($function_id === 'serialize' && isset($args[0]) && $type_provider) { $serialize_type = $type_provider->getType($args[0]->value); - if ($serialize_type) { - foreach ($serialize_type->getAtomicTypes() as $atomic_serialize_type) { - if ($atomic_serialize_type->isObjectType()) { - return false; - } - } + if ($serialize_type && $serialize_type->containsObjectType()) { + return false; } } diff --git a/src/Psalm/Internal/TypeVisitor/ContainsObjectTypeVisitor.php b/src/Psalm/Internal/TypeVisitor/ContainsObjectTypeVisitor.php new file mode 100644 index 00000000000..5375bcbfed7 --- /dev/null +++ b/src/Psalm/Internal/TypeVisitor/ContainsObjectTypeVisitor.php @@ -0,0 +1,36 @@ +as->hasObjectType()) + ) { + $this->contains_object_type = true; + return NodeVisitor::STOP_TRAVERSAL; + } + + return null; + } + + public function matches(): bool + { + return $this->contains_object_type; + } +} diff --git a/src/Psalm/Type/Union.php b/src/Psalm/Type/Union.php index 6ff34be3156..6a82d192e9e 100644 --- a/src/Psalm/Type/Union.php +++ b/src/Psalm/Type/Union.php @@ -9,6 +9,7 @@ use Psalm\Internal\Type\TypeCombiner; use Psalm\Internal\TypeVisitor\ContainsClassLikeVisitor; use Psalm\Internal\TypeVisitor\ContainsLiteralVisitor; +use Psalm\Internal\TypeVisitor\ContainsObjectTypeVisitor; use Psalm\Internal\TypeVisitor\FromDocblockSetter; use Psalm\Internal\TypeVisitor\TemplateTypeCollector; use Psalm\Internal\TypeVisitor\TypeChecker; @@ -1492,6 +1493,15 @@ public function containsAnyLiteral(): bool return $literal_visitor->matches(); } + public function containsObjectType(): bool + { + $object_type_visitor = new ContainsObjectTypeVisitor(); + + $object_type_visitor->traverseArray($this->types); + + return $object_type_visitor->matches(); + } + /** * @return list */ diff --git a/tests/UnusedCodeTest.php b/tests/UnusedCodeTest.php index 373ad1cf72b..e0e1baec322 100644 --- a/tests/UnusedCodeTest.php +++ b/tests/UnusedCodeTest.php @@ -754,6 +754,7 @@ public function __wakeup(): void function test(): bool { try { serialize(new Foo()); + serialize([new Foo()]); unserialize(""); } catch (\Throwable) { return false; From b2f095f1b8da753f42284957d885c38c8691a3e9 Mon Sep 17 00:00:00 2001 From: Vincent Langlet Date: Thu, 3 Nov 2022 14:39:48 +0100 Subject: [PATCH 2/5] Add tests --- tests/UnusedCodeTest.php | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/UnusedCodeTest.php b/tests/UnusedCodeTest.php index e0e1baec322..e692b2c5952 100644 --- a/tests/UnusedCodeTest.php +++ b/tests/UnusedCodeTest.php @@ -751,10 +751,12 @@ public function __wakeup(): void } } - function test(): bool { + function test(Foo|int $foo): bool { try { serialize(new Foo()); serialize([new Foo()]); + serialize([[new Foo()]]); + serialize($foo); unserialize(""); } catch (\Throwable) { return false; From dc977964c6cf97a5eeac15c092b112b2d37bca9e Mon Sep 17 00:00:00 2001 From: Vincent Langlet Date: Thu, 3 Nov 2022 16:05:40 +0100 Subject: [PATCH 3/5] Refacto --- src/Psalm/Internal/Codebase/Functions.php | 2 +- .../CanContainObjectTypeVisitor.php | 51 +++++++++++++++++++ .../TypeVisitor/ContainsObjectTypeVisitor.php | 36 ------------- src/Psalm/Type/Union.php | 6 +-- 4 files changed, 55 insertions(+), 40 deletions(-) create mode 100644 src/Psalm/Internal/TypeVisitor/CanContainObjectTypeVisitor.php delete mode 100644 src/Psalm/Internal/TypeVisitor/ContainsObjectTypeVisitor.php diff --git a/src/Psalm/Internal/Codebase/Functions.php b/src/Psalm/Internal/Codebase/Functions.php index 54e1bdca1f0..daba79560a6 100644 --- a/src/Psalm/Internal/Codebase/Functions.php +++ b/src/Psalm/Internal/Codebase/Functions.php @@ -530,7 +530,7 @@ public function isCallMapFunctionPure( if ($function_id === 'serialize' && isset($args[0]) && $type_provider) { $serialize_type = $type_provider->getType($args[0]->value); - if ($serialize_type && $serialize_type->containsObjectType()) { + if ($serialize_type && $serialize_type->canContainObjectType($codebase)) { return false; } } diff --git a/src/Psalm/Internal/TypeVisitor/CanContainObjectTypeVisitor.php b/src/Psalm/Internal/TypeVisitor/CanContainObjectTypeVisitor.php new file mode 100644 index 00000000000..91feb240b00 --- /dev/null +++ b/src/Psalm/Internal/TypeVisitor/CanContainObjectTypeVisitor.php @@ -0,0 +1,51 @@ +codebase = $codebase; + } + + protected function enterNode(TypeNode $type): ?int + { + if ( + $type instanceof Union + && UnionTypeComparator::canBeContainedBy($this->codebase, $type, new Union([new TObject()])) + || + $type instanceof Atomic + && AtomicTypeComparator::isContainedBy($this->codebase, $type, new TObject()) + ) { + $this->contains_object_type = true; + return NodeVisitor::STOP_TRAVERSAL; + } + + return null; + } + + public function matches(): bool + { + return $this->contains_object_type; + } +} diff --git a/src/Psalm/Internal/TypeVisitor/ContainsObjectTypeVisitor.php b/src/Psalm/Internal/TypeVisitor/ContainsObjectTypeVisitor.php deleted file mode 100644 index 5375bcbfed7..00000000000 --- a/src/Psalm/Internal/TypeVisitor/ContainsObjectTypeVisitor.php +++ /dev/null @@ -1,36 +0,0 @@ -as->hasObjectType()) - ) { - $this->contains_object_type = true; - return NodeVisitor::STOP_TRAVERSAL; - } - - return null; - } - - public function matches(): bool - { - return $this->contains_object_type; - } -} diff --git a/src/Psalm/Type/Union.php b/src/Psalm/Type/Union.php index 6a82d192e9e..d785043dbf4 100644 --- a/src/Psalm/Type/Union.php +++ b/src/Psalm/Type/Union.php @@ -9,7 +9,7 @@ use Psalm\Internal\Type\TypeCombiner; use Psalm\Internal\TypeVisitor\ContainsClassLikeVisitor; use Psalm\Internal\TypeVisitor\ContainsLiteralVisitor; -use Psalm\Internal\TypeVisitor\ContainsObjectTypeVisitor; +use Psalm\Internal\TypeVisitor\CanContainObjectTypeVisitor; use Psalm\Internal\TypeVisitor\FromDocblockSetter; use Psalm\Internal\TypeVisitor\TemplateTypeCollector; use Psalm\Internal\TypeVisitor\TypeChecker; @@ -1493,9 +1493,9 @@ public function containsAnyLiteral(): bool return $literal_visitor->matches(); } - public function containsObjectType(): bool + public function canContainObjectType(Codebase $codebase): bool { - $object_type_visitor = new ContainsObjectTypeVisitor(); + $object_type_visitor = new CanContainObjectTypeVisitor($codebase); $object_type_visitor->traverseArray($this->types); From fbaf6afb305235a0d08de8f38293023c7c824a1a Mon Sep 17 00:00:00 2001 From: Vincent Langlet Date: Thu, 3 Nov 2022 18:11:01 +0100 Subject: [PATCH 4/5] Try --- .../TypeVisitor/CanContainObjectTypeVisitor.php | 10 ++++++++-- tests/UnusedCodeTest.php | 4 +++- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/src/Psalm/Internal/TypeVisitor/CanContainObjectTypeVisitor.php b/src/Psalm/Internal/TypeVisitor/CanContainObjectTypeVisitor.php index 91feb240b00..ba88c86fc77 100644 --- a/src/Psalm/Internal/TypeVisitor/CanContainObjectTypeVisitor.php +++ b/src/Psalm/Internal/TypeVisitor/CanContainObjectTypeVisitor.php @@ -32,10 +32,16 @@ protected function enterNode(TypeNode $type): ?int { if ( $type instanceof Union - && UnionTypeComparator::canBeContainedBy($this->codebase, $type, new Union([new TObject()])) + && ( + UnionTypeComparator::canBeContainedBy($this->codebase, new Union([new TObject()]), $type) + && UnionTypeComparator::canBeContainedBy($this->codebase, $type, new Union([new TObject()])) + ) || $type instanceof Atomic - && AtomicTypeComparator::isContainedBy($this->codebase, $type, new TObject()) + && ( + AtomicTypeComparator::isContainedBy($this->codebase, new TObject(), $type) + || AtomicTypeComparator::isContainedBy($this->codebase, $type, new TObject()) + ) ) { $this->contains_object_type = true; return NodeVisitor::STOP_TRAVERSAL; diff --git a/tests/UnusedCodeTest.php b/tests/UnusedCodeTest.php index e692b2c5952..0c46ed635d0 100644 --- a/tests/UnusedCodeTest.php +++ b/tests/UnusedCodeTest.php @@ -751,12 +751,14 @@ public function __wakeup(): void } } - function test(Foo|int $foo): bool { + function test(Foo|int $foo, mixed $bar, iterable $baz): bool { try { serialize(new Foo()); serialize([new Foo()]); serialize([[new Foo()]]); serialize($foo); + serialize($bar); + serialize($baz); unserialize(""); } catch (\Throwable) { return false; From c02ed9da0a2f5c0969a7fe06b9b9005d03a819c9 Mon Sep 17 00:00:00 2001 From: Vincent Langlet Date: Thu, 3 Nov 2022 20:24:15 +0100 Subject: [PATCH 5/5] Fix cs --- src/Psalm/Internal/TypeVisitor/CanContainObjectTypeVisitor.php | 3 +-- src/Psalm/Type/Union.php | 2 +- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/src/Psalm/Internal/TypeVisitor/CanContainObjectTypeVisitor.php b/src/Psalm/Internal/TypeVisitor/CanContainObjectTypeVisitor.php index ba88c86fc77..460a723a474 100644 --- a/src/Psalm/Internal/TypeVisitor/CanContainObjectTypeVisitor.php +++ b/src/Psalm/Internal/TypeVisitor/CanContainObjectTypeVisitor.php @@ -30,8 +30,7 @@ public function __construct(Codebase $codebase) protected function enterNode(TypeNode $type): ?int { - if ( - $type instanceof Union + if ($type instanceof Union && ( UnionTypeComparator::canBeContainedBy($this->codebase, new Union([new TObject()]), $type) && UnionTypeComparator::canBeContainedBy($this->codebase, $type, new Union([new TObject()])) diff --git a/src/Psalm/Type/Union.php b/src/Psalm/Type/Union.php index d785043dbf4..d7ff6557bb4 100644 --- a/src/Psalm/Type/Union.php +++ b/src/Psalm/Type/Union.php @@ -7,9 +7,9 @@ use Psalm\Codebase; use Psalm\Internal\DataFlow\DataFlowNode; use Psalm\Internal\Type\TypeCombiner; +use Psalm\Internal\TypeVisitor\CanContainObjectTypeVisitor; use Psalm\Internal\TypeVisitor\ContainsClassLikeVisitor; use Psalm\Internal\TypeVisitor\ContainsLiteralVisitor; -use Psalm\Internal\TypeVisitor\CanContainObjectTypeVisitor; use Psalm\Internal\TypeVisitor\FromDocblockSetter; use Psalm\Internal\TypeVisitor\TemplateTypeCollector; use Psalm\Internal\TypeVisitor\TypeChecker;