-
Notifications
You must be signed in to change notification settings - Fork 650
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
Add new annotation: @psalm-self-out #3650
Conversation
This is a good start. I’d like Psalm to also be able to verify the integrity of |
Agree. |
tests/SelfOutTest.php
Outdated
} | ||
class Bar { | ||
/** | ||
* @psalm-self-out Foo |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be Foo&Bar
, really - and could you add an exception if the function doesn’t do that?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, but maybe let's detail a little bit what we mean? Using the closed/open file as example:
interface ClosedFile {
/**
* @psalm-self-out File&OpenFile
*/
public function open() {}
}
interface OpenFile {
/**
* @psalm-self-out File&ClosedFile
*/
public function close() {}
public function read() {}
}
class File implements OpenFile, ClosedFile {
// ... function defs here
}
Then we define psalm-self-out
as always requiring first part of the intersection type being the base-class, second part being the "next" state. This way, Psalm doesn't need to "remember" the base-class in a separate field.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But in this case File
is already File&OpenFile&ClosedFile
, by virtue of implementing those interfaces, isn't it? Or do you propose to reuse the same syntax to achieve different effects?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, you're right. Hm, so maybe intersection types cannot be used.
Or do you propose to reuse the same syntax to achieve different effects?
No no no.
Fun fact: In Smalltalk something similar is called |
@muglug Hm, how can I debug Circle CI without giving it access to absolutely everything on my account? |
Only thing left failing is taint tests, it seems. |
Ignore that, but would you mind squashing these commits? |
Can't do it without rebasing everthing. Can't you squash and merge instead if/when this PR gets accepted? The tab "Files changed" contains all changes for all commits in this PR, if you want an overview. |
Thanks! |
This PR adds the annotation
@param-self-out
for methods, changing the type of$this
.Use-case is to be able to create your own typestates besides the ones already implicit in Psalm (
resource
, andclosed-resource
) in an object-oriented manner.Typestates are mostly (only?) useful together with alias tracking or ownership inference, which is another discussion: #3614