Add support for conditional return type

[greg0ire]

Feature request

This is a follow up of #3849 , and of doctrine/dbal#4264

I tried your suggestion, and it seems the root cause is conditional return types are not supported: https://phpstan.org/r/bdef5abe-d824-4311-aa6e-e0cce31ede92

PHPDoc tag @phpstan-return has invalid value (($params is array{wrapperClass:mixed} ? T : Connection)): Unexpected token "$params", expected type at offset 82

Cc @pkruithof @morozov

[greg0ire]

As a side note, here is my unsuccessful attempt at keeping the annotations for Psalm, and letting PHPStan fallback on a simpler version: https://phpstan.org/r/02c85102-05d0-410f-a76f-037bd4717ed3

I think that's because of the @psalm-template annotation, which PHPStan needs to resolve.

[dktapps]

This would also be nice for dynamic return type extensions (some of the native ones would become obsolete, e.g microtime() would be able to have $get_as_float is true ? float : string or something like that).

[ondrejmirtes]

FYI the annoying message about "unable to resolve template type" will no longer show up in these cases (because the template type is not present in the return type used by PHPStan): https://phpstan.org/r/3273047f-31d7-42bb-8763-357e10553eb5

Thanks to commit: phpstan/phpstan-src@a5f9f0c

[voku]

Can we dynamic initialize "dynamic return type extensions" for conditional return types?

[ondrejmirtes]

@voku Your question is expressed in a weird way but dynamic return type extension is what you typically implement today to achieve conditional types.

[voku]

@voku Your question is expressed in a weird way but dynamic return type extension is what you typically implement today to achieve conditional types.

Sorry, yes "dynamic dynamic" 🙏 is not the best way to ask a question about dynamic initialization. 😇

I wanted to know if its possible to use the current extension code for dynamic return types via phpdoc instead of added more classes.

It would be much more easy to share this with others because they do not need to add the extra class, do not change the phpstan config and it would work out of the box.

[ondrejmirtes]

That's what this open feature request asks for, it's on my roadmap to implement this.

[phpstan-bot]

@greg0ire PHPStan now reports different result with your code snippet:

@@ @@
-13: PHPDoc tag @phpstan-return has invalid value (($params is array{wrapperClass:mixed} ? T : Connection)): Unexpected token "$params", expected type at offset 82
-18: Unable to resolve the template type T in call to method static method DriverManager::getConnection()
+13: PHPDoc tag @phpstan-return has invalid value (($params is array{wrapperClass:mixed} ? T : Connection)): Unexpected token "$params", expected type at offset 82

Full report

Line	Error
13	PHPDoc tag @phpstan-return has invalid value (($params is array{wrapperClass:mixed} ? T : Connection)): Unexpected token "$params", expected type at offset 82

[phpstan-bot]

@greg0ire PHPStan now reports different result with your code snippet:

@@ @@
-21: Unable to resolve the template type T in call to method static method DriverManager::getConnection()
+No errors

[greg0ire]

Awesome!

[ondrejmirtes]

Conditional types aren't yet implemented. The "unable to resolve" error disappeared because in PHPStan-read @return tag T is not present. I've talked about it in a previous comment: #3853 (comment)

[greg0ire]

Oh right, the message did not appear before because phpstan-bot was not implemented when the the release containing what you commented on was created? Anyway, that bot is great!

[ondrejmirtes]

Exactly :) Thank you!

[phpstan-bot]

@greg0ire After the latest commit in dev-master, PHPStan now reports different result with your code snippet:

@@ @@
-21: Unable to resolve the template type T in call to method static method DriverManager::getConnection()
+16: Template type T of method DriverManager::getConnection() is not referenced in a parameter.

Full report

Line	Error
16	Template type T of method DriverManager::getConnection() is not referenced in a parameter.

[phpstan-bot]

@ondrejmirtes After the latest commit in dev-master, PHPStan now reports different result with your code snippet:

@@ @@
-No errors
+16: Template type T of method DriverManager::getConnection() is not referenced in a parameter.

Full report

Line	Error
16	Template type T of method DriverManager::getConnection() is not referenced in a parameter.

[phpstan-bot]

@greg0ire After the latest commit in 1.6.x, PHPStan now reports different result with your code snippet:

@@ @@
-13: PHPDoc tag @phpstan-return has invalid value (($params is array{wrapperClass:mixed} ? T : Connection)): Unexpected token "$params", expected type at offset 82
 18: Unable to resolve the template type T in call to method static method DriverManager::getConnection()

Full report

Line	Error
18	Unable to resolve the template type T in call to method static method DriverManager::getConnection()

[ondrejmirtes]

About:

 Identify which dynamic return type extensions in PHPStan could be replaced with a stub that has conditional return type

I looked into some and maybe there won't be too many. Sometimes a function looks like a good candidate, like current, but then I saw that the parameter is array|object, and I don't want to have "unable to resolve template type" error when someone calls current($object).

But for example it looks like it could work for HrtimeFunctionReturnTypeExtension. But there's also a catch - the extension currently returns a "benevolent union type" - we have a special syntax that works in PHPDocs for that - it's __benevolent<Foo|Bar>, but again, it's not going to be as nice as I expected.

Also MicrotimeFunctionReturnTypeExtension is a good candidate.

[staabm]

Identify which dynamic return type extensions in PHPStan could be replaced with a stub that has conditional return type

an alternative approach could be checking the conditional types psalm defines and phpstan does not have a Extension for, so we could be inspired by their types

[ondrejmirtes]

Feel free to go through them and submit them here to 1.6.x branch along with some tests, that would be helpful!

[staabm]

Feel free to go through them and submit them here to 1.6.x branch along with some tests, that would be helpful!

I started doing so. will send PRs as i see fit.

most interessting observation atm regarding conditional types: I did not yet find a case, where the conditional types are "useful" for phpstan-core.

all I scanned right now are either already implemented as an Extension (and phpstan is most of the time more precise then the psalm conditional types), or can be aliased on existing logic.

I am sure though, that the conditional types are usefull for libraries and application code, for cases one does not want to go full into phpstan internals/extensions right now.

---

here a list of "special" cases psalm conditional types understand.. I don't say we should support them, but as I work thru the list right now, I will give you a summary:

• accessory types
  https://github.com/vimeo/psalm/blob/c209c66263ab24b04e5034d7160befc55af8d610/stubs/CoreGenericFunctions.phpstub#L10

• PHP_MAJOR_VERSION constant Recognize constants as constant types phpstan-src#1163
  https://github.com/vimeo/psalm/blob/c209c66263ab24b04e5034d7160befc55af8d610/stubs/CoreGenericFunctions.phpstub#L67

• a empty -type?
  https://github.com/vimeo/psalm/blob/c209c66263ab24b04e5034d7160befc55af8d610/stubs/CoreGenericFunctions.phpstub#L139

• complex conditionals
  https://github.com/vimeo/psalm/blob/c209c66263ab24b04e5034d7160befc55af8d610/stubs/CoreGenericFunctions.phpstub#L438-L454

• comparisons against literal strings
  https://github.com/vimeo/psalm/blob/c209c66263ab24b04e5034d7160befc55af8d610/stubs/CoreGenericFunctions.phpstub#L461-L465

• func_num_args "macro"
  https://github.com/vimeo/psalm/blob/c209c66263ab24b04e5034d7160befc55af8d610/stubs/CoreGenericFunctions.phpstub#L475

• accessory types, phpstan-core doesn't know atm
  https://github.com/vimeo/psalm/blob/c209c66263ab24b04e5034d7160befc55af8d610/stubs/CoreGenericFunctions.phpstub#L511-L515

• bitmask logic
  https://github.com/vimeo/psalm/blob/c209c66263ab24b04e5034d7160befc55af8d610/stubs/CoreGenericFunctions.phpstub#L627

• references to/comparison of generic types
  https://github.com/vimeo/psalm/blob/c209c66263ab24b04e5034d7160befc55af8d610/stubs/CoreGenericFunctions.phpstub#L978-L981
  https://github.com/vimeo/psalm/blob/c209c66263ab24b04e5034d7160befc55af8d610/stubs/CoreGenericFunctions.phpstub#L1090-L1095

• NULL type comparison
  https://github.com/vimeo/psalm/blob/c209c66263ab24b04e5034d7160befc55af8d610/stubs/CoreGenericFunctions.phpstub#L1346

I will update this list, in case I find more

---

there is also a news article with some references to builtin methods utilizing conditional return types

[ondrejmirtes]

I'll look into this myself:

What to do about conditional return type when inheriting PHPDoc? Parameter name needs to be remapped based on position.

[phpstan-bot]

@ondrejmirtes After the latest commit in 1.6.x, PHPStan now reports different result with your code snippet:

@@ @@
-18: Unable to resolve the template type T in call to method static method DriverManager::getConnection()
+No errors

[ondrejmirtes]

I'm gonna do these two next:

Test that OverridingMethodRuleTest works with conditional type as expected (verify covariance with parent method)
Test that IncompatiblePhpDocTypeRule works with conditional type as expected

[ondrejmirtes]

Tomorrow I want to work on this one:

New rule - always true/always false conditional type condition - like $a is int when there's already @param int $a

I think PHPStan 1.6.0 is on the right track to be released next week, but I don't want to promise anything :)

[ondrejmirtes]

I realized that all of these would have some overlap/duplications so this is going to be taken care of by a single rule:

New rule - is the type in ConditionalTypeNode part of the function's @template type map?
New rule - does the parameter in ConditionalTypeForParameterNode exist?
New rule - always true/always false conditional type condition - like $a is int when there's already @param int $a

[canvural]

I was playing with the conditional types and came up with this example from Psalm: https://phpstan.org/r/560549e5-71b5-47be-8da2-74d5d87089e1 I know it's still in work in progress, so I didn't want to open a new issue.

Shouldn't it dump float in the second case? Because int|float is not int, it should take the else branch

[ondrejmirtes]

@canvural Generics don't work like that. If you have T referenced multiple times in parameters, T is inferred as a union of all parameters. So in case of (T is int ? int : float), it's a matter of (int|float is int ? int : float), so both if and else are used in the result.

[canvural]

I know it is inferred as union. But the union int|float is not int So I thought only else branch would be considered.

[ondrejmirtes]

int|float can be int (IntegerType->isSuperTypeOf(UnionType(IntegerType, FloatType)) is maybe.

[ondrejmirtes]

Done: phpstan/phpstan-src@a8d9628

@rvanvelzen Please is there anything you want to add? I'd like to release PHPStan 1.6.0 later this week :)

[rvanvelzen]

Very nice! The conditional never type was the only thing I ran into, so as far as I'm concerned it's good to go :)

[ondrejmirtes]

Awesome! I need to write an article about the flagship things in PHPStan 1.6.0, compile the release notes, and it's good to go :)

[ondrejmirtes]

@canvural Your example can be solved with:

/**
 * @param int|float $a
 * @param int|float $b
 * @return ($a is int ? ($b is int ? int : float) : float)
 */
function add($a, $b) {
    return $a + $b;
}

[github-actions]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.