[BUGFIX] Harden fallback class map generation 03/45603/2
authorHelmut Hummel <info@helhum.io>
Wed, 6 Jan 2016 14:28:49 +0000 (15:28 +0100)
committerWouter Wolters <typo3@wouterwolters.nl>
Wed, 13 Jan 2016 09:49:08 +0000 (10:49 +0100)
It could happen when an extension does not provide any
autoload definition, that valid classes are not or
invalid classes are taken into account during class map generation.

To fix this, update to latest composer code that supports
a blacklist for class map generation and provide tests
folder and ext_update files as valid black list.

Also add "Resources" and "res" folder to black list, which
very often also contain php classes, which are then manually required.

Also fix, streamline and add tests to verify this behavior.

Resolves: #72557
Releases: 7.6, master
Change-Id: I0b949fea8b23edbf9c8b92a4ff87218a66bd0918
Reviewed-on: https://review.typo3.org/45603
Reviewed-by: Wouter Wolters <typo3@wouterwolters.nl>
Tested-by: Wouter Wolters <typo3@wouterwolters.nl>
Reviewed-by: Benni Mack <benni@typo3.org>
Tested-by: Benni Mack <benni@typo3.org>
typo3/sysext/core/Classes/Core/ClassLoadingInformationGenerator.php
typo3/sysext/core/Resources/PHP/ClassMapGenerator.php
typo3/sysext/core/Tests/Unit/Core/ClassLoadingInformationGeneratorTest.php
typo3/sysext/core/Tests/Unit/Core/Fixtures/test_extension/Tests/TestClass.php [new file with mode: 0644]
typo3/sysext/core/Tests/Unit/Core/Fixtures/test_extension/class.ext_update.php [new file with mode: 0644]

index 4a53ac3..49261b7 100644 (file)
@@ -145,15 +145,12 @@ class ClassLoadingInformationGenerator
     protected function createClassMap($classesPath, $useRelativePaths = false, $ignorePotentialTestClasses = false, $namespace = null)
     {
         $classMap = array();
-        foreach (ClassMapGenerator::createMap($classesPath, null, null, $namespace) as $class => $path) {
-            if ($ignorePotentialTestClasses) {
-                if ($this->isIgnoredPath($classesPath, $path)) {
-                    continue;
-                }
-                if ($this->isIgnoredClassName($class)) {
-                    continue;
-                }
-            }
+        $blacklistExpression = null;
+        if ($ignorePotentialTestClasses) {
+            $blacklistPathPrefix = realpath($classesPath);
+            $blacklistExpression = "{($blacklistPathPrefix/tests/|$blacklistPathPrefix/Tests/|$blacklistPathPrefix/Resources/|$blacklistPathPrefix/res/|$blacklistPathPrefix/class.ext_update.php)}";
+        }
+        foreach (ClassMapGenerator::createMap($classesPath, $blacklistExpression, null, $namespace) as $class => $path) {
             if ($useRelativePaths) {
                 $classMap[$class] = $this->makePathRelative($classesPath, $path);
             } else {
@@ -164,40 +161,6 @@ class ClassLoadingInformationGenerator
     }
 
     /**
-     * Check if the class path should be ignored.
-     * Currently only tests folders are ignored.
-     *
-     * @param string $packagePath
-     * @param string $path
-     * @return bool
-     */
-    protected function isIgnoredPath($packagePath, $path)
-    {
-        if (stripos($this->makePathRelative($packagePath, $path, false), 'tests') !== false) {
-            return true;
-        }
-
-        return false;
-    }
-
-    /**
-     * Check if class name should be ignored.
-     * Currently all classes with suffix "Test" and "Fixture" will be ignored
-     *
-     * @param string $className
-     * @return bool
-     */
-    protected function isIgnoredClassName($className)
-    {
-        foreach (array('Test', 'Fixture') as $suffix) {
-            if (preg_match('/(^|[a-z])' . $suffix . '$/', $className)) {
-                return true;
-            }
-        }
-        return false;
-    }
-
-    /**
      * Returns class alias map for given package
      *
      * @param PackageInterface $package The package to generate the class alias info for
index 656aabd..b487ed6 100644 (file)
@@ -1,23 +1,31 @@
 <?php
 
 /*
- * This file is copied from the Symfony package.
+ * This file is part of Composer.
  *
- * (c) Fabien Potencier <fabien@symfony.com>
+ * (c) Nils Adermann <naderman@naderman.de>
+ *     Jordi Boggiano <j.boggiano@seld.be>
  *
  * For the full copyright and license information, please view the LICENSE
  * file that was distributed with this source code.
+ */
+
+/*
+ * This file is copied from the Symfony package.
  *
- * @license MIT
+ * (c) Fabien Potencier <fabien@symfony.com>
  */
 
 namespace Composer\Autoload;
 
-use Composer\IO\IOInterface;
 use Symfony\Component\Finder\Finder;
+use Composer\IO\IOInterface;
 
 /**
  * ClassMapGenerator
+ *
+ * @author Gyula Sallai <salla016@gmail.com>
+ * @author Jordi Boggiano <j.boggiano@seld.be>
  */
 class ClassMapGenerator
 {
@@ -42,15 +50,14 @@ class ClassMapGenerator
      * Iterate over all files in the given directory searching for classes
      *
      * @param \Iterator|string $path      The path to search in or an iterator
-     * @param string           $whitelist Regex that matches against the file path
+     * @param string           $blacklist Regex that matches against the file path that exclude from the classmap.
      * @param IOInterface      $io        IO object
      * @param string           $namespace Optional namespace prefix to filter by
      *
-     * @return array A class map array
-     *
      * @throws \RuntimeException When the path is neither an existing file nor directory
+     * @return array             A class map array
      */
-    public static function createMap($path, $whitelist = null, IOInterface $io = null, $namespace = null)
+    public static function createMap($path, $blacklist = null, IOInterface $io = null, $namespace = null)
     {
         if (is_string($path)) {
             if (is_file($path)) {
@@ -59,7 +66,7 @@ class ClassMapGenerator
                 $path = Finder::create()->files()->followLinks()->name('/\.(php|inc|hh)$/')->in($path);
             } else {
                 throw new \RuntimeException(
-                    'Could not scan for classes inside "' . $path .
+                    'Could not scan for classes inside "'.$path.
                     '" which does not appear to be a file nor a folder'
                 );
             }
@@ -74,7 +81,7 @@ class ClassMapGenerator
                 continue;
             }
 
-            if ($whitelist && !preg_match($whitelist, strtr($filePath, '\\', '/'))) {
+            if ($blacklist && preg_match($blacklist, strtr($filePath, '\\', '/'))) {
                 continue;
             }
 
@@ -88,10 +95,10 @@ class ClassMapGenerator
 
                 if (!isset($map[$class])) {
                     $map[$class] = $filePath;
-                } elseif ($io && $map[$class] !== $filePath && !preg_match('{/(test|fixture|example)s?/}i', strtr($map[$class] . ' ' . $filePath, '\\', '/'))) {
+                } elseif ($io && $map[$class] !== $filePath && !preg_match('{/(test|fixture|example|stub)s?/}i', strtr($map[$class].' '.$filePath, '\\', '/'))) {
                     $io->writeError(
-                        '<warning>Warning: Ambiguous class resolution, "' . $class . '"' .
-                        ' was found in both "' . $map[$class] . '" and "' . $filePath . '", the first will be used.</warning>'
+                        '<warning>Warning: Ambiguous class resolution, "'.$class.'"'.
+                        ' was found in both "'.$map[$class].'" and "'.$filePath.'", the first will be used.</warning>'
                     );
                 }
             }
@@ -125,18 +132,18 @@ class ClassMapGenerator
                 }
             }
         } catch (\Exception $e) {
-            throw new \RuntimeException('Could not scan for classes inside ' . $path . ": \n" . $e->getMessage(), 0, $e);
+            throw new \RuntimeException('Could not scan for classes inside '.$path.": \n".$e->getMessage(), 0, $e);
         }
 
         // return early if there is no chance of matching anything in this file
-        if (!preg_match('{\b(?:class|interface' . $extraTypes . ')\s}i', $contents)) {
+        if (!preg_match('{\b(?:class|interface'.$extraTypes.')\s}i', $contents)) {
             return array();
         }
 
         // strip heredocs/nowdocs
         $contents = preg_replace('{<<<\s*(\'?)(\w+)\\1(?:\r\n|\n|\r)(?:.*?)(?:\r\n|\n|\r)\\2(?=\r\n|\n|\r|;)}s', 'null', $contents);
         // strip strings
-        $contents = preg_replace('{"[^"\\\\]*(\\\\.[^"\\\\]*)*"|\'[^\'\\\\]*(\\\\.[^\'\\\\]*)*\'}s', 'null', $contents);
+        $contents = preg_replace('{"[^"\\\\]*+(\\\\.[^"\\\\]*+)*+"|\'[^\'\\\\]*+(\\\\.[^\'\\\\]*+)*+\'}s', 'null', $contents);
         // strip leading non-php code if needed
         if (substr($contents, 0, 2) !== '<?') {
             $contents = preg_replace('{^.+?<\?}s', '<?', $contents, 1, $replacements);
@@ -154,8 +161,8 @@ class ClassMapGenerator
 
         preg_match_all('{
             (?:
-                 \b(?<![\$:>])(?P<type>class|interface' . $extraTypes . ') \s+ (?P<name>[a-zA-Z_\x7f-\xff:][a-zA-Z0-9_\x7f-\xff:\-]*)
-               | \b(?<![\$:>])(?P<ns>namespace) (?P<nsname>\s+[a-zA-Z_\x7f-\xff][a-zA-Z0-9_\x7f-\xff]*(?:\s*\\\\\s*[a-zA-Z_\x7f-\xff][a-zA-Z0-9_\x7f-\xff]*)*)? \s*[\{;]
+                 \b(?<![\$:>])(?P<type>class|interface'.$extraTypes.') \s++ (?P<name>[a-zA-Z_\x7f-\xff:][a-zA-Z0-9_\x7f-\xff:\-]*+)
+               | \b(?<![\$:>])(?P<ns>namespace) (?P<nsname>\s++[a-zA-Z_\x7f-\xff][a-zA-Z0-9_\x7f-\xff]*+(?:\s*+\\\\\s*+[a-zA-Z_\x7f-\xff][a-zA-Z0-9_\x7f-\xff]*+)*+)? \s*+ [\{;]
             )
         }ix', $contents, $matches);
 
@@ -169,7 +176,7 @@ class ClassMapGenerator
                 $name = $matches['name'][$i];
                 if ($name[0] === ':') {
                     // This is an XHP class, https://github.com/facebook/xhp
-                    $name = 'xhp' . substr(str_replace(array('-', ':'), array('_', '__'), $name), 1);
+                    $name = 'xhp'.substr(str_replace(array('-', ':'), array('_', '__'), $name), 1);
                 } elseif ($matches['type'][$i] === 'enum') {
                     // In Hack, something like:
                     //   enum Foo: int { HERP = '123'; }
index 6affca1..06459cd 100644 (file)
@@ -25,46 +25,6 @@ use TYPO3\CMS\Core\Tests\UnitTestCase;
 class ClassLoadingInformationGeneratorTest extends UnitTestCase
 {
     /**
-     * Data provider with different class names.
-     *
-     * @return array
-     */
-    public function isIgnoredClassNameIgnoresTestClassesDataProvider()
-    {
-        return array(
-            'FoTest' => array('FoTest', true),
-            'FoLowercasetest' => array('FoLowercasetest', false),
-            'DifferentClassTes' => array('DifferentClassTes', false),
-            'Test' => array('Test', true),
-            'FoFixture' => array('FoFixture', true),
-            'FoLowercasefixture' => array('FoLowercasefixture', false),
-            'DifferentClassFixtur' => array('DifferentClassFixtur', false),
-            'Fixture' => array('Fixture', true),
-            'Latest' => array('Latest', false),
-            'LaTest' => array('LaTest', true),
-            'Tx_RedirectTest_Domain_Model_Test' => array('Tx_RedirectTest_Domain_Model_Test', false),
-        );
-    }
-
-    /**
-     * @test
-     * @dataProvider isIgnoredClassNameIgnoresTestClassesDataProvider
-     *
-     * @param string $className
-     * @param bool $expectedResult
-     */
-    public function isIgnoredClassNameIgnoresTestClasses($className, $expectedResult)
-    {
-        $generator = $this->getAccessibleMock(
-            ClassLoadingInformationGenerator::class,
-            ['dummy'],
-            [$this->getMock(ClassLoader::class), array(), __DIR__]
-        );
-
-        $this->assertEquals($expectedResult, $generator->_call('isIgnoredClassName', $className));
-    }
-
-    /**
      * @test
      * @expectedException \TYPO3\CMS\Core\Error\Exception
      * @expectedExceptionCode 1444142481
@@ -248,6 +208,17 @@ class ClassLoadingInformationGeneratorTest extends UnitTestCase
                     '!$typo3InstallDir . \'/Fixtures/test_extension/Resources/PHP/Subdirectory/SubdirectoryTest.php\'',
                 ],
             ],
+            'No autoload section' => [
+                [],
+                [],
+                [
+                    '!$typo3InstallDir . \'/Fixtures/test_extension/Resources/PHP/Test.php\'',
+                    '!$typo3InstallDir . \'/Fixtures/test_extension/Tests/TestClass.php\'',
+                    '!$typo3InstallDir . \'/Fixtures/test_extension/Resources/PHP/AnotherTestFile.php\'',
+                    '!$typo3InstallDir . \'/Fixtures/test_extension/Resources/PHP/Subdirectory/SubdirectoryTest.php\'',
+                    '!$typo3InstallDir . \'/Fixtures/test_extension/class.ext_update.php\'',
+                ],
+            ],
             'Classmap section pointing to two files' => [
                 [
                     'autoload' => [
@@ -287,18 +258,24 @@ class ClassLoadingInformationGeneratorTest extends UnitTestCase
         foreach ($expectedPsr4Files as $expectation) {
             if ($expectation[0] === '!') {
                 $expectedCount = 0;
+                $expectation = substr($expectation, 1);
+                $message = sprintf('File "%s" is NOT expected to be in psr-4, but is.', $expectation);
             } else {
                 $expectedCount = 1;
+                $message = sprintf('File "%s" is expected to be in psr-4, but is not.', $expectation);
             }
-            $this->assertSame($expectedCount, substr_count($files['psr-4File'], $expectation));
+            $this->assertSame($expectedCount, substr_count($files['psr-4File'], $expectation), $message);
         }
         foreach ($expectedClassMapFiles as $expectation) {
             if ($expectation[0] === '!') {
                 $expectedCount = 0;
+                $expectation = substr($expectation, 1);
+                $message = sprintf('File "%s" is NOT expected to be in class map, but is.', $expectation);
             } else {
                 $expectedCount = 1;
+                $message = sprintf('File "%s" is expected to be in class map, but is not.', $expectation);
             }
-            $this->assertSame($expectedCount, substr_count($files['classMapFile'], $expectation));
+            $this->assertSame($expectedCount, substr_count($files['classMapFile'], $expectation), $message);
         }
     }
 
diff --git a/typo3/sysext/core/Tests/Unit/Core/Fixtures/test_extension/Tests/TestClass.php b/typo3/sysext/core/Tests/Unit/Core/Fixtures/test_extension/Tests/TestClass.php
new file mode 100644 (file)
index 0000000..b7d8096
--- /dev/null
@@ -0,0 +1,9 @@
+<?php
+
+/**
+ * Class TestClass
+ */
+class TestClass
+{
+
+}
\ No newline at end of file
diff --git a/typo3/sysext/core/Tests/Unit/Core/Fixtures/test_extension/class.ext_update.php b/typo3/sysext/core/Tests/Unit/Core/Fixtures/test_extension/class.ext_update.php
new file mode 100644 (file)
index 0000000..8d32f9a
--- /dev/null
@@ -0,0 +1,9 @@
+<?php
+
+/**
+ * Class ext_update
+ */
+class ext_update
+{
+
+}
\ No newline at end of file