Backport Hook and LegacyHook Attribute

nicxvan → changed the visibility of the branch 3482464-backport-hook-and to hidden.

This is 100% eligible for 10.4.

Need to get a second/third opinion from other committers about 11.0 and 10.3 - to me it makes sense because these classes will do literally nothing, they can even be empty (not that we should necessarily make them empty because that could be more confusing for people than having them the same as the 11.x versions).

The Needs Review Queue Bot → tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide → to find step-by-step guides for working with issues.

Adding it to 10.3 and 11.0 would allow the work on converting contrib modules to get underway using BC method - this is going to be quite the code churn leading up to Drupal 12.

Adding it to 11.0 is a no brainer - if it's added to 10.4, then it has to also exist in 11.0. Even if sites are expected to upgrade from 10.4 to 11.1, it's a real disconnect for a class not to appear in an intermediate version. It's harmless to add the classes. Any contrib module would need an exception to say "works with 10.4 through 11.1, but don't try it with 11.0 or it will WSOD your site". There will be D11.0 sites trying to use that code.

It would be nice to add it to 10.3 (so I can start coding BC OO hooks) but there's no compelling reason it needs to be added. No contrib marked <=10.3 is going to have any of these BC OO hook wraps that it needs the classes to exist. Any contrib release that adds them would already need to state ^10.4.

I hope it gets in all three versions too.

works with 10.4 through 11.1, but don't try it with 11.0 or it will WSOD your site

Just a note the worst that should happen is phpstan complaining the attributes are not used until 11.1

PHP always surprises me; it really does just ignore an Attribute class not existing. I guess that's why the syntax is #[Blah] so that older versions ignore it as a comment.

The syntax to ignore is for older PHP versions indeed.

But nonexisting attribute classes are ignored because PHP itself doesn't use attributes. Indeed, you can even run reflection against a function marked with LegacyHook in the absence of LegacyHook class, you can even retrieve the name of the attribute and it won't blow up until you try to instantiate it. There's no reason to run a class_exists against attributes. Much the same with use. It's just a string up to the point when you try to use it as an actual classname to instantiate. https://3v4l.org/sK4If As visible from this if we wanted to make a mess we didn't need to create the LegacyHook class at all, you can just check for the hook name whether it exists or not.

This is why I said people stuck on 10.1 or something already are fine to use it or to use contrib using this attribute -- it just doesn't matter.

Would be helpful to get this in, I understand that it's not strictly required, but phpstan complains and that will confuse a lot of people.

That said, I would recommend we do not just copy paste the documentation from 11.1, that is IMHO very confusing, but I'd epect that it would actually do something then on 10.x/11.0

I would clearly state that the two attribute classes only exist in those core versions that people are looking at to satisfy phpstan and as a place to document that it doesn't actually do anything beside that before 11.1.

Yep agreed with #16.

Do you just want the class comments updated?

How about something like:

This class will not have an effect until Drupal 11.1.0.

This class is to prevent phpstan errors for modules that support procedural and #Hook implementations.

That looks good - maybe the below is a bit more explicit?

This class is included in earlier Drupal versions to prevent phpstan errors for modules implementing object oriented hooks using the #Hook and #LegacyHook attributes.

Ok let me update those.

RTBC for me. I wondered about removing the rest of the phpdoc (and maybe even the constructor itself) too but not sure there's much benefit to that, you either stop at the class summary or you don't.

After commit please delete https://www.drupal.org/node/3442349#comment-15877121 →

I RTBCd this but it was two weeks ago and there's been no further feedback. We need this in the 10.4.0 release and the next 11.0 patch release.

Committed/pushed to 11.0.x/10.5.x/10.4.x/10.3.x - 10.2.x is about to go out of support and won't get any more patch releases.

catch → closed merge request !9908

Automatically closed - issue fixed for 2 weeks with no activity.