Updated: Comment 0

Problem/Motivation

If you write any kind of plugin you need to put this two classes into the file in order to make it working.

Before

<?php
use Drupal\Core\Entity\Annotation\EntityType;
use
Drupal\Core\Annotation\Translation;
/**
* @EntityType(
*/
?>

Proposed resolution

Add the @Translation and @{$PluginType} automatically, so the use statement is not needed anymore.

Some more information: http://docs.doctrine-project.org/projects/doctrine-common/en/latest/refe...

After

<?php
/**
* @EntityType(
*/
?>

Remaining tasks

Should we remove all the use statements all over core in this issue

User interface changes

API changes

Files: 
CommentFileSizeAuthor
#13 plugins-2090353-13.patch7.81 KBdawehner
PASSED: [[SimpleTest]]: [MySQL] 58,327 pass(es).
[ View ]
#13 interdiff.txt2.7 KBdawehner
#10 interdiff.txt3.75 KBdawehner
#10 plugin-2090353-10.patch8.06 KBdawehner
PASSED: [[SimpleTest]]: [MySQL] 59,264 pass(es).
[ View ]
#6 plugin-2090353-6.patch5.99 KBdawehner
PASSED: [[SimpleTest]]: [MySQL] 59,171 pass(es).
[ View ]
#6 interdiff.txt1005 bytesdawehner
#4 plugin-2090353-4.patch5.98 KBdawehner
PASSED: [[SimpleTest]]: [MySQL] 59,191 pass(es).
[ View ]
#4 interdiff.txt2.56 KBdawehner
#1 drupal-2090353-1.patch6.26 KBdawehner
PASSED: [[SimpleTest]]: [MySQL] 59,215 pass(es).
[ View ]

Comments

Status:Active» Needs review
StatusFileSize
new6.26 KB
PASSED: [[SimpleTest]]: [MySQL] 59,215 pass(es).
[ View ]

There we go, manually installing standard works for that.

The magic code which makes this possible is in DocParser:

<?php
           
if ($this->namespaces) {
                foreach (
$this->namespaces as $namespace) {
                    if (
$this->classExists($namespace.'\\'.$name)) {
                       
$name = $namespace.'\\'.$name;
                       
$found = true;
                        break;
                    }
                }
?>

The reason why I had to switch to SimpleAnnotation is because the normal AnnotationReader class does not allow you to specify the namespaces.

Issue tags:+Prague DX smackdown

Issue tags:-Prague DX smackdown
StatusFileSize
new2.56 KB
new5.98 KB
PASSED: [[SimpleTest]]: [MySQL] 59,191 pass(es).
[ View ]

Removed the changes in doctrine itself.

+++ b/core/vendor/doctrine/annotations/lib/Doctrine/Common/Annotations/AnnotationReader.php
@@ -94,14 +94,14 @@ static public function addGlobalIgnoredName($name)
+    protected $preParser;
...
+    protected $phpParser;
@@ -315,4 +315,27 @@ private function collectParsingMetadata(ReflectionClass $class)
+
...
+

I really like this patch, I think just being able to use the core annotation classes is great, and the various plugin implementations using their own (Like Views, ViewsDisplayPlugin etc..) can almost be enforced (kind of).

I guess this is the only issue... :) Have you created an upstream patch?

StatusFileSize
new1005 bytes
new5.99 KB
PASSED: [[SimpleTest]]: [MySQL] 59,171 pass(es).
[ View ]

Chx suggested to use that piece of code to get the namespace.

Status:Needs review» Reviewed & tested by the community

So. Nice!

Oh, you don't even need to. Great. Looks good!

Edit: I cross posted in #5 before I saw the patch after it.

This is insanely awesome.

EDIT:
To clarify to committers, this only removes a handful of "use" annotation lines to keep the patch down. But this essentially means we'll never need to "use" an annotation in any file.

Status:Reviewed & tested by the community» Needs review
StatusFileSize
new8.06 KB
PASSED: [[SimpleTest]]: [MySQL] 59,264 pass(es).
[ View ]
new3.75 KB

Let's move the drupal core specific code into the core namespace.

+++ b/core/lib/Drupal/Component/Plugin/Discovery/AnnotatedClassDiscovery.phpundefined
@@ -7,9 +7,9 @@
+use Doctrine\Common\Annotations\SimpleAnnotationReader;

This isn't currently used in the Component at all.

+++ b/core/lib/Drupal/Component/Plugin/Discovery/AnnotatedClassDiscovery.phpundefined
@@ -51,6 +58,19 @@ function __construct($plugin_namespaces = array(), $plugin_definition_annotation
+    if (!isset($this->annotationReader)) {
+      $this->annotationReader = new AnnotationReader();
+    }
+    return $this->annotationReader;

I really expected to see the SimpleAnnotationReader here. It's strange that we then override this completely in the Core portion of the code. I expected the namespace of the pluginDefinitionAnnotationName to be added to the reader here and have the Core portion of the code call parent::getAnnotationReader().

+++ b/core/lib/Drupal/Core/Plugin/Discovery/AnnotatedClassDiscovery.phpundefined
@@ -57,6 +58,23 @@ function __construct($subdir, \Traversable $root_namespaces, $plugin_definition_
+  protected function getAnnotationReader() {
+    if (!isset($this->annotationReader)) {
+      $this->annotationReader = new SimpleAnnotationReader();
+
+      // Add the Core annotation classes like @Translation.
+      $this->annotationReader->addNamespace('Drupal\Core\Annotation', array(DRUPAL_ROOT . '/core/lib/Drupal/Core/Annotation'));
+
+      // Add the namespaces from the main plugin annotation, like @EntityType.
+      $namespace = substr($this->pluginDefinitionAnnotationName, 0, strrpos($this->pluginDefinitionAnnotationName, '\\'));
+      $this->annotationReader->addNamespace($namespace);
+    }
+    return $this->annotationReader;

Wondering if we can get a sanity check from donquixote on this with regards to PSR-4. If that all checks out, then I'm on board, but I really do think that the substr()/addNamespace() combo here belongs in the Component.

Otherwise I generally like the basic approach here.

Eclipse

Status:Needs review» Needs work

oops

Eclipse

Status:Needs work» Needs review
StatusFileSize
new2.7 KB
new7.81 KB
PASSED: [[SimpleTest]]: [MySQL] 58,327 pass(es).
[ View ]

My idea was to make the component as generic as possible, but sure we can just do it like that.

Status:Needs review» Needs work
Issue tags:-DX (Developer Experience)

The last submitted patch, plugins-2090353-13.patch, failed testing.

Status:Needs work» Needs review
Issue tags:+DX (Developer Experience)

#13: plugins-2090353-13.patch queued for re-testing.

Status:Needs review» Reviewed & tested by the community

Great points @EclipseGc, great work @dawehner.

+++ b/core/lib/Drupal/Component/Plugin/Discovery/AnnotatedClassDiscovery.php
@@ -63,10 +86,8 @@ public function getDefinition($plugin_id) {
-    // Prevent @endlink from being parsed as an annotation.
-    $reader->addGlobalIgnoredName('endlink');
-    $reader->addGlobalIgnoredName('file');

I love that we get to skip this completely.

++ing the RTBC

Eclipse

Title:Don't require to put the use statement into plugin classesChange notice: Don't require to put the use statement into plugin classes
Status:Reviewed & tested by the community» Active
Issue tags:+Needs change record

HOLY CRAP I LOVE THIS PATCH SO MUCH!!!!

Committed and pushed to 8.x. YEAH!

Needs a change notice.

Here is one: [#2096117]
I am wondering whether we should change all the existing ones for all the various plugin ones.

Yeah, I mean, low priority, but it seems like the sort of thing we should go ahead and begin implementing in our plugins.

Eclipse

Just a silly question Can we do this for contirb as well?

Just a silly question Can we do this for contirb as well?

This works for all plugin annotations you define, if your plugin manager supports it specify the plugin annotation. In other
words core modules are just a special case of modules, so contrib modules will just work as well.

is there another way to do #1862202-172: Objectify the language system without putting the full namespace there?
not even the current namespace of the class is allowed:(
also other use statements in your class are not imported, which used to

Edit opened #2097585: Allow plugin managers to register namespaces to the annotation reader for this

Issue summary:View changes

blub

Title:Change notice: Don't require to put the use statement into plugin classesDon't require to put the use statement into plugin classes
Issue summary:View changes
Status:Active» Fixed

This has a change notice, so I think we're good here?

Status:Fixed» Closed (fixed)

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

Issue tags:-Needs change record