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

Change docblocks to reflect magic static methods #184

Closed
wants to merge 1 commit into from

Conversation

rizqidjamaluddin
Copy link

Some IDEs use a class' @method docblock annotations to autocomplete, suggest and typehint magic methods for users. The validator.php class has these, but doesn't mark them as static, so the suggested V::method() syntax continuously throws errors:

phpstorm

IDE I'm using is PHPStorm 7.1.3 (latest as of writing).

It appears that adding static annotations doesn't affect normal (dynamic) calls because PHP is okay with calling statics normally. Clearly the other way around doesn't seem to work!

(I noticed a similar request from long ago - I apologize if there's already a rationale for not doing this.)

Improves support for IDEs that examine class docblocks to suggest/typehint methods.
@augustohp augustohp added this to the 0.7.0 milestone Jun 26, 2014
@augustohp
Copy link
Member

Hey, thanks to your feedback and pull request! 🍻

Have you tested your pull requested code on other largely used IDEs (Eclipse PDT, Zend Studio, Netbeans), does this affect the behavior of autocomplete on them?

@alganet
Copy link
Member

alganet commented Jun 26, 2014

I guess we did those docblock tags right after PHP 5.3 (and __callStatic) was released and we focused on the __call only cause no IDE supported static magic docs back then. It will probably work fine as static now.

Calls from incompatible contexts were banned in 5.6 though, so we may need to add two lines of docs for each rule (one static and one for the instance). It seems that phpDocumentor2 will set the standard for those. They're discussing a format that could save us a lot of lines

@rizqidjamaluddin
Copy link
Author

I admit that I haven't checked with other IDEs, so I'm hoping any users who use those can confirm/deny if it works in their respective environments. I think (sans problems with incompatible contexts) any spec-compliant IDE shouldn't have a problem with this, but I absolutely understand the concerns!

Would having both lines cause any problems? I imagine not, unless some IDE thinks it's declaring each method twice somehow.

@appinteractive
Copy link

Hey there, please merge this pull request (it's 6 months now), it's not really usable without this in PhpStorm! It shouldn't break anything since its quite obvious to specify the static key there. I like this library but its not very well maintained. :(

@henriquemoody
Copy link
Member

I'm sorry by our late. Seems like there is a lot of people complaining about that, we just discuss it on #202 2 hours ago. In fact we cannot use only static declarations since you can use it in chains, but the best think to do, IMO is create 2 declarations for every rule, what you guys think about it?

@henriquemoody
Copy link
Member

Actually, when a method is static it can me used as non-static, so the static must work for both.

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

Successfully merging this pull request may close these issues.

None yet

5 participants