phpstan / phpdoc-parser

Next-gen phpDoc parser with support for intersection types and generics
MIT License
1.31k stars 62 forks source link

Add basic support for new-in-initializer #238

Open rvanvelzen opened 4 months ago

rvanvelzen commented 4 months ago

Implementation for #173

ondrejmirtes commented 4 months ago

Do you see this somehow being useful for the analysis, or is it just because we don't have a different syntax for "let's make this parameter optional but not nullable"?

One thing this will impact are types resolved for generics and conditional types (that should be tested), but otherwise not much imho.

rvanvelzen commented 4 months ago

Could you explain what this has to do with optional or non-nullable parameters?

One thing this will impact are types resolved for generics and conditional types (that should be tested), but otherwise not much imho.

While this is true, for default values in regular code generics aren't taken into account either. So implementing this in PHPStan itself is just a few lines of code.

ondrejmirtes commented 4 months ago

They are: https://phpstan.org/r/fe352005-91ed-4021-bd95-9d6df9f886a4 (and not generalized on bleeding edge https://phpstan.org/r/7fad672b-f8b0-4128-9282-d82a6599f2b6, unless in GenericObjectType).

ondrejmirtes commented 4 months ago

Could you explain what this has to do with optional or non-nullable parameters?

I don't really get the question, because the answer is "everything"? This syntax applies only to default value of parameters in @method. So I was interested how this enhances analysis in PHPStan. And the answer to that is that besides marking the parameter as optional, it will influence the result of generics and conditional types. Right?

rvanvelzen commented 4 months ago

They are: phpstan.org/r/fe352005-91ed-4021-bd95-9d6df9f886a4 (and not generalized on bleeding edge phpstan.org/r/7fad672b-f8b0-4128-9282-d82a6599f2b6, unless in GenericObjectType).

Ah, I actually meant that the default value itself is never generic: https://phpstan.org/r/dc1fee3f-b8de-448d-ba24-edc733770921

I think I understand what you're trying to figure out, and I think the short answer is: this is only to indicate that such a parameter has a default value, which is impossible otherwise right now. Not really relevant for analysis.

But this is nice and simple and mirrors the native syntax precisely. :)

ondrejmirtes commented 4 months ago

I agree. I just wanted to show the test with generics should be added in PHPStan to show we're actually reading the default value correctly. And if the class in new is actually generic, we should also resolve it correctly. So new Foo([1, 2, 3]) should result in correct Foo<T> same as in PHP code - we should probably construct PhpParser's New_ node and just ask Scope about it...