Skip to content
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

Don't generalize template types #1206

Draft
wants to merge 2 commits into
base: 1.9.x
Choose a base branch
from

Conversation

rvanvelzen
Copy link
Contributor

A little experiment to see what happens. Psalm doesn't generalize template types either, because a @var is easy enough to write.

@ondrejmirtes
Copy link
Member

a @var is easy enough to write

Yeah but the same argument can be used to keep the current behaviour :)

BTW Can you please tell me which items from phpstan/phpstan#3853 (comment) do you plan to work on? Once I finish the memory optimization I'm working on right now, the only thing before releasing 1.6.0 is the finished support for conditional return types, so I'd like to get that over the finish line too :) Thanks.

@rvanvelzen
Copy link
Contributor Author

a @var is easy enough to write

Yeah but the same argument can be used to keep the current behaviour :)

Definitely. Just throwing this out there :)

BTW Can you please tell me which items from phpstan/phpstan#3853 (comment) do you plan to work on? Once I finish the memory optimization I'm working on right now, the only thing before releasing 1.6.0 is the finished support for conditional return types, so I'd like to get that over the finish line too :) Thanks.

Sure. I'm planning on working on these today:

  • New rule - is the type in ConditionalTypeNode part of the function's @template type map?
  • New rule - does the parameter in ConditionalTypeForParameterNode exist?

I'm not sure how soon I'll get to any of the rest. I do realize that that doesn't help much, but I'd rather not make false promises.

@ondrejmirtes
Copy link
Member

Yeah, this is totally fine, I can take over some of the work myself, just wanted to make sure we're synchronized (I'll let you know when I'm gonna work on some of the tasks).

@rvanvelzen
Copy link
Contributor Author

rvanvelzen commented Apr 12, 2022

Just want to let you know that I probably won't be able to work on the conditional types this week. I was planning on doing more but life is busy as well :)

@ondrejmirtes
Copy link
Member

No problem, I'm gonna ping you once I take over a task :)

@ondrejmirtes
Copy link
Member

I'm interested in not generalizing the types, but IMHO it needs to be paired with this suggestion phpstan/phpstan#6732 (comment) to be usable. The whole thread is worth reading, but here I especially refer to the Collection<unresolved> part.

@ondrejmirtes
Copy link
Member

Alright, I realized we can not-generalize types outside of GenericObjectType: #2818

But it's still a BC break so it's going to be bleeding edge-only for now.

Inside of GenericObjectType we're gonna need this logic phpstan/phpstan#6732 (comment)

But anyway this is still going to fix a lot of issues.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants