PHPStorm plugin #332

Closed
avant1 opened this Issue Sep 19, 2015 · 9 comments

Comments

Projects
None yet
4 participants
@avant1
Contributor

avant1 commented Sep 19, 2015

Hi.
A few days ago I found PHPStorm plugin that makes code completion work for services fetched from container: pulyaevskiy/phpstorm-phpdi. It's pretty simple, but does it's job well.
No more _@_var annotations needed, it just works (if you using php5.5 syntax for defining services).

I find it very useful, so maybe it is worth to be added to the documentation?

It is not a PR adding link to the docs cause I'm not sure it is a good idea. Maybe @pulyaevskiy should give his agreement at first? It is published under MIT license, but I'm not good at software license types.

@mnapoli

This comment has been minimized.

Show comment
Hide comment
@mnapoli

mnapoli Sep 19, 2015

Member

Thanks for bringing that up, I had no idea it existed :) That sounds really interesting, a few remarks on the topic:

  • is that if one needs autocompletion after $container->get() that means they are using the service locator, which can be considered an anti-pattern => because of that I'm not against documenting it (it can be useful, especially working with legacy), but I think it needs to be explained
  • the same result can also be achieved with a .phpstorm.meta.php file (downside: requires a file into every project), for example:
<?php
namespace PHPSTORM_META
{
    $STATIC_METHOD_TYPES = [
        \DI\Container::get('') => [
            "" == "@",
        ],
        \Interop\Container\ContainerInterface::get('') => [
            "" == "@",
        ],
    ];
}

Still the plugin can be useful, all this deserves to be documented!

Member

mnapoli commented Sep 19, 2015

Thanks for bringing that up, I had no idea it existed :) That sounds really interesting, a few remarks on the topic:

  • is that if one needs autocompletion after $container->get() that means they are using the service locator, which can be considered an anti-pattern => because of that I'm not against documenting it (it can be useful, especially working with legacy), but I think it needs to be explained
  • the same result can also be achieved with a .phpstorm.meta.php file (downside: requires a file into every project), for example:
<?php
namespace PHPSTORM_META
{
    $STATIC_METHOD_TYPES = [
        \DI\Container::get('') => [
            "" == "@",
        ],
        \Interop\Container\ContainerInterface::get('') => [
            "" == "@",
        ],
    ];
}

Still the plugin can be useful, all this deserves to be documented!

@avant1

This comment has been minimized.

Show comment
Hide comment
@avant1

avant1 Sep 20, 2015

Contributor

is that if one needs autocompletion after $container->get() that means they are using the service locator, which can be considered an anti-pattern => because of that I'm not against documenting it (it can be useful, especially working with legacy), but I think it needs to be explained

I've been used this plugin with symfony2 application, and I had access to container only in controllers (probably, better solution would be passing services into controllers as dependencies, I understand, but it seems that using container to fetch services in controllers is widespread approach in symfony2).

So with symfony2-plugin I have autocompletion for core symfony2 services and my services that have to be added to symfony2 container (where tags are needed). And this plugin helped me to achieve code autocompletion for services defined in PHP-DI container.

the same result can also be achieved with a .phpstorm.meta.php file (downside: requires a file into every project):

Wow, thats interesting, I'll check this out.

I will open PR to add this to docs if I have time during next week.

Contributor

avant1 commented Sep 20, 2015

is that if one needs autocompletion after $container->get() that means they are using the service locator, which can be considered an anti-pattern => because of that I'm not against documenting it (it can be useful, especially working with legacy), but I think it needs to be explained

I've been used this plugin with symfony2 application, and I had access to container only in controllers (probably, better solution would be passing services into controllers as dependencies, I understand, but it seems that using container to fetch services in controllers is widespread approach in symfony2).

So with symfony2-plugin I have autocompletion for core symfony2 services and my services that have to be added to symfony2 container (where tags are needed). And this plugin helped me to achieve code autocompletion for services defined in PHP-DI container.

the same result can also be achieved with a .phpstorm.meta.php file (downside: requires a file into every project):

Wow, thats interesting, I'll check this out.

I will open PR to add this to docs if I have time during next week.

@mnapoli

This comment has been minimized.

Show comment
Hide comment
@mnapoli

mnapoli Sep 20, 2015

Member

👍 great

And right, I forgot that in Symfony that's the documented/recommended way to use the container. So indeed it can also be useful there.

Member

mnapoli commented Sep 20, 2015

👍 great

And right, I forgot that in Symfony that's the documented/recommended way to use the container. So indeed it can also be useful there.

@pulyaevskiy

This comment has been minimized.

Show comment
Hide comment
@pulyaevskiy

pulyaevskiy Sep 20, 2015

Contributor

Good discussion! Thanks for mentioning me.

Just to give a bit more background about the plugin.

We've built some sort of home grown application framework which uses PHP-DI.

In our workflow when we work with domain layer, api endpoints or persistence it's "forbidden" to depend on container. This way we avoid service locator issues when developing our apps, everything is pure dependency injection.

However there is few cases when we have to rely on container:

  • The application component itself is dependent on the container, obviously. So there is some code which makes calls to $container->get().
  • Testing. We have different kinds of tests. And there is some which rely on container as well. We probably could go further and make tests use DI as well, but at this point it seems like overkill, given that current approach works pretty well. Also it might be tricky with phpunit to use DI in test case classes.

And of course we have Symfony app where controllers and console commands rely on container so we needed some sort of convenience there.

One thing to note about this plugin: it makes big difference when refactoring things. It is not only autocompletion that helps, but also IDE uses this type information when you rename / move things around.

Contributor

pulyaevskiy commented Sep 20, 2015

Good discussion! Thanks for mentioning me.

Just to give a bit more background about the plugin.

We've built some sort of home grown application framework which uses PHP-DI.

In our workflow when we work with domain layer, api endpoints or persistence it's "forbidden" to depend on container. This way we avoid service locator issues when developing our apps, everything is pure dependency injection.

However there is few cases when we have to rely on container:

  • The application component itself is dependent on the container, obviously. So there is some code which makes calls to $container->get().
  • Testing. We have different kinds of tests. And there is some which rely on container as well. We probably could go further and make tests use DI as well, but at this point it seems like overkill, given that current approach works pretty well. Also it might be tricky with phpunit to use DI in test case classes.

And of course we have Symfony app where controllers and console commands rely on container so we needed some sort of convenience there.

One thing to note about this plugin: it makes big difference when refactoring things. It is not only autocompletion that helps, but also IDE uses this type information when you rename / move things around.

@mnapoli

This comment has been minimized.

Show comment
Hide comment
@mnapoli

mnapoli Sep 20, 2015

Member

@pulyaevskiy thanks for chiming in, your use cases make perfect sense to me, you seem to have something very good going on I'm always happy to learn about how people use this package ;)

One thing that could be considered for improving the plugin (but of course requires time to implement it): make it work only for DI\Container::get() or ContainerInterface::get() (and not all ->get() calls). If it worked with ContainerInterface that means it would work out of the box with a lot of DI containers which would be pretty cool.

Member

mnapoli commented Sep 20, 2015

@pulyaevskiy thanks for chiming in, your use cases make perfect sense to me, you seem to have something very good going on I'm always happy to learn about how people use this package ;)

One thing that could be considered for improving the plugin (but of course requires time to implement it): make it work only for DI\Container::get() or ContainerInterface::get() (and not all ->get() calls). If it worked with ContainerInterface that means it would work out of the box with a lot of DI containers which would be pretty cool.

@pulyaevskiy

This comment has been minimized.

Show comment
Hide comment
@pulyaevskiy

pulyaevskiy Sep 21, 2015

Contributor

@mnapoli

it would work out of the box with a lot of DI containers which would be pretty cool.

Technically it already works with pretty much any container (and not only container) which uses ->get(Foo::class) syntax. :-)

Though I agree that constraining it to ContainerInterface would be cleaner. I'll see if I can make it work this way (it might be a bit tricky to implement).

Contributor

pulyaevskiy commented Sep 21, 2015

@mnapoli

it would work out of the box with a lot of DI containers which would be pretty cool.

Technically it already works with pretty much any container (and not only container) which uses ->get(Foo::class) syntax. :-)

Though I agree that constraining it to ContainerInterface would be cleaner. I'll see if I can make it work this way (it might be a bit tricky to implement).

mnapoli added a commit that referenced this issue Oct 18, 2015

@mnapoli mnapoli added this to the 5.2 milestone Oct 18, 2015

@mnapoli

This comment has been minimized.

Show comment
Hide comment
@mnapoli

mnapoli Oct 18, 2015

Member

Was fixed in #334, it's online: http://php-di.org/doc/ide-integration.html

Awesome!

Member

mnapoli commented Oct 18, 2015

Was fixed in #334, it's online: http://php-di.org/doc/ide-integration.html

Awesome!

@mnapoli mnapoli closed this Oct 18, 2015

@lattwood

This comment has been minimized.

Show comment
Hide comment
@lattwood

lattwood Dec 21, 2015

Awesome, thanks for adding the documentation on it. There's also support for the make method now too.

pulyaevskiy/phpstorm-phpdi@ac2f56d

Awesome, thanks for adding the documentation on it. There's also support for the make method now too.

pulyaevskiy/phpstorm-phpdi@ac2f56d

@mnapoli

This comment has been minimized.

Show comment
Hide comment
@mnapoli

mnapoli Dec 21, 2015

Member

👍 great!

Member

mnapoli commented Dec 21, 2015

👍 great!

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