ContextAwarePluginBase compromised after commit of Core PluginBase

As my issue #2176275: Fatal thrown: Cannot use ... PluginBase because the name is already in use was closed as a duplicated of this one this really needs a better report.

• What was compromised?
• What steps to reproduce?
• Is there an exception and is it the same as reported in #2176275: Fatal thrown: Cannot use ... PluginBase because the name is already in use?
• The current patch 'ditches' the alias same as I tried but we need to understand the root cause.
• As tim.plunkett suggest tests in #2176275-12: Fatal thrown: Cannot use ... PluginBase because the name is already in use I've added the tag.

Not going to demote, but I don't see why we wouldn't provide a unit test for ContextAwarePluginBase in general which clearly would have caught this problem?!

As I've reported in #2176275: Fatal thrown: Cannot use ... PluginBase because the name is already in use my error is gone. I cannot reproduce it. And this patch is not in yet. So we can be sure we will have this kind of bug again in the near future.

When people search for " because the name is already in use " they land on a duplicate issue.

This issues summary deserves at least the exception thrown and steps to reproduce.

I'm fine with no tests as we currently have a solution (workaround) to replace the 'existing' alias into a non-existing.

But it still is a nasty issue.

@EclipseGc you're not very helpful :-(

I asked in #2 whether your exception was the same and still have no clue. So I cannot update the summary on your behalf either.

@EclipseGc thanks for your context.

I'll try to update the summary during the sprint weekend :)

Thanks for the explanation in #6: I was under the impression that generic unit tests for ContextAwarePluginBase i.e. for setContextValue() would have caught this bug. If that's not the case - or not *necessarily* the case - there is absolutely no point in providing those tests *in this issue*.

Well according to http://www.php.net/manual/en/language.namespaces.faq.php#language.namesp... this should not cause any problems at all. It should use the correct class. So setting back to needs work as I don't think that the test failures experienced can be because of this.

I tried to write a test for this but failed. My Graph API has 4 PHP Fatals

Drupal\Core\Executable\ExecutablePluginBase
Drupal\Core\Plugin\ContextAwarePluginBase
Drupal\language\Plugin\Condition\Language
Drupal\node\Plugin\Condition\NodeType

of which the deepest stacktrace is

PHP  10. Drupal\Core\Cron->run() /Users/clemens/Sites/drupal-devs/drush/commands/core/core.drush.inc:696
PHP  11. call_user_func_array() /Users/clemens/Sites/drupal/d8/www/core/lib/Drupal/Core/Cron.php:137
PHP  12. graphapi_class_generator_worker() /Users/clemens/Sites/drupal/d8/www/core/lib/Drupal/Core/Cron.php:137
PHP  13. graphapi_class_build_class() /Users/clemens/Sites/drupal/d8/modules/graphapi/modules/graphapi_class/graphapi_class.module:249
PHP  14. Fhaculty\Graph\Uml\ClassDiagramBuilder->createVertexClass() /Users/clemens/Sites/drupal/d8/modules/graphapi/modules/graphapi_class/graphapi_class.module:153
PHP  15. ReflectionClass->__construct() /Users/clemens/Sites/drupal/d8/modules/graphapi/vendor/clue/graph-uml/src/Fhaculty/Graph/Uml/ClassDiagramBuilder.php:81
PHP  16. Composer\Autoload\ClassLoader->loadClass() /Users/clemens/Sites/drupal/d8/modules/graphapi/vendor/clue/graph-uml/src/Fhaculty/Graph/Uml/ClassDiagramBuilder.php:0
PHP  17. include() /Users/clemens/Sites/drupal/d8/www/core/vendor/composer/ClassLoader.php:269
PHP  18. Composer\Autoload\ClassLoader->loadClass() /Users/clemens/Sites/drupal/d8/www/core/vendor/composer/ClassLoader.php:0
PHP  19. include() /Users/clemens/Sites/drupal/d8/www/core/vendor/composer/ClassLoader.php:269
PHP  20. Composer\Autoload\ClassLoader->loadClass() /Users/clemens/Sites/drupal/d8/www/core/vendor/composer/ClassLoader.php:0
PHP  21. include() /Users/clemens/Sites/drupal/d8/www/core/vendor/composer/ClassLoader.php:269
PHP  22. Composer\Autoload\ClassLoader->loadClass() /Users/clemens/Sites/drupal/d8/www/core/vendor/composer/ClassLoader.php:0
Drush command terminated abnormally due to an unrecoverable error.                                  [error]
Error: Cannot use Drupal\Component\Plugin\ContextAwarePluginBase as PluginBase because the name is already in use in /Users/clemens/Sites/drupal/d8/www/core/lib/Drupal/Core/Plugin/ContextAwarePluginBase.php, line 10

This shows I guess we needs to investigate into the autoloader process.

Using the classes through Graph API input filter generates the same fatal.

Attached the class diagram showing the classes in err. Note the double extends between Drupal\Component\Plugin\ContextAwarePluginBase and Drupal\Core\Plugin\ContextAwarePluginBase

As reported in #2176275: Fatal thrown: Cannot use ... PluginBase because the name is already in use the my track trace fails through Reflection

 [edited for brevity]
[Wed Feb 05 17:44:40 2014] [error] [client 127.0.0.1]

PHP  22. ReflectionClass->__construct() /.../modules/graphapi/vendor/clue/graph-uml/src/Fhaculty/Graph/Uml/ClassDiagramBuilder.php:81, referer: http://drupal.d8/admin/config/system/cron

PHP  23. Composer\\Autoload\\ClassLoader->loadClass() /.../modules/graphapi/vendor/clue/graph-uml/src/Fhaculty/Graph/Uml/ClassDiagramBuilder.php:0, referer: http://drupal.d8/admin/config/system/cron

PHP  24. include() /.../core/vendor/composer/ClassLoader.php:269, referer: http://drupal.d8/admin/config/system/cron

PHP  25. Composer\\Autoload\\ClassLoader->loadClass() /.../core/vendor/composer/ClassLoader.php:0, referer: http://drupal.d8/admin/config/system/cron

PHP  26. include() /.../core/vendor/composer/ClassLoader.php:269, referer: http://drupal.d8/admin/config/system/cron

PHP  27. Composer\\Autoload\\ClassLoader->loadClass() /.../core/vendor/composer/ClassLoader.php:0, referer: http://drupal.d8/admin/config/system/cron

My plea would be to commit this patch as I don't think we will find the root cause.

And it is at least bad practise to use an alias the way we do now.

OTOT we do have a way to reproduce this through using Graph API

I updated the patch with the name I used in #2181999: Invalid alias for \Drupal\Component\Plugin\ContextAwarePluginBase in \Drupal\Core\Plugin\ContextAwarePluginBase so it is slightly more descriptive.

I also confirmed that the official PHP documentation says we should not be having this problem. I searched online and asked in #php to see if this is a version-specific bug, or if the documentation might be wrong, but I have not found or received an answer to that so far.

@Xano Ehm ... in #2181999: Invalid alias for \Drupal\Component\Plugin\ContextAwarePluginBase in \Drupal\Core\Plugin\ContextAwarePluginBase you found this bug while unit testing.

In #2176275: Fatal thrown: Cannot use ... PluginBase because the name is already in use we hoped for a test reproducing this.

Can you provide for a test (sketch) please?

See also #6 which sounds like a similar failure and #13 for a list of failure classes found till now.

+++ b/core/lib/Drupal/Core/Plugin/ContextAwarePluginBase.php
@@ -7,10 +7,9 @@
-use Drupal\Component\Plugin\Discovery\DiscoveryInterface;

Why is this removed?

@Xano Ehm ... in #2181999: Invalid alias for \Drupal\Component\Plugin\ContextAwarePluginBase in \Drupal\Core\Plugin\ContextAwarePluginBase you found this bug while unit testing.

The unit test itself wasn't what triggered the problem. It was only the context in which the problem manifested itself.

Why is this removed?

Because it was part of the previous patch.

Well according to http://www.php.net/manual/en/language.namespaces.faq.php#language.namesp... this should not cause any problems at all. It should use the correct class. So setting back to needs work as I don't think that the test failures experienced can be because of this.

@alexpott

I understand your point here, and yes the docs say it works, but I'm not experiencing a test failure, I'm experiencing a manual instantiation and usage failure, and this patch solves that, which leads me to believe that we have a php version specific issue. :-(

Eclipse

Based on what I've read and heard from different people, this is a parse-time error. @EclipseGc and I experience the failure when instantiating classes that extend \Drupal\Core\Plugin\ContextAwarePluginBase, and @clemens.tolboom experienced the same problem when analyzing a child class through reflection. These errors show up in tests, but they are not actual test failures and they are likely caused by a PHP problem.

My conclusion is that, until we know what causes this, we should probably commit the working fix, especially as it does not actually change how things work. It does, however, enable everybody to start unit-testing their conditions and other context-aware plugins, which is a good thing.

The patch in #2136197-17: Move field/instance/widget/formatter settings out of annotation / plugin definition triggers this failure during NodeConditionTest.

#2136197-21: Move field/instance/widget/formatter settings out of annotation / plugin definition fails in NodeConditionTest due to this issue.
#2136197-24: Move field/instance/widget/formatter settings out of annotation / plugin definition is the failing patch plus #18 from here, and that is now passing.
Therefore I think this is RTBC.

Being that this blocks #2136197: Move field/instance/widget/formatter settings out of annotation / plugin definition it is critical and the fix is trivial. Let's get this done.

Yeah :)

As we don't have yet php 5.4 for our testbot this failure is for both 5.3 and 5.4 (on Mac locally) we may conclude this failure of PHP should be reported. We did not nailed this through a 'steps to reproduce' yet but have some insight through the failing tests.

Should we create a followup issue for this? Or let us not chase herrings aka bygones?

This seems similar to https://bugs.php.net/bug.php?id=55068 - perhaps it was dependent on the order that the autoloader ended up including certain files?