Come together with the global Drupal community in Rotterdam, 28 Sept – 1 Oct 2026. Sessions, contribution, connection, and Early Bird savings until 8 June.Problem/Motivation
Discovered via PHPStan in #3178534: Start running PHPStan on Drupal core (level 0):
Line core/tests/Drupal/Tests/Component/Annotation/PluginIdTest.php
------ -------------------------------------------------------------------------------------------------------------------------
19 Class Drupal\Component\Annotation\PluginID does not have a constructor and must be instantiated without any parameters.
24 Class Drupal\Component\Annotation\PluginID does not have a constructor and must be instantiated without any parameters.
50 Class Drupal\Component\Annotation\PluginID does not have a constructor and must be instantiated without any parameters.
The test checks that instantiating the class with parameters does nothing, but as there is no constructor then this test is meaningless.
Steps to reproduce
Proposed resolution
Remaining tasks
User interface changes
API changes
Data model changes
Release notes snippet
| Comment | File | Size | Author |
|---|---|---|---|
| #2 | 3205037.patch | 1.09 KB | longwave |
Comments
Comment #2
longwaveComment #3
quietone commentedThis looks correct to me.
Comment #4
alexpottWell the comment does say
// Assert plugin starts empty regardless of constructor.That sounds intentional to me. I think we need to do the git archaeology to see why this was added.
Comment #5
longwaveNot really sure what that comment is about nor what it is trying to test.
#2435607: Tests for Annotation Component is where this test was introduced, and this wasn't commented on at the time although it's present from the first patch in the issue.
The PluginID annotation was introduced in #1965164: Add a @PluginID annotation for plugins that only need an ID, like Views handlers and it has never had a constructor.
PluginID was converted to extend AnnotationBase in #2005716: Promote EntityType to a domain object but that has never had a constructor either.
So really I have no idea what that comment is supposed to mean or why this test was ever added!
Comment #6
neclimdulLooks like maybe I was assuming doctrine would throw any values into the constructor... or something? Annotation parsing checks there's a constructor before doing that even back then though so it didn't do anything when this was committed and looks like well before. Also, that wouldn't have covered anything. I have to wonder if that came out of a discussion or something... very weird.
+1 to the changes, doesn't really make sense the way its written.
Comment #7
alexpottBackported 9.1.x to keep tests aligned.
Committed and pushed 70eb124d5f to 9.2.x and 43136cd9e8 to 9.1.x. Thanks!