8000 Cleaned up and edited with latest L4 revisions by jonphipps · Pull Request #5 · laravelbook/laravel4-phpstorm-helper · GitHub
[go: up one dir, main page]
Skip to content

Cleaned up and edited with latest L4 revisions #5

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Cleaned up and edited with latest L4 revisions #5

wants to merge 3 commits into from

Conversation

jonphipps
Copy link

Latest Laravel 4 as of 02/20/2013, edited to include the defaults missed by t 8000 he codeintel parser, along with some other minor flaws in the output. Based strictly on the json config file supplied with the parser.

@andrew13
Copy link

This works well. Should be merged

@lukefor
Copy link
lukefor commented Feb 24, 2013

+1 working perfectly in PhpED too

@jonphipps
Copy link
Author

This also fixes issue #2

@kwolniak
Copy link

Not for me. Look at: https://dl.dropbox.com/u/871494/phps-laravel.jpg
There is a lot of errors/warnings: Non-static method 'get' should not be called statically or 'void' method 'to' result used, etc.

@jonphipps
Copy link
Author

@kwolniak Your first issue with "Non-static method 'get' should not be called statically" warning is a side-effect of re-declaring the methods statically and I don't think that can be avoided with the current version (or 6) of PHPstorm.
The " 'void' method 'to' result used" warning is the result of the Laravel 4 docblocks defaulting to @return void regardless of what is returned and rarely correcting it. There are many things wrong with the docblocks, and there's an open PR from @ROMOPAT that fixes some. But I suspect that it's both a low priority, given the fact that it's apparently not an issue for Sublime and probably no longer mergeable: laravel/framework#72

I ended up changing the severity of the checks for those in PHPstorm to a 'weak warning', although you can turn the check off entirely:

  • Preferences::Inspections::General::Dynamic method called as static
  • Preferences::Inspections::Probably Bug::Void function result used

Last, but hardly least Laravel 4 is a very moving target with classes coming, going, and being moved around. Much has changed in the last month. Running the codeintel script weekly seems to result in quite a few changes. There's also the issue of being able to properly reflect the methods returned by a serviceprovider that provides static version of methods from several classes, like Validator does. I think that some refactoring of the codeintel script to support merging several classes into a single service provider would be useful: laravelbook/laravel-codeintel-generator#3

Of the many L4 ide helpers, this is the only one that I know of that's being built by a code parser, and that means that there's at least a hope of keeping up with the changes. Until these issues are either resolved by the IDE makers or L4 stabilizes enough to make building this by hand worthwhile, this represents the best solution of found.

@barryvdh
Copy link

You could also take a look at my helper generator: https://github.com/barryvdh/laravel-ide-helper
This doesn't have static warnings or missing (default) arguments, and is generated dynamically, so you can update it yourself. (With artisan instead of python)

@MLenngren
Copy link

Barry. Really nice, best fix so far for me!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@jonphipps @andrew13 @lukefor @kwolniak @barryvdh @MLenngren
0