Report Drush 12 deprecations with command classes and drush.services.yml file about phpstan-drupal HOT 12 OPEN

OK, but the presence of drush.services.yml should always trigger a warning. Its always good to remove it. If you have one AND you have autowire/create, you are doing the same work twice. Thats likely harmless, but its confusing where changes should be made.

Yup, agreed.

from phpstan-drupal.

So anything which extends DrushCommands and doesn't have a create method, throw an error?

from phpstan-drupal.

So anything which extends DrushCommands and doesn't have a create method, throw an error?

Only if the module that owns that command file has a drush.services.yml file for dependency injection.

from phpstan-drupal.

Because if they do not, it's assumed it doesn't need dependency injection?

from phpstan-drupal.

Because if they do not, it's assumed it doesn't need dependency injection?

I think it depends if there is a drush.services.yml record for that class that contains arguments or not.

from phpstan-drupal.

As of, things have changed yet again: https://github.com/drush-ops/drush/releases/tag/12.5.0

Commandfiles are now encouraged to inject their dependencies via Autowire (drush-ops/drush#5893). Prior methods such as a create() method or a drush.services.yml file are deprecated and may be removed in Drush 13. Now is also a good time to update commandfiles to use PHP Attributes instead of annotations.

from phpstan-drupal.

I think this can be DrushAutowireTraitRule which reports an error if a command class doesn't have the AutowireTrait on the class.

from phpstan-drupal.

Well, I'm confused about the messaging from Drush about create() given that Drupal core also isn't strong on autowiring:

Prior methods such as a create() method or a drush.services.yml file are deprecated and may be removed in Drush 13.

... vs the docs from https://www.drush.org/12.x/dependency-injection/:

If Autowire is still insufficient, a commandfile may implement its own create() method (see below).

I would caution to warn only if AutowireTrait and create() are not used, because otherwise I can't make a contrib drush command compatible with the current and previous major versions, which is personally what I target doing.

from phpstan-drupal.

1. I would warn if a drush.services.yml is present. Thats the only thing thats going to be deprecated in the foreseeable future. Point people to https://www.drush.org/latest/dependency-injection/ for more info.
2. Perhaps warn if composer.extra.drush is present. Its no longer needed with Drush 12+

@davereid - yeah create() is not part of the deprecation. Its more that Autowire is new and shiny and slightly preferred. Feel free to keep using create(). AFAICT autowire is very much available and recommended for Contrib. Its just Drupal core thats slow.

from phpstan-drupal.

Thanks for the input and clarifications!

Right now we have:

            $servicesFileName = $module_dir . '/' . $module_name . '.services.yml';
            if (file_exists($servicesFileName)) {
                $this->serviceYamls[$module_name] = $servicesFileName;
            }

I guess we need the same for drush.services.yml. This way, the DrushAutowireRule can do something like the following.

If the class name is found in the service map, via a drush.services.yml check if it has ::create and/or AutowireTrait. If it does not, report an error that command classes need to use modern dependency injection, see https://www.drush.org/latest/dependency-injection/

I shouldn't need to check composer.json (which is harder to do, more of an Upgrade Status thing) then.

from phpstan-drupal.

OK, but the presence of drush.services.yml should always trigger a warning. Its always good to remove it. If you have one AND you have autowire/create, you are doing the same work twice. Thats likely harmless, but its confusing where changes should be made.

from phpstan-drupal.

Perfect. Thanks! I hit major camp brain fog and fatigue before writing any code. But at least the requirements are now captured. I appreciate it!

from phpstan-drupal.