-
Notifications
You must be signed in to change notification settings - Fork 509
Make ParametersAcceptorSelector::selectSingle() return union-ed conditional types #1159
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 7 commits
af88720
a71c977
28342ca
83b829a
fe8cae8
1337434
0c7fd11
5afa64d
7204844
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -8,10 +8,11 @@ | |
use PHPStan\Type\Generic\TemplateTypeMap; | ||
use PHPStan\Type\Type; | ||
use PHPStan\Type\TypeTraverser; | ||
use PHPStan\Type\TypeUtils; | ||
use function array_key_exists; | ||
use function array_map; | ||
|
||
class ResolvedFunctionVariant implements ParametersAcceptor | ||
class ResolvedFunctionVariant implements ParametersAcceptor, SingleParametersAcceptor | ||
{ | ||
|
||
/** @var ParameterReflection[]|null */ | ||
|
@@ -88,6 +89,19 @@ public function getReturnType(): Type | |
return $type; | ||
} | ||
|
||
/** | ||
* @return static | ||
*/ | ||
public function flattenConditionalsInReturnType(): self | ||
{ | ||
$returnType = $this->getReturnType(); | ||
|
||
$result = clone $this; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I dislike |
||
$result->returnType = TypeUtils::flattenConditionals($returnType); | ||
|
||
return $result; | ||
} | ||
|
||
private function resolveConditionalTypes(Type $type): Type | ||
{ | ||
return TypeTraverser::map($type, function (Type $type, callable $traverse): Type { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,13 @@ | ||
<?php declare(strict_types = 1); | ||
|
||
namespace PHPStan\Reflection; | ||
|
||
interface SingleParametersAcceptor | ||
{ | ||
|
||
/** | ||
* @return static | ||
*/ | ||
public function flattenConditionalsInReturnType(): self; | ||
|
||
} |
Original file line number | Diff line number | Diff line change | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,20 @@ | ||||||||||||||
<?php | ||||||||||||||
|
||||||||||||||
namespace DynamicMethodReturnGetSingleConditional; | ||||||||||||||
|
||||||||||||||
use function PHPStan\Testing\assertType; | ||||||||||||||
|
||||||||||||||
abstract class Foo | ||||||||||||||
{ | ||||||||||||||
/** | ||||||||||||||
* @return ($input is 1 ? true : false) | ||||||||||||||
*/ | ||||||||||||||
abstract public function get(int $input): mixed; | ||||||||||||||
|
||||||||||||||
public function doFoo(): void | ||||||||||||||
{ | ||||||||||||||
assertType('bool', $this->get(0)); | ||||||||||||||
assertType('bool', $this->get(1)); | ||||||||||||||
assertType('bool', $this->get(2)); | ||||||||||||||
Comment on lines
+16
to
+18
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. do we expect a more precise type here?
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No - there is return type extension that just returns |
||||||||||||||
} | ||||||||||||||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I dislike
clone
, please create new instance explicitly withnew self
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, that's disappointing. Given that these classes are marked
@api
and the constructor isn't final, constructing an instance ofstatic
seems impossible.Or would you accept a version that returns
$this
ifself::class !== static::class
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Give me a moment, I'll think of something 😊
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, these classes should ideally be
final
, there's no reason to extend them. I've marked a new todo for 2.0 when the time comes - class has either to be@api
or final (it can be both).There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we can mark them
@final
in the meantime?