Browse Source

Fix UniversalClassLoader matching collisions.

The current `loadClass()` implementation tries to load a class from the first matching prefix then stops, producing false-negative results. This is especially evident in groups of related libraries, such as Doctrine:

    Doctrine
    Doctrine\Common
    Doctrine\Common\DataFixtures
    Doctrine\DBAL
    Doctrine\DBAL\Migrations

Each of these libraries is submoduled into a different vendor directory. Depending on what order these libraries are added to a UniversalClassLoader instance, classes may or may not actually be loaded. This fix continues searching registered namespaces and prefixes if the first partial match is negative.
Justin Hileman 14 years ago
parent
commit
cfd4e2186f

+ 4 - 6
src/Symfony/Component/HttpFoundation/UniversalClassLoader.php

@@ -132,13 +132,12 @@ class UniversalClassLoader
             $namespace = substr($class, 0, $pos);
             foreach ($this->namespaces as $ns => $dir) {
                 if (0 === strpos($namespace, $ns)) {
-                    $class = substr($class, $pos + 1);
-                    $file = $dir.DIRECTORY_SEPARATOR.str_replace('\\', DIRECTORY_SEPARATOR, $namespace).DIRECTORY_SEPARATOR.str_replace('_', DIRECTORY_SEPARATOR, $class).'.php';
+                    $className = substr($class, $pos + 1);
+                    $file = $dir.DIRECTORY_SEPARATOR.str_replace('\\', DIRECTORY_SEPARATOR, $namespace).DIRECTORY_SEPARATOR.str_replace('_', DIRECTORY_SEPARATOR, $className).'.php';
                     if (file_exists($file)) {
                         require $file;
+                        return;
                     }
-
-                    return;
                 }
             }
         } else {
@@ -148,9 +147,8 @@ class UniversalClassLoader
                     $file = $dir.DIRECTORY_SEPARATOR.str_replace('_', DIRECTORY_SEPARATOR, $class).'.php';
                     if (file_exists($file)) {
                         require $file;
+                        return;
                     }
-
-                    return;
                 }
             }
         }

+ 8 - 0
tests/Symfony/Tests/Component/HttpFoundation/Fixtures/alpha/NamespaceCollision/A/Bar.php

@@ -0,0 +1,8 @@
+<?php
+
+namespace NamespaceCollision\A;
+
+class Bar
+{
+    public static $loaded = true;
+}

+ 8 - 0
tests/Symfony/Tests/Component/HttpFoundation/Fixtures/alpha/NamespaceCollision/A/Foo.php

@@ -0,0 +1,8 @@
+<?php
+
+namespace NamespaceCollision\A;
+
+class Foo
+{
+    public static $loaded = true;
+}

+ 6 - 0
tests/Symfony/Tests/Component/HttpFoundation/Fixtures/alpha/PrefixCollision/A/Bar.php

@@ -0,0 +1,6 @@
+<?php
+
+class PrefixCollision_A_Bar
+{
+    public static $loaded = true;
+}

+ 6 - 0
tests/Symfony/Tests/Component/HttpFoundation/Fixtures/alpha/PrefixCollision/A/Foo.php

@@ -0,0 +1,6 @@
+<?php
+
+class PrefixCollision_A_Foo
+{
+    public static $loaded = true;
+}

+ 8 - 0
tests/Symfony/Tests/Component/HttpFoundation/Fixtures/beta/NamespaceCollision/A/B/Bar.php

@@ -0,0 +1,8 @@
+<?php
+
+namespace NamespaceCollision\A\B;
+
+class Bar
+{
+    public static $loaded = true;
+}

+ 8 - 0
tests/Symfony/Tests/Component/HttpFoundation/Fixtures/beta/NamespaceCollision/A/B/Foo.php

@@ -0,0 +1,8 @@
+<?php
+
+namespace NamespaceCollision\A\B;
+
+class Foo
+{
+    public static $loaded = true;
+}

+ 6 - 0
tests/Symfony/Tests/Component/HttpFoundation/Fixtures/beta/PrefixCollision/A/B/Bar.php

@@ -0,0 +1,6 @@
+<?php
+
+class PrefixCollision_A_B_Bar
+{
+    public static $loaded = true;
+}

+ 6 - 0
tests/Symfony/Tests/Component/HttpFoundation/Fixtures/beta/PrefixCollision/A/B/Foo.php

@@ -0,0 +1,6 @@
+<?php
+
+class PrefixCollision_A_B_Foo
+{
+    public static $loaded = true;
+}

+ 100 - 1
tests/Symfony/Tests/Component/HttpFoundation/UniversalClassLoaderTest.php

@@ -37,5 +37,104 @@ class UniversalClassLoaderTest extends \PHPUnit_Framework_TestCase
             array('\\Pearlike_Bar',    '\\Pearlike_Bar',    '->loadClass() loads Pearlike_Bar class with a leading slash'),
         );
     }
-}
 
+    /**
+     * @dataProvider namespaceCollisionClassProvider
+     */
+    public function testLoadClassNamespaceCollision($namespaces, $className, $message)
+    {
+        $loader = new UniversalClassLoader();
+        $loader->registerNamespaces($namespaces);
+
+        $loader->loadClass($className);
+        $this->assertTrue(class_exists($className), $message);
+    }
+
+    public static function namespaceCollisionClassProvider()
+    {
+        return array(
+            array(
+                array(
+                    'NamespaceCollision\\A' => __DIR__ . DIRECTORY_SEPARATOR . 'Fixtures/alpha',
+                    'NamespaceCollision\\A\\B' => __DIR__ . DIRECTORY_SEPARATOR . 'Fixtures/beta',
+                ),
+                'NamespaceCollision\A\Foo',
+                '->loadClass() loads NamespaceCollision\A\Foo from alpha.',
+            ),
+            array(
+                array(
+                    'NamespaceCollision\\A\\B' => __DIR__ . DIRECTORY_SEPARATOR . 'Fixtures/beta',
+                    'NamespaceCollision\\A' => __DIR__ . DIRECTORY_SEPARATOR . 'Fixtures/alpha',
+                ),
+                'NamespaceCollision\A\Bar',
+                '->loadClass() loads NamespaceCollision\A\Bar from alpha.',
+            ),
+            array(
+                array(
+                    'NamespaceCollision\\A' => __DIR__ . DIRECTORY_SEPARATOR . 'Fixtures/alpha',
+                    'NamespaceCollision\\A\\B' => __DIR__ . DIRECTORY_SEPARATOR . 'Fixtures/beta',
+                ),
+                'NamespaceCollision\A\B\Foo',
+                '->loadClass() loads NamespaceCollision\A\B\Foo from beta.',
+            ),
+            array(
+                array(
+                    'NamespaceCollision\\A\\B' => __DIR__ . DIRECTORY_SEPARATOR . 'Fixtures/beta',
+                    'NamespaceCollision\\A' => __DIR__ . DIRECTORY_SEPARATOR . 'Fixtures/alpha',
+                ),
+                'NamespaceCollision\A\B\Bar',
+                '->loadClass() loads NamespaceCollision\A\B\Bar from beta.',
+            ),
+        );
+    }
+
+    /**
+     * @dataProvider prefixCollisionClassProvider
+     */
+    public function testLoadClassPrefixCollision($prefixes, $className, $message)
+    {
+        $loader = new UniversalClassLoader();
+        $loader->registerPrefixes($prefixes);
+
+        $loader->loadClass($className);
+        $this->assertTrue(class_exists($className), $message);
+    }
+
+    public static function prefixCollisionClassProvider()
+    {
+        return array(
+            array(
+                array(
+                    'PrefixCollision_A_' => __DIR__ . DIRECTORY_SEPARATOR . 'Fixtures/alpha',
+                    'PrefixCollision_A_B_' => __DIR__ . DIRECTORY_SEPARATOR . 'Fixtures/beta',
+                ),
+                'PrefixCollision_A_Foo',
+                '->loadClass() loads PrefixCollision_A_Foo from alpha.',
+            ),
+            array(
+                array(
+                    'PrefixCollision_A_B_' => __DIR__ . DIRECTORY_SEPARATOR . 'Fixtures/beta',
+                    'PrefixCollision_A_' => __DIR__ . DIRECTORY_SEPARATOR . 'Fixtures/alpha',
+                ),
+                'PrefixCollision_A_Bar',
+                '->loadClass() loads PrefixCollision_A_Bar from alpha.',
+            ),
+            array(
+                array(
+                    'PrefixCollision_A_' => __DIR__ . DIRECTORY_SEPARATOR . 'Fixtures/alpha',
+                    'PrefixCollision_A_B_' => __DIR__ . DIRECTORY_SEPARATOR . 'Fixtures/beta',
+                ),
+                'PrefixCollision_A_B_Foo',
+                '->loadClass() loads PrefixCollision_A_B_Foo from beta.',
+            ),
+            array(
+                array(
+                    'PrefixCollision_A_B_' => __DIR__ . DIRECTORY_SEPARATOR . 'Fixtures/beta',
+                    'PrefixCollision_A_' => __DIR__ . DIRECTORY_SEPARATOR . 'Fixtures/alpha',
+                ),
+                'PrefixCollision_A_B_Bar',
+                '->loadClass() loads PrefixCollision_A_B_Bar from beta.',
+            ),
+        );
+    }
+}