From 66391ab57ad49c2a40bad59fc1fc9e1f12e39d97 Mon Sep 17 00:00:00 2001 From: Daniel Hensby Date: Fri, 13 Feb 2015 14:34:41 +0000 Subject: [PATCH] FIX Imported namespaces now correctly used to determine class inheritance Fixes #3707 --- core/manifest/ClassManifest.php | 225 +++++++++++++++--- .../manifest/NamespacedClassManifestTest.php | 154 +++++++++++- .../module/classes/ClassI.php | 12 + 3 files changed, 358 insertions(+), 33 deletions(-) create mode 100644 tests/core/manifest/fixtures/namespaced_classmanifest/module/classes/ClassI.php diff --git a/core/manifest/ClassManifest.php b/core/manifest/ClassManifest.php index 631415615..627f805cd 100644 --- a/core/manifest/ClassManifest.php +++ b/core/manifest/ClassManifest.php @@ -106,6 +106,33 @@ class SS_ClassManifest { )); } + /** + * Create a {@link TokenisedRegularExpression} that extracts the namespaces imported with the 'use' keyword + * + * This searches symbols for a `use` followed by 1 or more namespaces which are optionally aliased using the `as` + * keyword. The relevant matching tokens are added one-by-one into an array (using `save_to` param). + * + * eg: use Namespace\ClassName as Alias, OtherNamespace\ClassName; + * + * @return TokenisedRegularExpression + */ + public static function get_imported_namespace_parser() { + return new TokenisedRegularExpression(array( + 0 => T_USE, + 1 => T_WHITESPACE, + 2 => array(T_NS_SEPARATOR, 'save_to' => 'importString[]', 'optional' => true), + 3 => array(T_STRING, 'save_to' => 'importString[]', 'can_jump_to' => array(2, 8)), + 4 => array(T_WHITESPACE, 'save_to' => 'importString[]'), + 5 => array(T_AS, 'save_to' => 'importString[]'), + 6 => array(T_WHITESPACE, 'save_to' => 'importString[]'), + 7 => array(T_STRING, 'save_to' => 'importString[]'), + 8 => array(T_WHITESPACE, 'optional' => true), + 9 => array(',', 'save_to' => 'importString[]', 'optional' => true, 'can_jump_to' => 2), + 10 => array(T_WHITESPACE, 'optional' => true, 'can_jump_to' => 2), + 11 => ';', + )); + } + /** * Constructs and initialises a new class manifest, either loading the data * from the cache or re-scanning for classes. @@ -333,6 +360,124 @@ class SS_ClassManifest { } } + /** + * Find a the full namespaced declaration of a class (or interface) from a list of candidate imports + * + * This is typically used to determine the full class name in classes that have imported namesapced symbols (having + * used the `use` keyword) + * + * NB: remember the '\\' is an escaped backslash and is interpreted as a single \ + * + * @param string $class The class (or interface) name to find in the candidate imports + * @param string $namespace The namespace that was declared for the classes definition (if there was one) + * @param array $imports The list of imported symbols (Classes or Interfaces) to test against + * + * @return string The fully namespaced class name + */ + protected function findClassOrInterfaceFromCandidateImports($class, $namespace = '', $imports = array()) { + + //normalise the namespace + $namespace = rtrim($namespace, '\\'); + + //by default we'll use the $class as our candidate + $candidateClass = $class; + + if (!$class) { + return $candidateClass; + } + //if the class starts with a \ then it is explicitly in the global namespace and we don't need to do + // anything else + if (substr($class, 0, 1) == '\\') { + $candidateClass = substr($class, 1); + return $candidateClass; + } + //if there's a namespace, starting assumption is the class is defined in that namespace + if ($namespace) { + $candidateClass = $namespace . '\\' . $class; + } + + if (empty($imports)) { + return $candidateClass; + } + + //normalised class name (PHP is case insensitive for symbols/namespaces + $lClass = strtolower($class); + + //go through all the imports and see if the class exists within one of them + foreach ($imports as $alias => $import) { + //normalise import + $import = trim($import, '\\'); + + //if there is no string key, then there was no declared alias - we'll use the main declaration + if (is_int($alias)) { + $alias = strtolower($import); + } else { + $alias = strtolower($alias); + } + + //exact match? Then it's a class in the global namespace that was imported OR it's an alias of + // another namespace + // or if it ends with the \ClassName then it's the class we are looking for + if ($lClass == $alias + || substr_compare( + $alias, + '\\' . $lClass, + strlen($alias) - strlen($lClass) - 1, + // -1 because the $lClass length is 1 longer due to \ + strlen($alias) + ) === 0 + ) { + $candidateClass = $import; + break; + } + } + return $candidateClass; + } + + /** + * Return an array of array($alias => $import) from tokenizer's tokens of a PHP file + * + * NB: If there is no alias we don't set a key to the array + * + * @param array $tokens The parsed tokens from tokenizer's parsing of a PHP file + * + * @return array The array of imports as (optional) $alias => $import + */ + protected function getImportsFromTokens($tokens) { + //parse out the imports + $imports = self::get_imported_namespace_parser()->findAll($tokens); + + //if there are any imports, clean them up + // imports come to us as array('importString' => array([array of matching tokens])) + // we need to join this nested array into a string and split out the alias and the import + if (!empty($imports)) { + $cleanImports = array(); + foreach ($imports as $import) { + if (!empty($import['importString'])) { + //join the array up into a string + $importString = implode('', $import['importString']); + //split at , to get each import declaration + $importSet = explode(',', $importString); + foreach ($importSet as $importDeclaration) { + //split at ' as ' (any case) to see if we are aliasing the namespace + $importDeclaration = preg_split('/\s+as\s+/i', $importDeclaration); + //shift off the fully namespaced import + $qualifiedImport = array_shift($importDeclaration); + //if there are still items in the array, it's the alias + if (!empty($importDeclaration)) { + $cleanImports[array_shift($importDeclaration)] = $qualifiedImport; + } + else { + $cleanImports[] = $qualifiedImport; + } + } + } + } + $imports = $cleanImports; + } + return $imports; + } + public function handleFile($basename, $pathname, $depth) { if ($basename == self::CONF_FILE) { $this->configs[] = $pathname; @@ -342,6 +487,7 @@ class SS_ClassManifest { $classes = null; $interfaces = null; $namespace = null; + $imports = null; // The results of individual file parses are cached, since only a few // files will have changed and TokenisedRegularExpression is quite @@ -352,14 +498,17 @@ class SS_ClassManifest { if ($data = $this->cache->load($key)) { $valid = ( - isset($data['classes']) && isset($data['interfaces']) && isset($data['namespace']) - && is_array($data['classes']) && is_array($data['interfaces']) && is_string($data['namespace']) + isset($data['classes']) && is_array($data['classes']) + && isset($data['interfaces']) && is_array($data['interfaces']) + && isset($data['namespace']) && is_string($data['namespace']) + && isset($data['imports']) && is_array($data['imports']) ); if ($valid) { - $classes = $data['classes']; + $classes = $data['classes']; $interfaces = $data['interfaces']; $namespace = $data['namespace']; + $imports = $data['imports']; } } @@ -367,28 +516,52 @@ class SS_ClassManifest { $tokens = token_get_all($file); $classes = self::get_namespaced_class_parser()->findAll($tokens); + $namespace = self::get_namespace_parser()->findAll($tokens); + if($namespace) { - $namespace = implode('', $namespace[0]['namespaceName']) . '\\'; + $namespace = implode('', $namespace[0]['namespaceName']); } else { $namespace = ''; } + $imports = $this->getImportsFromTokens($tokens); + $interfaces = self::get_interface_parser()->findAll($tokens); - $cache = array('classes' => $classes, 'interfaces' => $interfaces, 'namespace' => $namespace); + $cache = array( + 'classes' => $classes, + 'interfaces' => $interfaces, + 'namespace' => $namespace, + 'imports' => $imports + ); $this->cache->save($cache, $key); } foreach ($classes as $class) { - $name = $namespace . $class['className']; - $extends = isset($class['extends']) ? implode('', $class['extends']) : null; + $name = $class['className']; + if ($namespace) { + $namespace = rtrim($namespace, '\\'); + $name = $namespace . '\\' . $name; + } + $extends = isset($class['extends']) ? implode('', $class['extends']) : null; $implements = isset($class['interfaces']) ? $class['interfaces'] : null; - if($extends && $extends[0] != '\\') { - $extends = $namespace . $extends; - } elseif($extends) { - $extends = substr($extends, 1); + if ($extends) { + $extends = $this->findClassOrInterfaceFromCandidateImports($extends, $namespace, $imports); + } + + if (!empty($implements)) { + //join all the tokens + $implements = implode('', $implements); + //split at comma + $implements = explode(',', $implements); + //normalise interfaces + foreach ($implements as &$interface) { + $interface = $this->findClassOrInterfaceFromCandidateImports($interface, $namespace, $imports); + } + //release the var name + unset($interface); } $lowercaseName = strtolower($name); @@ -414,32 +587,24 @@ class SS_ClassManifest { } if ($implements) { - $interface = $namespace; - for($i = 0; $i < count($implements); ++$i) { - if($implements[$i] == ',') { - $interface = $namespace; - continue; - } - if($implements[$i] == '\\' && $interface == $namespace) { - $interface = ''; - } else { - $interface .= $implements[$i]; - } - if($i == count($implements)-1 || $implements[$i+1] == ',') { - $interface = strtolower($interface); + foreach ($implements as $interface) { + $interface = strtolower($interface); - if (!isset($this->implementors[$interface])) { - $this->implementors[$interface] = array($name); - } else { - $this->implementors[$interface][] = $name; - } + if (!isset($this->implementors[$interface])) { + $this->implementors[$interface] = array($name); + } else { + $this->implementors[$interface][] = $name; } } } } + $interfaceBase = ''; + if ($namespace) { + $interfaceBase = $namespace . '\\'; + } foreach ($interfaces as $interface) { - $this->interfaces[strtolower($namespace . $interface['interfaceName'])] = $pathname; + $this->interfaces[strtolower($interfaceBase . $interface['interfaceName'])] = $pathname; } } diff --git a/tests/core/manifest/NamespacedClassManifestTest.php b/tests/core/manifest/NamespacedClassManifestTest.php index c87dea4e4..68e662f08 100644 --- a/tests/core/manifest/NamespacedClassManifestTest.php +++ b/tests/core/manifest/NamespacedClassManifestTest.php @@ -15,6 +15,152 @@ class NamespacedClassManifestTest extends SapphireTest { $this->base = dirname(__FILE__) . '/fixtures/namespaced_classmanifest'; $this->manifest = new SS_ClassManifest($this->base, false, true, false); + SS_ClassLoader::instance()->pushManifest($this->manifest, false); + } + + public function tearDown() { + parent::tearDown(); + SS_ClassLoader::instance()->popManifest(); + } + + public function testGetImportedNamespaceParser() { + $file = file_get_contents($this->base . DIRECTORY_SEPARATOR . 'module/classes/ClassI.php'); + $tokens = token_get_all($file); + $parsedTokens = SS_ClassManifest::get_imported_namespace_parser()->findAll($tokens); + + $expectedItems = array( + array('ModelAdmin'), + array('Controller', ' ', 'as', ' ', 'Cont'), + array( + 'SS_HTTPRequest', ' ', 'as', ' ', 'Request', ',', + 'SS_HTTPResponse', ' ', 'AS', ' ', 'Response', ',', + 'PermissionProvider', ' ', 'AS', ' ', 'P', + ), + array('silverstripe', '\\', 'test', '\\', 'ClassA'), + array('\\', 'DataObject'), + ); + + $this->assertEquals(count($expectedItems), count($parsedTokens)); + + foreach ($expectedItems as $i => $item) { + $this->assertEquals($item, $parsedTokens[$i]['importString']); + } + } + + public function testGetImportsFromTokens() { + $file = file_get_contents($this->base . DIRECTORY_SEPARATOR . 'module/classes/ClassI.php'); + $tokens = token_get_all($file); + + $method = new ReflectionMethod($this->manifest, 'getImportsFromTokens'); + $method->setAccessible(true); + + $expectedImports = array( + 'ModelAdmin', + 'Cont' => 'Controller', + 'Request' => 'SS_HTTPRequest', + 'Response' => 'SS_HTTPResponse', + 'P' => 'PermissionProvider', + 'silverstripe\test\ClassA', + '\DataObject', + ); + + $imports = $method->invoke($this->manifest, $tokens); + + $this->assertEquals($expectedImports, $imports); + + } + + public function testClassInfoIsCorrect() { + $this->assertContains('SilverStripe\Framework\Tests\ClassI', ClassInfo::implementorsOf('PermissionProvider')); + + //because we're using a nested manifest we have to "coalesce" the descendants again to correctly populate the + // descendants of the core classes we want to test against - this is a limitation of the test manifest not + // including all core classes + $method = new ReflectionMethod($this->manifest, 'coalesceDescendants'); + $method->setAccessible(true); + $method->invoke($this->manifest, 'ModelAdmin'); + + $this->assertContains('SilverStripe\Framework\Tests\ClassI', ClassInfo::subclassesFor('ModelAdmin')); + } + + public function testFindClassOrInterfaceFromCandidateImports() { + $method = new ReflectionMethod($this->manifest, 'findClassOrInterfaceFromCandidateImports'); + $method->setAccessible(true); + + $this->assertTrue(ClassInfo::exists('silverstripe\test\ClassA')); + + $this->assertEquals('PermissionProvider', $method->invokeArgs($this->manifest, array( + '\PermissionProvider', + 'Test\Namespace', + array( + 'TestOnly', + 'Controller', + ), + ))); + + $this->assertEquals('PermissionProvider', $method->invokeArgs($this->manifest, array( + 'PermissionProvider', + 'Test\NAmespace', + array( + 'PermissionProvider', + ) + ))); + + $this->assertEmpty($method->invokeArgs($this->manifest, array( + '', + 'TextNamespace', + array( + 'PermissionProvider', + ), + ))); + + $this->assertEmpty($method->invokeArgs($this->manifest, array( + '', + '', + array() + ))); + + $this->assertEquals('silverstripe\test\ClassA', $method->invokeArgs($this->manifest, array( + 'ClassA', + 'Test\Namespace', + array( + 'silverstripe\test\ClassA', + 'PermissionProvider', + ), + ))); + + $this->assertEquals('ClassA', $method->invokeArgs($this->manifest, array( + '\ClassA', + 'Test\Namespace', + array( + 'silverstripe\test', + ), + ))); + + $this->assertEquals('ClassA', $method->invokeArgs($this->manifest, array( + 'ClassA', + 'silverstripe\test', + array( + '\ClassA', + ), + ))); + + $this->assertEquals('ClassA', $method->invokeArgs($this->manifest, array( + 'Alias', + 'silverstripe\test', + array( + 'Alias' => '\ClassA', + ), + ))); + + $this->assertEquals('silverstripe\test\ClassA', $method->invokeArgs($this->manifest, array( + 'ClassA', + 'silverstripe\test', + array( + 'silverstripe\test\ClassB', + ), + ))); + } public function testGetItemPath() { @@ -43,7 +189,8 @@ class NamespacedClassManifestTest extends SapphireTest { 'silverstripe\test\classg' => "{$this->base}/module/classes/ClassG.php", 'silverstripe\test\classh' => "{$this->base}/module/classes/ClassH.php", 'sstemplateparser' => FRAMEWORK_PATH."/view/SSTemplateParser.php", - 'sstemplateparseexception' => FRAMEWORK_PATH."/view/SSTemplateParser.php" + 'sstemplateparseexception' => FRAMEWORK_PATH."/view/SSTemplateParser.php", + 'silverstripe\framework\tests\classi' => "{$this->base}/module/classes/ClassI.php", ); $this->assertEquals($expect, $this->manifest->getClasses()); @@ -54,7 +201,7 @@ class NamespacedClassManifestTest extends SapphireTest { array('sstemplateparser', 'sstemplateparseexception', 'silverstripe\test\classa', 'silverstripe\test\classb', 'silverstripe\test\classc', 'silverstripe\test\classd', 'silverstripe\test\classe', 'silverstripe\test\classf', 'silverstripe\test\classg', - 'silverstripe\test\classh'), + 'silverstripe\test\classh', 'silverstripe\framework\tests\classi'), $this->manifest->getClassNames()); } @@ -88,7 +235,8 @@ class NamespacedClassManifestTest extends SapphireTest { $expect = array( 'silverstripe\test\interfacea' => array('silverstripe\test\ClassE'), 'interfacea' => array('silverstripe\test\ClassF'), - 'silverstripe\test\subtest\interfacea' => array('silverstripe\test\ClassG') + 'silverstripe\test\subtest\interfacea' => array('silverstripe\test\ClassG'), + 'permissionprovider' => array('SilverStripe\Framework\Tests\ClassI'), ); $this->assertEquals($expect, $this->manifest->getImplementors()); } diff --git a/tests/core/manifest/fixtures/namespaced_classmanifest/module/classes/ClassI.php b/tests/core/manifest/fixtures/namespaced_classmanifest/module/classes/ClassI.php new file mode 100644 index 000000000..252aa3ef7 --- /dev/null +++ b/tests/core/manifest/fixtures/namespaced_classmanifest/module/classes/ClassI.php @@ -0,0 +1,12 @@ +