Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Don't generalize template types #1206

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

Draft
rvanvelzen wants to merge 2 commits into phpstan:1.9.x
base: 1.9.x
Choose a base branch
Loading
from rvanvelzen:dont-generalize-templates

Conversation

@rvanvelzen
Copy link
Contributor

@rvanvelzen rvanvelzen commented Apr 12, 2022

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

vdauchy reacted with thumbs up emoji mad-briller reacted with heart emoji
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.

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.

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).

Copy link
Contributor Author

rvanvelzen commented Apr 12, 2022
edited
Loading

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 :)

Copy link
Member

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

rvanvelzen reacted with thumbs up emoji

@rvanvelzen rvanvelzen force-pushed the dont-generalize-templates branch from efdf2c2 to c4747ec Compare May 27, 2022 14:38
@rvanvelzen rvanvelzen changed the base branch from 1.6.x to 1.7.x May 27, 2022 14:40
@rvanvelzen rvanvelzen force-pushed the dont-generalize-templates branch from c4747ec to 5d33443 Compare May 27, 2022 14:43
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.

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.

vdauchy reacted with heart emoji

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

AltStyle によって変換されたページ (->オリジナル) /