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

correct type inference for PhpStorm #106

Merged
merged 1 commit into from
Mar 31, 2015
Merged

Conversation

tiger-seo
Copy link
Contributor

supposedly, this will help other IDE's too

supposedly, this will help other IDE's too
@@ -148,7 +148,7 @@ public function getType()
* sets the last modification time of the stream content
*
* @param int $filemtime
* @return vfsStreamContent
* @return $this
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldnt it be self?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no, self may be used for static methods, but for instance appropriate one is $this

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

According to http://www.phpdoc.org/docs/latest/references/phpdoc/types.html $this is not a valid type, only self could be used. But in this case it would be interpreted as vfsStreamAbstractContent, and I'm not particularly happy about that as the abstract type should not be visible in the API.
I'm not sure if self would help with PhpStorm then, but I'd rather follow a standard then to change it to something IDE specific. Maybe it would be a better idea to simply use fully qualified class names in this places?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Who uses documentation produced by phpdoc? I don't. But me myself and many many my collegues uses IDE and it comes very important to have expected behaviour. @mikey179 which IDE you use?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mikey179 please see http://phpdoc.org/docs/latest/guides/types.html here u can find $this ;)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also please see https://github.com/phpDocumentor/fig-standards/blob/master/proposed/phpdoc.md#keyword

$this, the element to which this type applies is the same exact instance as the current class in the given context. As such this type is a stricter version of static as, in addition, the returned instance must not only be of the same class but also the same instance.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Must have overlooked $this.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 94.67% when pulling 05e2933 on tiger-seo:master into 811bf97 on mikey179:master.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage remained the same at 94.67% when pulling 05e2933 on tiger-seo:master into 811bf97 on mikey179:master.

mikey179 added a commit that referenced this pull request Mar 31, 2015
correct type inference for PhpStorm
@mikey179 mikey179 merged commit 29e2ba8 into bovigo:master Mar 31, 2015
@tiger-seo
Copy link
Contributor Author

thank you 👍

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.

None yet

4 participants