There are plugin annotations that are implemented as subclasses of other annotation classes in:
In each instance, the same plugin manager retrieves definitions for the plugins of the class and subclasses together. Currently, subclasses can not be handled because of this in Drupal\Core\Plugin\Discovery\AttributeDiscoveryWithAnnotations::parseClass():
// Annotations use static reflection and are able to analyze a class that
// extends classes or uses traits that do not exist. Attribute discovery
// will trigger a fatal error with such classes, so only call it if the
// class has a class attribute.
if ($reflection_class->hasClassAttribute($this->pluginDefinitionAttributeName)) {
return parent::parseClass($class, $fileinfo);
}
hasClassAttribute returns TRUE if the Class .php file contains an attribute that is an exact string match for $this->pluginDefinitionAttributeName, case-insensitive. This means that plugins with the subclass attribute fail this check, and the class is not picked up as a plugin.
Instead of using a case-insensitive exact string match, change implementation of hasClassAttribute to use is_a() to check that the attribute class is the same class or subclass of $this->pluginDefinitionAttributeName.
Show commandsStart within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
changes
Comments
Comment #3
godotislateMR is hitting what looks like an unrelated test failure.
Comment #4
godotislateAddressed MR comment. Failing test was resolved upstream.
Comment #5
berdir> 3427388-update-drupalcomponentannotationdoctrinestaticreflectionparserhasclassattribute-to
This might be the longest branch name I've ever seen, certainly the longest "word" in a branch name ever ;)
I usually just put the class name and not full namespace in issue titles, but I don't know if that's an official/common thing or just me ;)
I think the one thing I'd like to see is that this doesn't trigger a class not found error. I think we can test that if we add something like #NonExistingAttribute on one of the test classes, should just ignore that then. I'm not 100% sure I understand the docs around is_a() and when it does trigger the classloader and when not.
Comment #6
godotislateI have a habit of being as specific as possible, though there is no other StaticReflectionParser, so I really didn't need to. I regretted it once I saw the generated branch name.
OK, that's what you meant before, because I saw there were tests cases for nonexistent attribute names, but it went the other way. Added a test case and class for that and pushed a commit. We'll see if PHPStan yells.
Comment #7
godotislatePHPStan didn't like the nonexistent class, but I set it to ignore that line, and we're green again.
Comment #8
berdirSometimes we also ignore files through core/phpstan.neon.dist, I guess either works, also plenty of examples using a comment. All of those use a @phpstan-ignore-next-line though, with comment above as we don't do same-line comments, so I'd suggest to adjust that here as well. Also works as a class dockblock, per example in core/tests/Drupal/Tests/Core/Test/TestSuiteBaseTest.php.
Comment #9
berdirI wrote basically the exact same changes while trying to get #3396166: Convert entity type discovery to PHP attributes working again, @alexpott also +1'd the changes in Slack, so I think this is ready and unblocks several issues.
Comment #10
alexpottCommitted and pushed 0356ea590c to 11.x and 0be35cacd6 to 10.3.x and cbbdff7008 to 10.2.x. Thanks!
Backported to 10.2.x as this is a bugfix - something works with annotations that doesn't with attributes. It'll help contrib the earlier this is works correctly.
Nice test coverage!