From ba2940ada1ec1176b563715e417b6421a9f2c979 Mon Sep 17 00:00:00 2001 From: Andreas Hennings Date: Wed, 21 May 2014 06:38:46 +0200 Subject: [PATCH 1/5] Correct the folder name for CustomBlockLocalTasksTest. --- .../Tests/Menu/CustomBlockLocalTasksTest.php | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename core/modules/block/custom_block/tests/Drupal/{custom_blocks => custom_block}/Tests/Menu/CustomBlockLocalTasksTest.php (100%) diff --git a/core/modules/block/custom_block/tests/Drupal/custom_blocks/Tests/Menu/CustomBlockLocalTasksTest.php b/core/modules/block/custom_block/tests/Drupal/custom_block/Tests/Menu/CustomBlockLocalTasksTest.php similarity index 100% rename from core/modules/block/custom_block/tests/Drupal/custom_blocks/Tests/Menu/CustomBlockLocalTasksTest.php rename to core/modules/block/custom_block/tests/Drupal/custom_block/Tests/Menu/CustomBlockLocalTasksTest.php -- 1.8.5.1 From df501b4c1209fb1a015c2e7efbd9f14bdbd8cb76 Mon Sep 17 00:00:00 2001 From: Andreas Hennings Date: Fri, 23 May 2014 09:52:14 +0200 Subject: [PATCH 2/5] Replace FieldDefinitionTestBase::getNamespacePath() with FieldDefinitionTestBase::getModuleInfoFilePath() --- .../tests/src/Field/PathFieldDefinitionTest.php | 4 +-- .../Tests/Core/Field/FieldDefinitionTestBase.php | 36 +++++++++++++--------- 2 files changed, 24 insertions(+), 16 deletions(-) diff --git a/core/modules/path/tests/src/Field/PathFieldDefinitionTest.php b/core/modules/path/tests/src/Field/PathFieldDefinitionTest.php index fb99b82..ac3d1fe 100644 --- a/core/modules/path/tests/src/Field/PathFieldDefinitionTest.php +++ b/core/modules/path/tests/src/Field/PathFieldDefinitionTest.php @@ -41,8 +41,8 @@ protected function getPluginId() { /** * {@inheritdoc} */ - protected function getNamespacePath() { - return dirname(dirname(dirname(__DIR__))) . '/lib/Drupal/path'; + protected function getModuleInfoFilePath() { + return dirname(dirname(dirname(__DIR__))) . '/path.info.yml'; } /** diff --git a/core/tests/Drupal/Tests/Core/Field/FieldDefinitionTestBase.php b/core/tests/Drupal/Tests/Core/Field/FieldDefinitionTestBase.php index 49a3922..35d454d 100644 --- a/core/tests/Drupal/Tests/Core/Field/FieldDefinitionTestBase.php +++ b/core/tests/Drupal/Tests/Core/Field/FieldDefinitionTestBase.php @@ -28,15 +28,22 @@ * {@inheritdoc} */ public function setUp() { - $namespace_path = $this->getNamespacePath(); - // Suppport both PSR-0 and PSR-4 directory layouts. - $module_name = basename($namespace_path); - if ($module_name == 'src') { - $module_name = basename($module_name); + + $module_info_file = $this->getModuleInfoFilePath(); + if (!preg_match('#^(.*)/([^/]+)\.info\.yml$#', $module_info_file, $m)) { + throw new \Exception("Unexpected module info file."); } - $namespaces = new \ArrayObject(array( - 'Drupal\\' . $module_name => $namespace_path, - )); + + list(, $module_dir, $module_name) = $m; + + $namespaces = new \ArrayObject( + array( + "Drupal\\$module_name" => array( + $module_dir . '/src', + $module_dir . '/lib/Drupal/' . $module_name, + ), + ) + ); $language_manager = $this->getMock('Drupal\Core\Language\LanguageManagerInterface'); $language_manager->expects($this->once()) @@ -72,15 +79,16 @@ public function setUp() { abstract protected function getPluginId(); /** - * Returns the path to the module's classes. + * Returns the path to the *.info.yml file of the module where the test should + * look for plugins. * - * Depending on whether the module follows the PSR-0 or PSR-4 directory layout - * this should be either /path/to/module/lib/Drupal/mymodule or - * /path/to/module/src. + * This will be used to determine both the module name and the module + * directory. * * @return string - * The path to the module's classes. + * The path to the MODULE.info.yml file, e.g. + * DRUPAL_ROOT . "/core/modules/path/path.info.yml". */ - abstract protected function getNamespacePath(); + abstract protected function getModuleInfoFilePath(); } -- 1.8.5.1 From f005a680de5bd6ed014b476c2ad121c9aa0eb7a1 Mon Sep 17 00:00:00 2001 From: Andreas Hennings Date: Fri, 23 May 2014 09:58:11 +0200 Subject: [PATCH 3/5] OPTIONAL: Add default implementation for getModuleInfoFilePath(), so that subclasses don't need to implement it. --- .../tests/src/Field/PathFieldDefinitionTest.php | 7 -- .../Tests/Core/Field/FieldDefinitionTestBase.php | 91 +++++++++++++++++++++- 2 files changed, 90 insertions(+), 8 deletions(-) diff --git a/core/modules/path/tests/src/Field/PathFieldDefinitionTest.php b/core/modules/path/tests/src/Field/PathFieldDefinitionTest.php index ac3d1fe..42144d3 100644 --- a/core/modules/path/tests/src/Field/PathFieldDefinitionTest.php +++ b/core/modules/path/tests/src/Field/PathFieldDefinitionTest.php @@ -39,13 +39,6 @@ protected function getPluginId() { } /** - * {@inheritdoc} - */ - protected function getModuleInfoFilePath() { - return dirname(dirname(dirname(__DIR__))) . '/path.info.yml'; - } - - /** * Tests FieldDefinition::getColumns(). * * @covers \Drupal\Core\Field\FieldDefinition::getColumns diff --git a/core/tests/Drupal/Tests/Core/Field/FieldDefinitionTestBase.php b/core/tests/Drupal/Tests/Core/Field/FieldDefinitionTestBase.php index 35d454d..fcd6222 100644 --- a/core/tests/Drupal/Tests/Core/Field/FieldDefinitionTestBase.php +++ b/core/tests/Drupal/Tests/Core/Field/FieldDefinitionTestBase.php @@ -85,10 +85,99 @@ public function setUp() { * This will be used to determine both the module name and the module * directory. * + * This defaults to the module that contains the test, but can be overridden. + * * @return string * The path to the MODULE.info.yml file, e.g. * DRUPAL_ROOT . "/core/modules/path/path.info.yml". + * + * @throws \Exception + */ + protected function getModuleInfoFilePath() { + + $class = get_class($this); + $module_info_file = $this->classExtractModuleInfoFilePath($class); + if ($module_info_file === FALSE) { + throw new \Exception("No module info file found for class '$class'."); + } + return $module_info_file; + } + + /** + * Determines if a given class is part of a Drupal module, and determines the + * path to the *.info.yml file, if possible. + * + * @todo Put this in a globally accessible place. + * + * @param string $class + * The fully-qualified class name, e.g. + * "Drupal\\path\\Tests\\PathFieldDefinitionTest". + * + * @return string|false + * The path to the module info file, or FALSE if it could not be determined. + */ + private function classExtractModuleInfoFilePath($class) { + + if (!preg_match("/Drupal\\\\(.+)\\\\Tests\\\\(.+)$/", $class, $m)) { + return FALSE; + } + + list(, $module_name, $relative_class_name) = $m; + + $relative_path = str_replace('\\', '/', $relative_class_name) . '.php'; + + $suffixes = array( + // PSR-4 location for PHPUnit tests. + '/tests/src/' . $relative_path, + // PSR-0 location for PHPUnit tests. + // @todo Remove when PSR-0 support has ended. + "/tests/Drupal/$module_name/Tests/" . $relative_path, + ); + + $class_file = (new \ReflectionClass($class))->getFileName(); + if ($class_file === FALSE) { + return FALSE; + } + + // Normalize the directory separator to '/'. + $class_file = str_replace(DIRECTORY_SEPARATOR, '/', $class_file); + + foreach ($suffixes as $suffix) { + $dir = $this->stringRemoveSuffix($class_file, $suffix); + if (FALSE !== $dir) { + $module_info_file = $dir . '/' . $module_name . '.info.yml'; + if (is_file($module_info_file)) { + return $module_info_file; + } + } + } + + return FALSE; + } + + /** + * Removes a suffix from a string, if possible. + * + * @todo Put this in a globally accessible place. + * + * @param string $string + * A string to test. + * @param string $suffix + * A string that could be a suffix of $string. + * + * @return bool|string + * The $prefix, so that $prefix . $suffix === $string, or + * FALSE, if $string does not end with $suffix. */ - abstract protected function getModuleInfoFilePath(); + private function stringRemoveSuffix($string, $suffix) { + if (FALSE !== $pos = strrpos($string, $suffix)) { + if ($pos + strlen($suffix) === strlen($string)) { + // $string ends with $suffix, so return the beginning of $string. + return substr($string, 0, $pos); + } + } + // $string does not end with $suffix. + return FALSE; + } } -- 1.8.5.1 From fd30a64de60a2041d69e42f03fca617a79c0d52f Mon Sep 17 00:00:00 2001 From: Andreas Hennings Date: Wed, 21 May 2014 22:30:31 +0200 Subject: [PATCH 4/5] Improve core/scripts/switch-psr4.sh, to make sure that remaining empty PSR-0 directories are removed. --- core/scripts/switch-psr4.sh | 108 ++++++++++++++++++++++++++++++++++++-------- 1 file changed, 89 insertions(+), 19 deletions(-) diff --git a/core/scripts/switch-psr4.sh b/core/scripts/switch-psr4.sh index 07b6826..f460647 100644 --- a/core/scripts/switch-psr4.sh +++ b/core/scripts/switch-psr4.sh @@ -187,7 +187,9 @@ function process_candidate_dir($dir) { function process_extension($name, $dir) { if (!is_dir($source = "$dir/lib/Drupal/$name")) { - // Nothing to do in this module. + // No PSR-0 class files to be moved, but there could still be a remaining + // empty "lib" directory. + remove_empty_directory_tree_if_exists("$dir/lib"); return; } @@ -199,8 +201,7 @@ function process_extension($name, $dir) { move_directory_contents($source, $destination); // Clean up. - require_dir_empty("$dir/lib/Drupal"); - rmdir("$dir/lib/Drupal"); + remove_empty_directory_tree("$dir/lib"); } /** @@ -218,6 +219,9 @@ function process_extension_phpunit($name, $dir) { if (!is_dir($source = "$dir/tests/Drupal/$name/Tests")) { // Nothing to do in this module. + // No PSR-0 PHPUnit class files to be moved, but there could still be a + // remaining empty "tests/lib" directory. + remove_empty_directory_tree_if_exists("$dir/tests/lib"); return; } @@ -229,10 +233,7 @@ function process_extension_phpunit($name, $dir) { move_directory_contents($source, $dest); // Clean up. - require_dir_empty("$dir/tests/Drupal/$name"); - rmdir("$dir/tests/Drupal/$name"); - require_dir_empty("$dir/tests/Drupal"); - rmdir("$dir/tests/Drupal"); + remove_empty_directory_tree("$dir/tests/Drupal"); } /** @@ -284,6 +285,63 @@ function move_directory_contents($source, $destination) { } /** + * Removes a directory tree, if it exists and it contains no files. + * + * Throws an exception, if the given path is a file instead of a directory, or + * if the directory tree contains any files. + * + * @param string $dir + * Path to the directory tree to be removed. + * + * @throws \Exception + */ +function remove_empty_directory_tree_if_exists($dir) { + + if (!file_exists($dir)) { + // The path is neither a file nor a directory. + return; + } + + // Recursively remove the directory and subfolders, and throw an exception if + // any file is found. + remove_empty_directory_tree($dir); +} + +/** + * Removes a directory tree, if it contains only subdirectories and no files. + * + * Throws an exception if any file is found. + * + * @param string $dir + * Path to the directory tree to be removed. + * + * @throws \Exception + */ +function remove_empty_directory_tree($dir) { + + // Throw an exception if the directory does not exist. + require_is_readable_dir($dir); + + // Recursively remove subdirectories, and verify that no files exist anywhere + // in the directory tree. + foreach (new \DirectoryIterator($dir) as $fileinfo) { + if ($fileinfo->isDot()) { + continue; + } + $path = $fileinfo->getPathname(); + if ($fileinfo->isFile()) { + throw new \Exception("File '$path' found in a directory that is expected to not contain any files."); + } + if ($fileinfo->isDir()) { + remove_empty_directory_tree($path); + } + } + + // Remove the parent directory. + rmdir($dir); +} + +/** * Throws an exception if a directory is not empty. * * @param string $dir @@ -292,18 +350,11 @@ function move_directory_contents($source, $destination) { * @throws \Exception */ function require_dir_empty($dir) { - if (is_file($dir)) { - throw new \Exception("The path '$dir' is a file, when it should be a directory."); - } - if (!is_dir($dir)) { - throw new \Exception("The directory '$dir' does not exist."); - } - if (!is_readable($dir)) { - throw new \Exception("The directory '$dir' is not readable."); - } - /** - * @var \DirectoryIterator $fileinfo - */ + + // Throw an exception if the directory does not exist. + require_is_readable_dir($dir); + + // Verify that no files or subfolders exist anywhere in the directory. foreach (new \DirectoryIterator($dir) as $fileinfo) { if ($fileinfo->isDot()) { continue; @@ -317,3 +368,22 @@ function require_dir_empty($dir) { } } } + +/** + * Throws an exception if a given path is not a readable directory. + * + * @param $dir + * + * @throws \Exception + */ +function require_is_readable_dir($dir) { + if (is_file($dir)) { + throw new \Exception("The path '$dir' is a file, when it should be a directory."); + } + if (!is_dir($dir)) { + throw new \Exception("The directory '$dir' does not exist."); + } + if (!is_readable($dir)) { + throw new \Exception("The directory '$dir' is not readable."); + } +} -- 1.8.5.1 From da6ffcf45587269741d175dadb8b8e8b9519e3d8 Mon Sep 17 00:00:00 2001 From: Andreas Hennings Date: Wed, 21 May 2014 17:58:39 +0200 Subject: [PATCH 5/5] Throw exceptions in case that $definition['class'] is not a class of the expected type. --- core/lib/Drupal/Core/Field/FieldDefinition.php | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/core/lib/Drupal/Core/Field/FieldDefinition.php b/core/lib/Drupal/Core/Field/FieldDefinition.php index cfeccf8..7b695f1 100644 --- a/core/lib/Drupal/Core/Field/FieldDefinition.php +++ b/core/lib/Drupal/Core/Field/FieldDefinition.php @@ -475,7 +475,16 @@ public function getSchema() { if (!isset($this->schema)) { // Get the schema from the field item class. $definition = \Drupal::service('plugin.manager.field.field_type')->getDefinition($this->getType()); + if (!isset($definition['class'])) { + throw new \Exception("\$definition['class'] is not set."); + } $class = $definition['class']; + if (!class_exists($class)) { + throw new \Exception(var_export($class, TRUE) . " is not a class."); + } + if (!is_subclass_of($class, 'Drupal\Core\Field\FieldItemInterface')) { + throw new \Exception("class $class must implement FieldItemInterface."); + } $schema = $class::schema($this); // Fill in default values. $schema += array( -- 1.8.5.1