HEAD BROKEN: Fatal error: Cannot use Drupal\Component\Utility\Xss as Xss because the name is already in use in /var/lib/drupaltestbot/sites/default/files/checkout/core/modules/views/src/Plugin/views/field/Field.php on line 11

TestBot run this.

RTBC for #8.

/me is tired

Hiding test files.

I do not understand at all why this patch fixes whatever problem but nevertheless committed and pushed to 8.0.x. Thanks!

Oops I guess we never said directly on the issue... There is a Drupal\views\Plugin\views\field\Xss, in the same namespace as this class, Drupal\views\Plugin\views\field. So it collides with the core utility in Drupal\Component\Utility\Xss. In PluginInstanceTest::testPluginInstances(), a reflection of each of the plugins of the given type is checked and then an instance is created.

So with HEAD locally, the plugins are tested in alphabetical order and all is well:

    Instance of field:bulk_form created
Pass      Other      PluginInstanceTes   95 Drupal\views\Tests\PluginInstanceTe
    Instance of field:boolean created
Pass      Other      PluginInstanceTes   95 Drupal\views\Tests\PluginInstanceTe
    Instance of field:broken created
Pass      Other      PluginInstanceTes   95 Drupal\views\Tests\PluginInstanceTe
    Instance of field:counter created
Pass      Other      PluginInstanceTes   95 Drupal\views\Tests\PluginInstanceTe
    Instance of field:custom created
Pass      Other      PluginInstanceTes   95 Drupal\views\Tests\PluginInstanceTe
    Instance of field:date created
Pass      Other      PluginInstanceTes   95 Drupal\views\Tests\PluginInstanceTe
    Instance of field:dropbutton created
Pass      Other      PluginInstanceTes   95 Drupal\views\Tests\PluginInstanceTe
    Instance of field:entity_label created
Pass      Other      PluginInstanceTes   95 Drupal\views\Tests\PluginInstanceTe
    Instance of field:entity_operations created
Pass      Other      PluginInstanceTes   95 Drupal\views\Tests\PluginInstanceTe
    Instance of field:field created
Pass      Other      PluginInstanceTes   95 Drupal\views\Tests\PluginInstanceTe
    Instance of field:file_size created
Pass      Other      PluginInstanceTes   95 Drupal\views\Tests\PluginInstanceTe
    Instance of field:language created
Pass      Other      PluginInstanceTes   95 Drupal\views\Tests\PluginInstanceTe
    Instance of field:machine_name created
Pass      Other      PluginInstanceTes   95 Drupal\views\Tests\PluginInstanceTe
    Instance of field:markup created
Pass      Other      PluginInstanceTes   95 Drupal\views\Tests\PluginInstanceTe
    Instance of field:numeric created
Pass      Other      PluginInstanceTes   95 Drupal\views\Tests\PluginInstanceTe
    Instance of field:serialized created
Pass      Other      PluginInstanceTes   95 Drupal\views\Tests\PluginInstanceTe
    Instance of field:standard created
Pass      Other      PluginInstanceTes   95 Drupal\views\Tests\PluginInstanceTe
    Instance of field:time_interval created
Pass      Other      PluginInstanceTes   95 Drupal\views\Tests\PluginInstanceTe
    Instance of field:url created
Pass      Other      PluginInstanceTes   95 Drupal\views\Tests\PluginInstanceTe
    Instance of field:xss created
Pass      Other      PluginInstanceTes   95 Drupal\views\Tests\PluginInstanceTe
 

Applying @larowlan's array_reverse():

diff --git a/core/modules/views/src/Tests/PluginInstanceTest.php b/core/modules/views/src/Tests/PluginInstanceTest.php
index 1893432..2532a3f 100644
--- a/core/modules/views/src/Tests/PluginInstanceTest.php
+++ b/core/modules/views/src/Tests/PluginInstanceTest.php
@@ -83,7 +83,7 @@ public function testPluginInstances() {
     foreach ($this->definitions as $type => $plugins) {
       // Get a plugin manager for this type.
       $manager = $this->container->get("plugin.manager.views.$type");
-      foreach ($plugins as $id => $definition) {
+      foreach (array_reverse($plugins) as $id => $definition) {
         // Get a reflection class for this plugin.
         // We only want to test true plugins, i.e. They extend PluginBase.
         $reflection = new \ReflectionClass($definition['class']);

Then the fatal happens when it gets to Field:

   Instance of field:xss created
Pass      Other      PluginInstanceTes   95 Drupal\views\Tests\PluginInstanceTe
    Instance of field:url created
Pass      Other      PluginInstanceTes   95 Drupal\views\Tests\PluginInstanceTe
    Instance of field:time_interval created
Pass      Other      PluginInstanceTes   95 Drupal\views\Tests\PluginInstanceTe
    Instance of field:standard created
Pass      Other      PluginInstanceTes   95 Drupal\views\Tests\PluginInstanceTe
    Instance of field:serialized created
Pass      Other      PluginInstanceTes   95 Drupal\views\Tests\PluginInstanceTe
    Instance of field:numeric created
Pass      Other      PluginInstanceTes   95 Drupal\views\Tests\PluginInstanceTe
    Instance of field:markup created
Pass      Other      PluginInstanceTes   95 Drupal\views\Tests\PluginInstanceTe
    Instance of field:machine_name created
Pass      Other      PluginInstanceTes   95 Drupal\views\Tests\PluginInstanceTe
    Instance of field:language created
Pass      Other      PluginInstanceTes   95 Drupal\views\Tests\PluginInstanceTe
    Instance of field:file_size created
ail      PHP Fatal  Field.php           11 Unknown                            
    Cannot use Drupal\Component\Utility\Xss as Xss because the name is already
Fail      Fatal erro Unknown              0 Unknown                            
    [05-Feb-2015 11:29:43 Europe/Berlin] PHP Stack trace:
Fail      Fatal erro Unknown              0 Unknown                            
    [05-Feb-2015 11:29:43 Europe/Berlin] PHP   1. {main}()
    /Applications/MAMP/htdocs/d8git/core/scripts/run-tests.sh:0
Fail      Fatal erro Unknown              0 Unknown                            
    [05-Feb-2015 11:29:43 Europe/Berlin] PHP   2.
    simpletest_script_run_one_test()
    /Applications/MAMP/htdocs/d8git/core/scripts/run-tests.sh:39
Fail      Fatal erro Unknown              0 Unknown                            
    [05-Feb-2015 11:29:44 Europe/Berlin] PHP   3.
    Drupal\simpletest\TestBase->run()
    /Applications/MAMP/htdocs/d8git/core/scripts/run-tests.sh:642
Fail      Fatal erro Unknown              0 Unknown                            
    [05-Feb-2015 11:29:44 Europe/Berlin] PHP   4.
    Drupal\views\Tests\PluginInstanceTest->testPluginInstances()
    /Applications/MAMP/htdocs/d8git/core/modules/simpletest/src/TestBase.php:977
Fail      Fatal erro Unknown              0 Unknown                            
    [05-Feb-2015 11:29:44 Europe/Berlin] PHP   5. ReflectionClass->__construct()
    /Applications/MAMP/htdocs/d8git/core/modules/views/src/Tests/PluginInstanceTest.php:89
Fail      Fatal erro Unknown              0 Unknown                            
    [05-Feb-2015 11:29:44 Europe/Berlin] PHP   6. spl_autoload_call()
    /Applications/MAMP/htdocs/d8git/core/modules/views/src/Tests/PluginInstanceTest.php:89
Fail      Fatal erro Unknown              0 Unknown                            
    [05-Feb-2015 11:29:44 Europe/Berlin] PHP   7.
    Composer\Autoload\ClassLoader->loadClass()
    /Applications/MAMP/htdocs/d8git/core/modules/views/src/Tests/PluginInstanceTest.php:0
Fail      Fatal erro Unknown              0 Unknown                            
    [05-Feb-2015 11:29:44 Europe/Berlin] PHP   8.
    Composer\Autoload\includeFile()
    /Applications/MAMP/htdocs/d8git/core/vendor/composer/ClassLoader.php:278

Line 89 is the creation of the reflection:

        $reflection = new \ReflectionClass($definition['class']);

So when the Xss reflection is created first, it pukes making a reflection of Field with a different class named Xss aliased inside it.

Apparently some testbot environment change is causing the definition order to change for:

    $this->definitions = Views::getPluginDefinitions();
     foreach ($plugins as $id => $definition) {

I guess (not sure of this part) with the array manipulation around a closure in Views::getPluginTypes():

 return array_keys(array_filter(static::$plugins, function($plugin_type) use ($type) {
      return $plugin_type == $type;
    }));

The use of reflection combined with some PHP weirdness combined with Views' general violation of our class naming policy is what made the "magic" happen here.

FWIW, I think it'd be worth a followup to this hotfix -- since the fatal only happens because of the way the test uses reflection, I think, and we don't know from inside the Field plugin why we need to alias core's Xss utility as the Views Xss plugin isn't used there. The autoloader isn't what's causing this.

Discussed with @alexpott a bit who suggested:

I think this must down to the scope that reflection operates in - which is not isolated from the current state

which might imply that we should change the test instead (or additionally).

FWIW, the reflection scope is not just limited to the function scope. I thought of fixing it like this:

diff --git a/core/modules/views/src/Tests/PluginInstanceTest.php b/core/modules/views/src/Tests/PluginInstanceTest.php
index 1893432..b78ed4d 100644
--- a/core/modules/views/src/Tests/PluginInstanceTest.php
+++ b/core/modules/views/src/Tests/PluginInstanceTest.php
@@ -83,19 +83,23 @@ public function testPluginInstances() {
     foreach ($this->definitions as $type => $plugins) {
       // Get a plugin manager for this type.
       $manager = $this->container->get("plugin.manager.views.$type");
-      foreach ($plugins as $id => $definition) {
-        // Get a reflection class for this plugin.
-        // We only want to test true plugins, i.e. They extend PluginBase.
-        $reflection = new \ReflectionClass($definition['class']);
-        if ($reflection->isSubclassOf('Drupal\views\Plugin\views\PluginBase')) {
-          // Create a plugin instance and check what it is. This is not just
-          // good to check they can be created but for throwing any notices for
-          // method signatures etc... too.
-          $instance = $manager->createInstance($id);
-          $this->assertTrue($instance instanceof $definition['class'], format_string('Instance of @type:@id created', array('@type' => $type, '@id' => $id)));
-        }
+      foreach (array_reverse($plugins) as $id => $definition) {
+        $this->checkPluginInstance($id, $definition, $manager, $type);
       }
     }
   }
 
+  protected function checkPluginInstance($id, $definition, $manager, $type) {
+    // Get a reflection class for this plugin.
+    // We only want to test true plugins, i.e. They extend PluginBase.
+    $reflection = new \ReflectionClass($definition['class']);
+    if ($reflection->isSubclassOf('Drupal\views\Plugin\views\PluginBase')) {
+      // Create a plugin instance and check what it is. This is not just
+      // good to check they can be created but for throwing any notices for
+      // method signatures etc... too.
+      $instance = $manager->createInstance($id);
+      $this->assertTrue($instance instanceof $definition['class'], format_string('Instance of @type:@id created', array('@type' => $type, '@id' => $id)));
+    }
+  }

but that still throws the fatal upon creating the Field reflection.

So this is not about reflection... if you rewind to commit b8364ec and run the following script with drush

$manager = \Drupal::service("plugin.manager.views.field");
var_dump($manager->createInstance('xss')->getPluginDefinition());
var_dump($manager->createInstance('field')->getPluginDefinition());

You'll get the following backtrace

PHP Fatal error:  Cannot use Drupal\Component\Utility\Xss as Xss because the name is already in use in /Volumes/devdisk/dev/sites/drupal8alt.dev/core/modules/views/src/Plugin/views/field/Field.php on line 11
PHP Stack trace:
PHP   1. {main}() /Volumes/devdisk/dev/drush5/drush.php:0
PHP   2. drush_main() /Volumes/devdisk/dev/drush5/drush.php:16
PHP   3. Drush\Boot\DrupalBoot->bootstrap_and_dispatch() /Volumes/devdisk/dev/drush5/drush.php:76
PHP   4. drush_dispatch($command = *uninitialized*, $arguments = *uninitialized*) /Volumes/devdisk/dev/drush5/lib/Drush/Boot/DrupalBoot.php:46
PHP   5. call_user_func_array:{/Volumes/devdisk/dev/drush5/includes/command.inc:178}(*uninitialized*, *uninitialized*) /Volumes/devdisk/dev/drush5/includes/command.inc:178
PHP   6. drush_command(*uninitialized*) /Volumes/devdisk/dev/drush5/includes/command.inc:178
PHP   7. _drush_invoke_hooks($command = *uninitialized*, $args = *uninitialized*) /Volumes/devdisk/dev/drush5/includes/command.inc:210
PHP   8. call_user_func_array:{/Volumes/devdisk/dev/drush5/includes/command.inc:359}(*uninitialized*, *uninitialized*) /Volumes/devdisk/dev/drush5/includes/command.inc:359
PHP   9. drush_core_php_script(*uninitialized*) /Volumes/devdisk/dev/drush5/includes/command.inc:359
PHP  10. include() /Volumes/devdisk/dev/drush5/commands/core/core.drush.inc:1066
PHP  11. Drupal\views\Plugin\ViewsHandlerManager->createInstance($plugin_id = *uninitialized*, $configuration = *uninitialized*) /Volumes/devdisk/dev/sites/drupal8alt.dev/d.php:5
PHP  12. Drupal\Component\Plugin\PluginManagerBase->createInstance($plugin_id = *uninitialized*, $configuration = *uninitialized*) /Volumes/devdisk/dev/sites/drupal8alt.dev/core/modules/views/src/Plugin/ViewsHandlerManager.php:126
PHP  13. Drupal\Core\Plugin\Factory\ContainerFactory->createInstance($plugin_id = *uninitialized*, $configuration = *uninitialized*) /Volumes/devdisk/dev/sites/drupal8alt.dev/core/lib/Drupal/Component/Plugin/PluginManagerBase.php:63
PHP  14. Drupal\Component\Plugin\Factory\DefaultFactory::getPluginClass($plugin_id = *uninitialized*, $plugin_definition = *uninitialized*, $required_interface = *uninitialized*) /Volumes/devdisk/dev/sites/drupal8alt.dev/core/lib/Drupal/Core/Plugin/Factory/ContainerFactory.php:21
PHP  15. class_exists(*uninitialized*) /Volumes/devdisk/dev/sites/drupal8alt.dev/core/lib/Drupal/Component/Plugin/Factory/DefaultFactory.php:86
PHP  16. spl_autoload_call(*uninitialized*) /Volumes/devdisk/dev/sites/drupal8alt.dev/core/lib/Drupal/Component/Plugin/Factory/DefaultFactory.php:86
PHP  17. Composer\Autoload\ClassLoader->loadClass($class = *uninitialized*) /Volumes/devdisk/dev/sites/drupal8alt.dev/core/lib/Drupal/Component/Plugin/Factory/DefaultFactory.php:0
PHP  18. Composer\Autoload\includeFile($file = *uninitialized*) /Volumes/devdisk/dev/drush5/vendor/composer/ClassLoader.php:269

You don't even need to use the plugin manager to cause this :)

var_dump(class_exists('Drupal\views\Plugin\views\field\Xss'));
var_dump(class_exists('Drupal\views\Plugin\views\field\Field'));

will produce the same problems... oh PHP.

https://bugs.php.net/bug.php?id=62042 seems related

... So how did we managed to fix a critical bug without adding a test?

@dawehner, definition of "hotfix". It was late at night and every patch was failing, without HEAD having failed.

https://bugs.php.net/bug.php?id=62042 sounds like exactly the thing.

This is super confusing ... this particular problem exists in core since kinda begin of december afaik.

@dawehner, yeah, thence hotfixing it... as best we can tell it's somehow a side effect of having different testbots.

I started filing a followup issue, but based on this:

var_dump(class_exists('Drupal\views\Plugin\views\field\Xss'));
var_dump(class_exists('Drupal\views\Plugin\views\field\Field'));

This definitely seems like a PHP bug, and not something we could or should anticipate. Code can't know every class that might end up in the same namespace and plan its aliases accordingly, and there's no way of rewriting that test that would avoid the problem. We could rewrite the test itself to not do the reflection check before creating the plugins, to avoid such a failure in the future. We could change our naming and aliasing standards so we don't have such a high risk of name collisions. Or we could do nothing.

Thoughts?

FWIW, this naming policy (which Views violates) would have prevented us from running into the PHP bug:
https://www.drupal.org/node/608152#naming
As would (I think) using things with sub-namespaces, i.e.
use Drupal\Component\Utility\Xss as Utility\Xss;

Another issue like this today: #2446259: [HEAD BROKEN] Rename \Drupal\Core\Render\Element\Html to avoid namespace clash with \Drupal\Component\Utility\Html in render elements.