drush-ops / drush

Drush is a command-line shell and scripting interface for Drupal, a veritable Swiss Army knife designed to make life easier for those who spend their working hours hacking away at the command prompt.
https://www.drush.org
2.33k stars 1.08k forks source link

Drush unrelated src/FooGenerator.php file causes error #5662

Open Sweetchuck opened 1 year ago

Sweetchuck commented 1 year ago

Describe the bug

When there is a file src/FooGenerator.php which contains the class \Drupal\foo\FooGenerator then it causes the following error for every Drush command (including drush status or drush list)

[preflight] Class "Drush\FooGenerator" does not exist [warning] Drush command terminated abnormally.

This package is not a normal module. This package contains site-wide drush commands. In the composer.json the type is drupal-drush. Like this:

{
    "name": "drupal/foo",
    "type": "drupal-drush"
}

This package and the Drush commands from this package work perfectly with Drush-11.x. I experience this error after I had upgraded drush/drush to 12.x.

This line looks suspicious to me. https://github.com/drush-ops/drush/blame/12.x/src/Runtime/ServiceManager.php#L399

System Configuration

Q A
Drush version? 12.1.0.0
Drupal version? 10.0.7
PHP version 8.2
OS? Linux
weitzman commented 1 year ago

Please try Drush 12.1.0 to get #5656 ... FYI generators need porting in order to be functional on Drush 12.

Sweetchuck commented 1 year ago

The class I am talking about is not a Drush generator. src/FooGenerator.php Totally Drush unrelated class. It just happens to be the class/file name ends with ...Generator.

I thought that a Drush Command or Generator has to be in the following directories in order to be discovered:

Isn't it true?

I don't see the connection between the mentioned #5656 and my problem.

Sweetchuck commented 1 year ago

I noticed this problem yesterday, and I reported this issue today. I haven't noticed that there were a new Drush release 9 hours ago. I will try it with Drush 12.1.0 soon.

Sweetchuck commented 1 year ago

Actually I did used the latest release (Drush Commandline Tool 12.1.0.0) as I mentioned it in the original post.

weitzman commented 1 year ago

This generator discovery via filesystem scan is a breaking change from Drush 11. We used to discover via the Drupal container. I recommend renaming your class.

It is possible that the discovery could be tighter scoped to fewer directories so leaving this open as a feature request.

greg-1-anderson commented 1 year ago

I will see about tightening down the generator instantiator to avoid problems with false matches. Certainly it would be undesirable if the same file were discovered both as a command file and a generator.

@Sweetchuck: You said that your file src/FooGenerator.php contains a Drush sitewide command file. How is this file being discovered? CommandFiles should end in Command.php or Commands.php. I am wondering if you are relying on a side-effect of Drush 11, that searches for Commands||Handlers||Generators and then figures out what is what. If so, it is possible that Drush 12 might stop recognizing your file as a CommandFile once this bug is fixed. If that comes to pass, then you might need to rename your class. Sounds like you might be using some other discovery mechanism, though, since your class is \Drupal\foo\FooGenerator instead of Drush\FooGenerator. Could you explain?

Sweetchuck commented 1 year ago

@greg-1-anderson

You said that your file src/FooGenerator.php contains a Drush sitewide command file

This is not what I intended to say.

The mentioned ./src/FooGenerator.php nor a Drush command neither a Drush generator. It is a totally Drush independent class.

Sweetchuck commented 1 year ago

@greg-1-anderson When I have a Composer package and its type is drupal-drush and the name is drupal/foo, then what are the correct PHP namespaces for the following directories?

  1. ./src/
  2. ./Commands/
  3. ./Generators/
Chi-teck commented 1 year ago

Can CommandFileDiscovery check class attributes?

greg-1-anderson commented 1 year ago

You can't check class attributes until the file is loaded. The discovery classes don't load files.

Namespaces depend on the discovery mechanism. PSR-4 discovery is best; it appends the namespaces Drush\Commands & etc. to whatever namespace you declare in your composer.json's autoload section.

The CommandFileDiscovery checks have different rules; perhaps we should be (should have been?) more strict on where we search. If CommandFileDiscovery is causing problems, and the behavior is different than Drush 11, then perhaps we should change it as a bug fix. I sort of thought that (the non-Module) CommandFileDiscovery was only there as backwards-compatibility, but I'll have to review and see what the path / namespace mappings are.

Sweetchuck commented 1 year ago

Any update on this?