From 470885e4f1c366d769660471a60d25f64902cd87 Mon Sep 17 00:00:00 2001 From: someniatko Date: Tue, 12 Jul 2022 13:43:53 +0300 Subject: [PATCH 1/7] #8200 - improve inferring the "final" `static` type when calling static methods inside a different class differentiate between `static` defined in a class which CALLS a given static method, and `static` defined in the method which IS CALLED. --- .../ExistingAtomicStaticCallAnalyzer.php | 34 +++++++++++ tests/Template/Issue8200Test.php | 58 +++++++++++++++++++ 2 files changed, 92 insertions(+) create mode 100644 tests/Template/Issue8200Test.php diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/Call/StaticMethod/ExistingAtomicStaticCallAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/Call/StaticMethod/ExistingAtomicStaticCallAnalyzer.php index 7855200e95e..d8af9168d55 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/Call/StaticMethod/ExistingAtomicStaticCallAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Expression/Call/StaticMethod/ExistingAtomicStaticCallAnalyzer.php @@ -551,6 +551,18 @@ private static function getMethodReturnType( ) { $static_type = $context->self; $context_final = $codebase->classlike_storage_provider->get($context->self)->final; + } elseif ($context->calling_method_id !== null) { + $self_method_return = $codebase->methods->getMethodReturnType( + MethodIdentifier::fromMethodIdReference($context->calling_method_id), + $context->self + ); + // differentiate between these cases: + // 1. "static" in return type comes from return type of the + // method CALLING the currently analyzed static method - use $context->self. + // 2. "static" comes from the CALLED static method - use $fq_class_name. + $static_type = $self_method_return !== null && self::hasStaticInType($self_method_return) + ? $context->self + : $fq_class_name; } else { $static_type = $fq_class_name; } @@ -613,4 +625,26 @@ private static function getMethodReturnType( return $return_type_candidate; } + + /** + * Dumb way to determine whether a type contains "static" somewhere inside. + */ + private static function hasStaticInType(Union $union_type): bool + { + foreach ($union_type->getAtomicTypes() as $atomic_type) { + if ($atomic_type instanceof Atomic\TGenericObject) { + foreach ($atomic_type->type_params as $type_param) { + if (self::hasStaticInType($type_param)) { + return true; + } + } + } elseif ($atomic_type instanceof TNamedObject) { + if ($atomic_type->value === 'static') { + return true; + } + } + } + + return false; + } } diff --git a/tests/Template/Issue8200Test.php b/tests/Template/Issue8200Test.php new file mode 100644 index 00000000000..a20b5fa7507 --- /dev/null +++ b/tests/Template/Issue8200Test.php @@ -0,0 +1,58 @@ +,error_levels?:string[]}> + */ + public function providerValidCodeParse(): iterable + { + return [ + 'return TemplatedClass' => [ + ' + * + * @psalm-pure + */ + public static function just($value): self + { + return new self($value); + } + } + + abstract class Test + { + final private function __construct() {} + + /** @return Maybe */ + final public static function create(): Maybe + { + return Maybe::just(new static()); + } + }', + ], + ]; + } +} From 3a5054018bcc3a0708e53883fa0e4935a4ff6dad Mon Sep 17 00:00:00 2001 From: someniatko Date: Tue, 12 Jul 2022 21:00:19 +0300 Subject: [PATCH 2/7] #8200 - generalize ExistingAtomicStaticCallAnalyzer::hasStaticInType() for non-object cases --- .../ExistingAtomicStaticCallAnalyzer.php | 24 ++++++++------- tests/Template/Issue8200Test.php | 29 +++++++++++++++++++ 2 files changed, 43 insertions(+), 10 deletions(-) diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/Call/StaticMethod/ExistingAtomicStaticCallAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/Call/StaticMethod/ExistingAtomicStaticCallAnalyzer.php index d8af9168d55..4d13f11bd32 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/Call/StaticMethod/ExistingAtomicStaticCallAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Expression/Call/StaticMethod/ExistingAtomicStaticCallAnalyzer.php @@ -35,6 +35,7 @@ use Psalm\Type\Union; use function array_map; +use function assert; use function count; use function explode; use function in_array; @@ -629,17 +630,20 @@ private static function getMethodReturnType( /** * Dumb way to determine whether a type contains "static" somewhere inside. */ - private static function hasStaticInType(Union $union_type): bool + private static function hasStaticInType(Type\TypeNode $type): bool { - foreach ($union_type->getAtomicTypes() as $atomic_type) { - if ($atomic_type instanceof Atomic\TGenericObject) { - foreach ($atomic_type->type_params as $type_param) { - if (self::hasStaticInType($type_param)) { - return true; - } - } - } elseif ($atomic_type instanceof TNamedObject) { - if ($atomic_type->value === 'static') { + assert($type instanceof Atomic || $type instanceof Union); + $union_parts = $type instanceof Union + ? $type->getAtomicTypes() + : [ $type ]; + + foreach ($union_parts as $atomic_type) { + if ($atomic_type instanceof TNamedObject && $atomic_type->value === 'static') { + return true; + } + + foreach ($atomic_type->getChildNodes() as $child_type) { + if (self::hasStaticInType($child_type)) { return true; } } diff --git a/tests/Template/Issue8200Test.php b/tests/Template/Issue8200Test.php index a20b5fa7507..2e04bc1a51b 100644 --- a/tests/Template/Issue8200Test.php +++ b/tests/Template/Issue8200Test.php @@ -53,6 +53,35 @@ final public static function create(): Maybe } }', ], + 'return list' => [ + ' + * + * @psalm-pure + */ + public static function mklist($value): array + { + return [ $value ]; + } + } + + abstract class Test + { + final private function __construct() {} + + /** @return list */ + final public static function create(): array + { + return Lister::mklist(new static()); + } + }', + ] ]; } } From b3e673d7ec3dd2e07393ae08ee4e37a57f46c0d0 Mon Sep 17 00:00:00 2001 From: someniatko Date: Tue, 12 Jul 2022 21:17:10 +0300 Subject: [PATCH 3/7] #8200 - flip logic of determining "source" of `static` type in ExistingAtomicStaticCallAnalyzer::getMethodReturnType() --- .../ExistingAtomicStaticCallAnalyzer.php | 14 +++---- tests/Template/Issue8200Test.php | 38 +++++++++++++++++++ 2 files changed, 43 insertions(+), 9 deletions(-) diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/Call/StaticMethod/ExistingAtomicStaticCallAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/Call/StaticMethod/ExistingAtomicStaticCallAnalyzer.php index 4d13f11bd32..21484782e0b 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/Call/StaticMethod/ExistingAtomicStaticCallAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Expression/Call/StaticMethod/ExistingAtomicStaticCallAnalyzer.php @@ -553,17 +553,13 @@ private static function getMethodReturnType( $static_type = $context->self; $context_final = $codebase->classlike_storage_provider->get($context->self)->final; } elseif ($context->calling_method_id !== null) { - $self_method_return = $codebase->methods->getMethodReturnType( - MethodIdentifier::fromMethodIdReference($context->calling_method_id), - $context->self - ); // differentiate between these cases: - // 1. "static" in return type comes from return type of the + // 1. "static" comes from the CALLED static method - use $fq_class_name. + // 2. "static" in return type comes from return type of the // method CALLING the currently analyzed static method - use $context->self. - // 2. "static" comes from the CALLED static method - use $fq_class_name. - $static_type = $self_method_return !== null && self::hasStaticInType($self_method_return) - ? $context->self - : $fq_class_name; + $static_type = self::hasStaticInType($return_type_candidate) + ? $fq_class_name + : $context->self; } else { $static_type = $fq_class_name; } diff --git a/tests/Template/Issue8200Test.php b/tests/Template/Issue8200Test.php index 2e04bc1a51b..202466b9019 100644 --- a/tests/Template/Issue8200Test.php +++ b/tests/Template/Issue8200Test.php @@ -81,6 +81,44 @@ final public static function create(): array return Lister::mklist(new static()); } }', + ], + 'use TemplatedClass as an intermediate variable inside a method' => [ + ' + * + * @psalm-pure + */ + public static function just($value): self + { + return new self($value); + } + } + + abstract class Test + { + final private function __construct() {} + + final public static function create(): static + { + $maybe = Maybe::just(new static()); + return $maybe->value; + } + }', ] ]; } From ecbceb1d58736fe582949e2d2b7b198f529bc098 Mon Sep 17 00:00:00 2001 From: someniatko Date: Tue, 12 Jul 2022 21:38:23 +0300 Subject: [PATCH 4/7] #8200 - move Issue8200Test to ClassTemplateTest --- tests/Template/ClassTemplateTest.php | 105 ++++++++++++++++++++++ tests/Template/Issue8200Test.php | 125 --------------------------- 2 files changed, 105 insertions(+), 125 deletions(-) delete mode 100644 tests/Template/Issue8200Test.php diff --git a/tests/Template/ClassTemplateTest.php b/tests/Template/ClassTemplateTest.php index 62ee25a2380..9ea95e8d35b 100644 --- a/tests/Template/ClassTemplateTest.php +++ b/tests/Template/ClassTemplateTest.php @@ -4474,6 +4474,111 @@ class B {} ', 'error_message' => 'InvalidArgument', ], + 'return TemplatedClass' => [ + ' + * + * @psalm-pure + */ + public static function just($value): self + { + return new self($value); + } + } + + abstract class Test + { + final private function __construct() {} + + /** @return Maybe */ + final public static function create(): Maybe + { + return Maybe::just(new static()); + } + }', + ], + 'return list created in a static method of another class' => [ + ' + * + * @psalm-pure + */ + public static function mklist($value): array + { + return [ $value ]; + } + } + + abstract class Test + { + final private function __construct() {} + + /** @return list */ + final public static function create(): array + { + return Lister::mklist(new static()); + } + }', + ], + 'use TemplatedClass as an intermediate variable inside a method' => [ + ' + * + * @psalm-pure + */ + public static function just($value): self + { + return new self($value); + } + } + + abstract class Test + { + final private function __construct() {} + + final public static function create(): static + { + $maybe = Maybe::just(new static()); + return $maybe->value; + } + }', + ], ]; } } diff --git a/tests/Template/Issue8200Test.php b/tests/Template/Issue8200Test.php deleted file mode 100644 index 202466b9019..00000000000 --- a/tests/Template/Issue8200Test.php +++ /dev/null @@ -1,125 +0,0 @@ -,error_levels?:string[]}> - */ - public function providerValidCodeParse(): iterable - { - return [ - 'return TemplatedClass' => [ - ' - * - * @psalm-pure - */ - public static function just($value): self - { - return new self($value); - } - } - - abstract class Test - { - final private function __construct() {} - - /** @return Maybe */ - final public static function create(): Maybe - { - return Maybe::just(new static()); - } - }', - ], - 'return list' => [ - ' - * - * @psalm-pure - */ - public static function mklist($value): array - { - return [ $value ]; - } - } - - abstract class Test - { - final private function __construct() {} - - /** @return list */ - final public static function create(): array - { - return Lister::mklist(new static()); - } - }', - ], - 'use TemplatedClass as an intermediate variable inside a method' => [ - ' - * - * @psalm-pure - */ - public static function just($value): self - { - return new self($value); - } - } - - abstract class Test - { - final private function __construct() {} - - final public static function create(): static - { - $maybe = Maybe::just(new static()); - return $maybe->value; - } - }', - ] - ]; - } -} From 931b3bb18b3fa900cae5d51130ee8360ad4be58f Mon Sep 17 00:00:00 2001 From: someniatko Date: Tue, 12 Jul 2022 21:41:06 +0300 Subject: [PATCH 5/7] #8200 - simplify ExistingAtomicStaticCallAnalyzer::hasStaticType() --- .../ExistingAtomicStaticCallAnalyzer.php | 18 +++++------------- 1 file changed, 5 insertions(+), 13 deletions(-) diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/Call/StaticMethod/ExistingAtomicStaticCallAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/Call/StaticMethod/ExistingAtomicStaticCallAnalyzer.php index 21484782e0b..d7ac4ef16ff 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/Call/StaticMethod/ExistingAtomicStaticCallAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Expression/Call/StaticMethod/ExistingAtomicStaticCallAnalyzer.php @@ -35,7 +35,6 @@ use Psalm\Type\Union; use function array_map; -use function assert; use function count; use function explode; use function in_array; @@ -628,21 +627,14 @@ private static function getMethodReturnType( */ private static function hasStaticInType(Type\TypeNode $type): bool { - assert($type instanceof Atomic || $type instanceof Union); - $union_parts = $type instanceof Union - ? $type->getAtomicTypes() - : [ $type ]; + if ($type instanceof TNamedObject && $type->value === 'static') { + return true; + } - foreach ($union_parts as $atomic_type) { - if ($atomic_type instanceof TNamedObject && $atomic_type->value === 'static') { + foreach ($type->getChildNodes() as $child_type) { + if (self::hasStaticInType($child_type)) { return true; } - - foreach ($atomic_type->getChildNodes() as $child_type) { - if (self::hasStaticInType($child_type)) { - return true; - } - } } return false; From 21a6dd909669f1faf85f9be8607e252183e91bb2 Mon Sep 17 00:00:00 2001 From: someniatko Date: Tue, 12 Jul 2022 21:53:14 +0300 Subject: [PATCH 6/7] #8200 - move tests to the correct provider ("valid" instead of "invalid") --- tests/Template/ClassTemplateTest.php | 210 +++++++++++++-------------- 1 file changed, 105 insertions(+), 105 deletions(-) diff --git a/tests/Template/ClassTemplateTest.php b/tests/Template/ClassTemplateTest.php index 9ea95e8d35b..73a0fa9d811 100644 --- a/tests/Template/ClassTemplateTest.php +++ b/tests/Template/ClassTemplateTest.php @@ -3694,6 +3694,111 @@ protected function setUp(): void } }', ], + 'return TemplatedClass' => [ + ' + * + * @psalm-pure + */ + public static function just($value): self + { + return new self($value); + } + } + + abstract class Test + { + final private function __construct() {} + + /** @return Maybe */ + final public static function create(): Maybe + { + return Maybe::just(new static()); + } + }', + ], + 'return list created in a static method of another class' => [ + ' + * + * @psalm-pure + */ + public static function mklist($value): array + { + return [ $value ]; + } + } + + abstract class Test + { + final private function __construct() {} + + /** @return list */ + final public static function create(): array + { + return Lister::mklist(new static()); + } + }', + ], + 'use TemplatedClass as an intermediate variable inside a method' => [ + ' + * + * @psalm-pure + */ + public static function just($value): self + { + return new self($value); + } + } + + abstract class Test + { + final private function __construct() {} + + final public static function create(): static + { + $maybe = Maybe::just(new static()); + return $maybe->value; + } + }', + ], ]; } @@ -4474,111 +4579,6 @@ class B {} ', 'error_message' => 'InvalidArgument', ], - 'return TemplatedClass' => [ - ' - * - * @psalm-pure - */ - public static function just($value): self - { - return new self($value); - } - } - - abstract class Test - { - final private function __construct() {} - - /** @return Maybe */ - final public static function create(): Maybe - { - return Maybe::just(new static()); - } - }', - ], - 'return list created in a static method of another class' => [ - ' - * - * @psalm-pure - */ - public static function mklist($value): array - { - return [ $value ]; - } - } - - abstract class Test - { - final private function __construct() {} - - /** @return list */ - final public static function create(): array - { - return Lister::mklist(new static()); - } - }', - ], - 'use TemplatedClass as an intermediate variable inside a method' => [ - ' - * - * @psalm-pure - */ - public static function just($value): self - { - return new self($value); - } - } - - abstract class Test - { - final private function __construct() {} - - final public static function create(): static - { - $maybe = Maybe::just(new static()); - return $maybe->value; - } - }', - ], ]; } } From 1e0b57226494f7aa4608bf9e7e681c5976e85fba Mon Sep 17 00:00:00 2001 From: someniatko Date: Thu, 14 Jul 2022 10:03:47 +0300 Subject: [PATCH 7/7] #8200 - bikeshedding the tests --- tests/Template/ClassTemplateTest.php | 154 +++++++++++++-------------- 1 file changed, 77 insertions(+), 77 deletions(-) diff --git a/tests/Template/ClassTemplateTest.php b/tests/Template/ClassTemplateTest.php index 73a0fa9d811..413f13add1f 100644 --- a/tests/Template/ClassTemplateTest.php +++ b/tests/Template/ClassTemplateTest.php @@ -3697,107 +3697,107 @@ protected function setUp(): void 'return TemplatedClass' => [ ' - * - * @psalm-pure + * @template-covariant A + * @psalm-immutable */ - public static function just($value): self + final class Maybe { - return new self($value); - } - } + /** + * @param null|A $value + */ + public function __construct(private $value = null) {} - abstract class Test - { - final private function __construct() {} + /** + * @template B + * @param B $value + * @return Maybe + * + * @psalm-pure + */ + public static function just($value): self + { + return new self($value); + } + } - /** @return Maybe */ - final public static function create(): Maybe + abstract class Test { - return Maybe::just(new static()); - } - }', + final private function __construct() {} + + /** @return Maybe */ + final public static function create(): Maybe + { + return Maybe::just(new static()); + } + }', ], 'return list created in a static method of another class' => [ ' - * - * @psalm-pure - */ - public static function mklist($value): array + final class Lister { - return [ $value ]; + /** + * @template B + * @param B $value + * @return list + * + * @psalm-pure + */ + public static function mklist($value): array + { + return [ $value ]; + } } - } - - abstract class Test - { - final private function __construct() {} - /** @return list */ - final public static function create(): array + abstract class Test { - return Lister::mklist(new static()); - } - }', + final private function __construct() {} + + /** @return list */ + final public static function create(): array + { + return Lister::mklist(new static()); + } + }', ], 'use TemplatedClass as an intermediate variable inside a method' => [ ' - * - * @psalm-pure + * @template-covariant A + * @psalm-immutable */ - public static function just($value): self + final class Maybe { - return new self($value); - } - } + /** + * @param A $value + */ + public function __construct(public $value) {} - abstract class Test - { - final private function __construct() {} + /** + * @template B + * @param B $value + * @return Maybe + * + * @psalm-pure + */ + public static function just($value): self + { + return new self($value); + } + } - final public static function create(): static + abstract class Test { - $maybe = Maybe::just(new static()); - return $maybe->value; - } - }', + final private function __construct() {} + + final public static function create(): static + { + $maybe = Maybe::just(new static()); + return $maybe->value; + } + }', ], ]; }