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

(RFC) Implement template default syntax #148

Merged
merged 2 commits into from
Oct 14, 2022

Conversation

rvanvelzen
Copy link
Contributor

Proof-of-concept implementation for template default syntax (for phpstan/phpstan#4801)

@ondrejmirtes
Copy link
Member

/cc @orklah Does Psalm already have this / would you be interested too?

@orklah
Copy link

orklah commented Sep 6, 2022

Psalm doesn't have this but it could be interesting (especially because Psalm sometimes struggle to assign values to classes with templates that were never properly created)

I doubt I'll be in position to implement this myself anytime soon though

@ondrejmirtes
Copy link
Member

Alright, I'm fine with going ahead with this. But what the syntax should be? The issue proposes a separate @template-default tag. What's the precedent from other languages that have this feature?

How would the syntax proposed here for @template work when the bound is involved?

@rvanvelzen
Copy link
Contributor Author

I've taken the syntax from TypeScript. At least C++ also uses this syntax, but I haven't looked at other languages.

Because the default value is tightly coupled to the type parameter (just like the bound is), I don't think it's a good idea to have a separate tag for it.

@rvanvelzen rvanvelzen marked this pull request as ready for review September 8, 2022 06:31
@ondrejmirtes
Copy link
Member

Nice, it makes sense to me. Could you also please submit a proof of concept to phpstan-src to make sure these are not released with a big time gap between them? :) Thank you.

What we need to think about is modifying the rules here https://github.com/phpstan/phpstan-src/tree/1.8.x/src/Rules/Generics for this new feature:

@rvanvelzen rvanvelzen changed the base branch from 1.8.x to 1.9.x October 13, 2022 13:54
@ondrejmirtes ondrejmirtes merged commit 7d1e812 into phpstan:1.9.x Oct 14, 2022
@ondrejmirtes
Copy link
Member

Thank you!

@alexander-schranz
Copy link

Nice to see this merged. 🎉 🚀

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

Successfully merging this pull request may close these issues.

4 participants