Do not allow commands in parent classes

[greg-1-anderson]

Set flag in the Annotated Command commandFactory, if it exists, to ignore commands in parent classes.

This prohibition was added in annotated-command v4.5.7, but it will be reverted to maintain backwards compatibility. See consolidation/annotated-command#277. Once this revert happens, Drush should continue to disallow extending command classes, to avoid side effects in modules.

[greg-1-anderson]

I merged the Annotated Command PRs into the latest dev releases (4.x-dev and 5.x dev). We should merge this into Drush, then do a stable release of Drush, then do a stable release of Annotated Command.

No rush, as long as we don't forget.

[DieterHolvoet]

Does that mean that the backwards compatibility break doesn't apply for Drush?

[greg-1-anderson]

Backwards compatibility is only enforced for defined behavior. If you use a feature that's not supposed to be a feature because it's part of undefined behavior, then continuing use of that feature is not guaranteed. Command classes that need to have variable behavior based on environment should use hasa rather than isa (i.e. use dependency injection) in the implementation. Inheritance wasn't considered; it was neither specifically supported nor explicitly prohibited. For the library, we should be permissive. I'm undecided about whether to default this to true in the 5.x version of the library, but I'm inclined to let it go. It's safe to do this in the library because we don't provide any command classes. If people want to design their command classes to use inheritance to extend the behavior, that's on them.

Drush, on the other hand, has a lot of command classes. None of them were designed to be extended. If we allow folks to do this, internal changes to Drush could break folks using this undefined behavior. I think it's much safer to prohibit the extension of command classes in Drush by disabling the undefined behavior that we cannot support.

If you have a strong need for this feature to keep working in the context of Drush, appeal to @weitzman, he has final say. All we need to do in that instance is close this PR as will-not-fix. I recommend that we merge it instead.

[greg-1-anderson]

I middle ground could be to add final to all of our command classes, and continue to let modules extend their command classes. I don't really like this idea either, but it's one level safer than just punting entirely.

[DieterHolvoet]

Thanks for your explanation, makes sense! I'm not opposed to merging this, I was just curious about your reasoning.

[weitzman]

A middle ground could be to add final to all of our command classes, and continue to let modules extend their command classes.

I like that. I'm inclined to not break anyone's install. Folks can watch #5283 for progress.

[greg-1-anderson]

Annotated Command library 4.6.1 released; closing here.