Add internal, event, and property to the list of ignored annotations in the plugin annotation system

This patch illustrates a proof-of-concept solution.

Obviously we don't want to use reflection when we have a good solution in Doctrine\Common\Annotations\AnnotationReader already, which has an API for ignoring some annotations.

Add the autoloader to index.php, as in the original issue, and then observe the classes.txt file to see that these annotation-based classes are no longer being requested.

OK, so maybe reflection isn't such a bad idea. :-)

The only upstream change needed is that $parser be protected instead of private. We can subclass to our needs after that.

I had to look it up, but the reason we switched to SimpleAnnotationReader is stated in #2090353-2: Don't require to put the use statement into plugin classes.

The namespace addition lives on DocParser, and SimpleAnnotationReader just sets it. And of course AnnotationReader doesn't give us an accessor for this.

If $parser were protected (in either SimpleAnnotationReader or AnnotationReader, or both) we could just subclass and change it, while also getting the benefits of the superclass.

+++ b/core/lib/Drupal/Component/Annotation/Plugin/Discovery/AnnotatedClassDiscovery.php
@@ -71,6 +73,13 @@ protected function getAnnotationReader() {
     if (!isset($this->annotationReader)) {
       $this->annotationReader = new SimpleAnnotationReader();
 
+      AnnotationReader::addGlobalIgnoredName('see');
+      AnnotationReader::addGlobalIgnoredName('code');
+      AnnotationReader::addGlobalIgnoredName('endcode');
+      AnnotationReader::addGlobalIgnoredName('ingroup');
+      AnnotationReader::addGlobalIgnoredName('property');
+      AnnotationReader::addGlobalIgnoredName('todo');

I'm pretty sure SimpleAnnotationReader tells DocParser *not* to use those globally ignored names. Also, the global ignored names array already contains all our tags so we don't have to set them.

So basically it's either an upstream fix or we write our own version of AnnotationReader.

The upstream fix is one of two things: Give AnnotationReader a way to set the namespace, or make $parser protected so we can subclass to our own needs.

However, if you can apply the patch in #10 and then try to repro with the autoloader in the issue summary and get no requests for \Drupal\whatever\see then yay.

Here's a test. It will fail. :-)

Add more unsupported annotations as needed.

@alexpott's PR for Doctrine: https://github.com/doctrine/annotations/pull/56

This patch creates a new class called DrupalAnnotationReader. It's mostly a copy-paste of Doctrine's SimpleAnnotationReader, with all the extra stuff we want.

It includes the test from #14, the ignore list from AnnotationReader, and some of @alexpott's modifications from https://github.com/doctrine/annotations/pull/56

That PR for Doctrine has been around since late April, so it doesn't look like they're moving very quickly.

Will do coding standards stuff if this comes back green and there's some consensus.

Here's the test in #14 combined with the solution in #10, which fails.

That's #17. :-)

I thought I might try to tract the intractable again.

There are still a few fails. The most interesting one is Drupal\system\Tests\Plugin\Discovery\AnnotatedClassDiscoveryTest which finds the @file annotation and thinks it's a plugin annotation.

Hi, I'm Mr. Testing. But I neglected to just run the test, which passes. :-)

Re-running the experiment in the issue summary I see no non-plugin annotations slipping in to the autoload, so I think this issue is done. But if anyone wants the test, here it is.

Valid for 8.2.x beta because it's a test improvement.

Removed @file docblock.

Test still passes.

+++ b/core/tests/Drupal/Tests/Component/Plugin/Discovery/AnnotatedClassDiscoveryTest.php
@@ -68,7 +71,7 @@
-      ['\\DrupalTest\\TestClass' => [vfsStream::url('test_class/DrupalTest')]], '\\DrupalTest\\Component\\Annotation\\'
+      ['\\DrupalTest\\TestClass' => [vfsStream::url('root/DrupalTest')]], '\\DrupalTest\\Component\\Annotation\\'

Good catch.

1) Drupal\Tests\Component\Plugin\Discovery\AnnotatedClassDiscoveryTest::testAutoloadBadAnnotations
Attempted to autoload a non-plugin annotation: addtogroup

So it looks like we do try to autoload all annotation.

Do we want to go back to the earlier attempts at rolling our own reader?

Converted to a dataprovider so we can test all the testcases.

They all still fail except for { and }.

Forgot the interdiff.

I just opened this PR: https://github.com/doctrine/annotations/pull/211

I opened it because DocParser *never* ignores annotations when you also have namespaces, so therefore we can't even roll our own annotation reader.