Conversation
|
Thank you, this looks very promising. I'm also doing everything with "80/20" in mind 😊 |
c4f3c13 to
383b9f7
Compare
|
Hi, I'm gonna rebase this myself as I just implemented nested generic bounds on master (e671cc0) and I caused quite a few conflicts for you. Stay tuned :) |
c823aa2 to
c8de393
Compare
ondrejmirtes
left a comment
There was a problem hiding this comment.
I rebased this and added one commit that fixes prefix priority. I really like this! :)
Another test idea: Please try to use an alias right after defining it/importing in class docblock, for example in a @method tag.
|
Oh, wrong rebase, will fix the problem :) And please, go through the review one-by-one and add each fix as a new commit :) |
57e1645 to
8d275bd
Compare
|
Alright, giving the branch back to you :) I don't know about the memory limited exceeded in the few failing jobs - it's probably a coincidence that it happened in this branch. |
|
Maybe the memory is leaking somewhere after all... |
643ffd8 to
eff0cad
Compare
|
So, I've fixed some of the issues from your review. I'd say the most notable change is 210cb62 in which I've wrapped every type alias declaration in a I'll try and take a look at the memory issues too, they don't seem to be just a coincidence 🤔 |
|
Looking forward to this being finished, it looks awesome already :) |
This adds support for nested type aliases and helps preserve scope when importing aliases. Aliases are resolved lazily because eager resolution would create a chicken-and-egg problem where aliases need to be resolved while they are already being resolved, thus not being resolved at all as a result of recursion protection.
…no longer stored in cache
eff0cad to
98a8e8b
Compare
…failing the resolution
98a8e8b to
3519a52
Compare
…lving the doc block
|
While thinking about ObjectType and IdentifierTypeNode, I realized unwanted behaviour: <?php
/** @phpstan-type int \stdClass */
class Fooooo
{
/** @param int $a */
public function doFoo($a)
{
\PHPStan\dumpType($a); // should be int, but current is stdClass
}
}I'm gonna solve it by deleting TypeAliasesTypeNodeResolverExtension and doing the logic in the right place in TypeNodeResolver::resolveIdentifierTypeNode() (after things that are never supposed to be aliased are handled). It should have performance benefits as well. |
src/Type/TypeAliasResolver.php
Outdated
| if (array_key_exists($aliasNameInClassScope, $this->resolvedLocalTypeAliases)) { | ||
| return $this->resolvedLocalTypeAliases[$aliasNameInClassScope]; | ||
| } | ||
|
|
|
Alright, I think we're green :) I have two more things to ask from you:
Thank you. |
|
Glad to hear that :) I'll add the check and the docs. Thank you for your help! |
I wonder why nobody noticed these issues in phpstan/phpstan-src#460... And lack of circular reference means we can't really typecheck `yield $promise;` properly.
Resolves phpstan/phpstan#3001. Depends on phpstan/phpdoc-parser#62.
Since Psalm only seems to support class-scoped type aliases, I did the same here. As I've said before, "it seems to be the easiest direction to take and is imo a good first iteration" that neatly follows the 80/20 rule.
I think the implementation still has some rough edges, but it appears to work surprisingly well 😅 looking forward to your thoughts and remarks!