From 6a339e08fa9c1b116d5c823c3038273e9ff3c5fd Mon Sep 17 00:00:00 2001 From: Thrijith Thankachan Date: Tue, 23 Apr 2019 08:22:40 +0530 Subject: [PATCH 1/3] PHPCS: Add new ruleset to the project Update .distignore and .gitignore with phpcs/phpunit config files Update wp-cli-tests to 2.1 --- .distignore | 2 ++ .gitignore | 3 +++ composer.json | 2 +- phpcs.xml.dist | 54 ++++++++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 60 insertions(+), 1 deletion(-) create mode 100644 phpcs.xml.dist diff --git a/.distignore b/.distignore index b964b40c7..95b52fb02 100644 --- a/.distignore +++ b/.distignore @@ -6,6 +6,8 @@ .travis.yml behat.yml circle.yml +phpcs.xml.dist +phpunit.xml.dist bin/ features/ utils/ diff --git a/.gitignore b/.gitignore index ff4941991..bcf211b32 100644 --- a/.gitignore +++ b/.gitignore @@ -6,3 +6,6 @@ vendor/ *.tar.gz composer.lock *.log +phpunit.xml +phpcs.xml +.phpcs.xml diff --git a/composer.json b/composer.json index 805af023f..1968f8033 100644 --- a/composer.json +++ b/composer.json @@ -18,7 +18,7 @@ }, "require-dev": { "wp-cli/scaffold-command": "^1 || ^2", - "wp-cli/wp-cli-tests": "^2.0.7" + "wp-cli/wp-cli-tests": "^2.1" }, "config": { "process-timeout": 7200, diff --git a/phpcs.xml.dist b/phpcs.xml.dist new file mode 100644 index 000000000..1d5cd073f --- /dev/null +++ b/phpcs.xml.dist @@ -0,0 +1,54 @@ + + + Custom ruleset for WP-CLI package-command + + + + + . + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + From 243d1ebe953fd452c830a1858a48d48712e136fd Mon Sep 17 00:00:00 2001 From: Thrijith Thankachan Date: Tue, 23 Apr 2019 12:37:17 +0530 Subject: [PATCH 2/3] PHPCS: Update ruleset to exclude classes from PrefixAllGlobals.NonPrefixedClassFound sniff and some files Add CS fixes in src/Package_Command.php tests/test-composer-json.php --- package-command.php | 6 +- phpcs.xml.dist | 10 + src/Package_Command.php | 366 +++++++++++++++++++---------------- tests/test-composer-json.php | 36 ++-- 4 files changed, 235 insertions(+), 183 deletions(-) diff --git a/package-command.php b/package-command.php index 9aa3a9dca..8736f5b50 100644 --- a/package-command.php +++ b/package-command.php @@ -4,8 +4,8 @@ return; } -$autoload = dirname( __FILE__ ) . '/vendor/autoload.php'; -if ( file_exists( $autoload ) && ! class_exists( 'Package_Command' ) ) { - require_once $autoload; +$wpcli_package_autoloader = dirname( __FILE__ ) . '/vendor/autoload.php'; +if ( file_exists( $wpcli_package_autoloader ) && ! class_exists( 'Package_Command' ) ) { + require_once $wpcli_package_autoloader; } WP_CLI::add_command( 'package', 'Package_Command' ); diff --git a/phpcs.xml.dist b/phpcs.xml.dist index 1d5cd073f..fd7c40d41 100644 --- a/phpcs.xml.dist +++ b/phpcs.xml.dist @@ -13,6 +13,11 @@ . + + */src/WP_CLI/JsonManipulator\.php$ + */tests/test-json-manipulator\.php$ + @@ -51,4 +56,9 @@ + + + */src/Package_Command\.php$ + + diff --git a/src/Package_Command.php b/src/Package_Command.php index 52a5fee96..5ca78bfe0 100644 --- a/src/Package_Command.php +++ b/src/Package_Command.php @@ -1,4 +1,5 @@ set_composer_auth_env_var(); - $git_package = $dir_package = false; - $version = 'dev-master'; + $git_package = false; + $dir_package = false; + $version = 'dev-master'; if ( $this->is_git_repository( $package_name ) ) { $git_package = $package_name; preg_match( '#([^:\/]+\/[^\/]+)\.git#', $package_name, $matches ); @@ -216,18 +218,18 @@ public function install( $args, $assoc_args ) { } else { WP_CLI::error( "Couldn't parse package name from expected path '/'." ); } - } else if ( ( false !== strpos( $package_name, '://' ) && false !== stripos( $package_name, '.zip' ) ) + } elseif ( ( false !== strpos( $package_name, '://' ) && false !== stripos( $package_name, '.zip' ) ) || ( pathinfo( $package_name, PATHINFO_EXTENSION ) === 'zip' && is_file( $package_name ) ) ) { // Download the remote ZIP file to a temp directory $temp = false; if ( false !== strpos( $package_name, '://' ) ) { - $temp = Utils\get_temp_dir() . uniqid( 'wp-cli-package_', true /*more_entropy*/ ) . ".zip"; - $options = array( - 'timeout' => 600, - 'filename' => $temp - ); - $response = Utils\http_request( 'GET', $package_name, null, array(), $options ); - if ( 20 != substr( $response->status_code, 0, 2 ) ) { + $temp = Utils\get_temp_dir() . uniqid( 'wp-cli-package_', true /*more_entropy*/ ) . '.zip'; + $options = [ + 'timeout' => 600, + 'filename' => $temp, + ]; + $response = Utils\http_request( 'GET', $package_name, null, [], $options ); + if ( 20 !== (int) substr( $response->status_code, 0, 2 ) ) { @unlink( $temp ); // @codingStandardsIgnoreLine WP_CLI::error( sprintf( "Couldn't download package from '%s' (HTTP code %d).", $package_name, $response->status_code ) ); } @@ -243,7 +245,7 @@ public function install( $args, $assoc_args ) { } list( $package_name, $version ) = self::get_package_name_and_version_from_dir_package( $dir_package ); // Move to a location based on the package name - $local_dir = rtrim( WP_CLI::get_runner()->get_packages_dir_path(), '/' ) . '/local/'; + $local_dir = rtrim( WP_CLI::get_runner()->get_packages_dir_path(), '/' ) . '/local/'; $actual_dir_package = $local_dir . str_replace( '/', '-', $package_name ); Extractor::copy_overwrite_files( $dir_package, $actual_dir_package ); Extractor::rmdir( $dir_package ); @@ -257,12 +259,12 @@ public function install( $args, $assoc_args ) { try { Extractor::rmdir( $dir_package ); } catch ( Exception $rmdir_e ) { - // Ignore. + WP_CLI::warning( $rmdir_e->getMessage() ); } } WP_CLI::error( $e->getMessage() ); } - } else if ( is_dir( $package_name ) && file_exists( $package_name . '/composer.json' ) ) { + } elseif ( is_dir( $package_name ) && file_exists( $package_name . '/composer.json' ) ) { $dir_package = $package_name; if ( ! Utils\is_path_absolute( $dir_package ) ) { $dir_package = getcwd() . DIRECTORY_SEPARATOR . $dir_package; @@ -291,7 +293,7 @@ public function install( $args, $assoc_args ) { } } - WP_CLI::log( sprintf( "Installing package %s (%s)", $package_name, $version ) ); + WP_CLI::log( sprintf( 'Installing package %s (%s)', $package_name, $version ) ); // Read the WP-CLI packages composer.json and do some initial error checking. list( $json_path, $composer_backup, $composer_backup_decoded ) = $this->get_composer_json_path_backup_decoded(); @@ -301,7 +303,7 @@ public function install( $args, $assoc_args ) { $this->register_revert_shutdown_function( $json_path, $composer_backup, $revert ); // Add the 'require' to composer.json - WP_CLI::log( sprintf( "Updating %s to require the package...", $json_path ) ); + WP_CLI::log( sprintf( 'Updating %s to require the package...', $json_path ) ); $json_manipulator = new JsonManipulator( $composer_backup ); $json_manipulator->addMainKey( 'name', 'wp-cli/wp-cli' ); $json_manipulator->addMainKey( 'version', self::get_wp_cli_version_composer() ); @@ -310,25 +312,47 @@ public function install( $args, $assoc_args ) { if ( $git_package ) { WP_CLI::log( sprintf( 'Registering %s as a VCS repository...', $git_package ) ); - $json_manipulator->addSubNode( 'repositories', $package_name, array( 'type' => 'vcs', 'url' => $git_package ), true /*caseInsensitive*/ ); - } else if ( $dir_package ) { + $json_manipulator->addSubNode( + 'repositories', + $package_name, + [ + 'type' => 'vcs', + 'url' => $git_package, + ], + true /*caseInsensitive*/ + ); + } elseif ( $dir_package ) { WP_CLI::log( sprintf( 'Registering %s as a path repository...', $dir_package ) ); - $json_manipulator->addSubNode( 'repositories', $package_name, array( 'type' => 'path', 'url' => $dir_package ), true /*caseInsensitive*/ ); + $json_manipulator->addSubNode( + 'repositories', + $package_name, + [ + 'type' => 'path', + 'url' => $dir_package, + ], + true /*caseInsensitive*/ + ); } // If the composer file does not contain the current package index repository, refresh the repository definition. - if ( empty( $composer_backup_decoded['repositories']['wp-cli']['url'] ) || self::PACKAGE_INDEX_URL != $composer_backup_decoded['repositories']['wp-cli']['url'] ) { + if ( empty( $composer_backup_decoded['repositories']['wp-cli']['url'] ) || self::PACKAGE_INDEX_URL !== $composer_backup_decoded['repositories']['wp-cli']['url'] ) { WP_CLI::log( 'Updating package index repository url...' ); - $json_manipulator->addRepository( 'wp-cli', array( 'type' => 'composer', 'url' => self::PACKAGE_INDEX_URL ) ); + $json_manipulator->addRepository( + 'wp-cli', + [ + 'type' => 'composer', + 'url' => self::PACKAGE_INDEX_URL, + ] + ); } file_put_contents( $json_path, $json_manipulator->getContents() ); $composer = $this->get_composer(); // Set up the EventSubscriber - $event_subscriber = new \WP_CLI\PackageManagerEventSubscriber; + $event_subscriber = new \WP_CLI\PackageManagerEventSubscriber(); $composer->getEventDispatcher()->addSubscriber( $event_subscriber ); // Set up the installer - $install = Installer::create( new ComposerIO, $composer ); + $install = Installer::create( new ComposerIO(), $composer ); $install->setUpdate( true ); // Installer class will only override composer.lock with this flag $install->setPreferSource( true ); // Use VCS when VCS for easier contributions. @@ -345,7 +369,7 @@ public function install( $args, $assoc_args ) { if ( 0 === $res ) { $revert = false; - WP_CLI::success( "Package installed." ); + WP_CLI::success( 'Package installed.' ); } else { $res_msg = $res ? " (Composer return code {$res})" : ''; // $res may be null apparently. WP_CLI::debug( "composer.json content:\n" . file_get_contents( $json_path ), 'packages' ); @@ -423,7 +447,7 @@ public function list_( $args, $assoc_args ) { * $ cd $(wp package path) && pwd * /home/vagrant/.wp-cli/packages */ - function path( $args ) { + public function path( $args ) { $packages_dir = WP_CLI::get_runner()->get_packages_dir_path(); if ( ! empty( $args ) ) { $packages_dir .= 'vendor/' . $args[0]; @@ -458,11 +482,11 @@ public function update() { $composer = $this->get_composer(); // Set up the EventSubscriber - $event_subscriber = new \WP_CLI\PackageManagerEventSubscriber; + $event_subscriber = new \WP_CLI\PackageManagerEventSubscriber(); $composer->getEventDispatcher()->addSubscriber( $event_subscriber ); // Set up the installer - $install = Installer::create( new ComposerIO, $composer ); + $install = Installer::create( new ComposerIO(), $composer ); $install->setUpdate( true ); // Installer class will only override composer.lock with this flag $install->setPreferSource( true ); // Use VCS when VCS for easier contributions. WP_CLI::log( 'Using Composer to update packages...' ); @@ -476,7 +500,7 @@ public function update() { WP_CLI::log( '---' ); if ( 0 === $res ) { - WP_CLI::success( "Packages updated." ); + WP_CLI::success( 'Packages updated.' ); } else { $res_msg = $res ? " (Composer return code {$res})" : ''; // $res may be null apparently. WP_CLI::error( "Failed to update packages{$res_msg}." ); @@ -503,8 +527,9 @@ public function uninstall( $args ) { list( $package_name ) = $args; $this->set_composer_auth_env_var(); - if ( false === ( $package = $this->get_installed_package_by_name( $package_name ) ) ) { - WP_CLI::error( "Package not installed." ); + $package = $this->get_installed_package_by_name( $package_name ); + if ( false === $package ) { + WP_CLI::error( 'Package not installed.' ); } $package_name = $package->getPrettyName(); // Make sure package name is what's in composer.json. @@ -528,7 +553,7 @@ public function uninstall( $args ) { $composer = $this->get_composer(); // Set up the installer. - $install = Installer::create( new NullIO, $composer ); + $install = Installer::create( new NullIO(), $composer ); $install->setUpdate( true ); // Installer class will only override composer.lock with this flag $install->setPreferSource( true ); // Use VCS when VCS for easier contributions. @@ -542,7 +567,7 @@ public function uninstall( $args ) { if ( 0 === $res ) { $revert = false; - WP_CLI::success( "Uninstalled package." ); + WP_CLI::success( 'Uninstalled package.' ); } else { $res_msg = $res ? " (Composer return code {$res})" : ''; // $res may be null apparently. WP_CLI::error( "Package removal failed{$res_msg}." ); @@ -574,9 +599,10 @@ private function get_composer() { chdir( pathinfo( $composer_path, PATHINFO_DIRNAME ) ); // Prevent DateTime error/warning when no timezone set + // phpcs:ignore WordPress.WP.TimezoneChange.timezone_change_date_default_timezone_set, WordPress.PHP.NoSilencedErrors.Discouraged -- The package is loaded before WordPress load, For environments that don't have set time in php.ini. date_default_timezone_set( @date_default_timezone_get() ); - $composer = Factory::create( new NullIO, $composer_path ); + $composer = Factory::create( new NullIO(), $composer_path ); } catch ( Exception $e ) { WP_CLI::error( sprintf( 'Failed to get composer instance: %s', $e->getMessage() ) ); } @@ -595,7 +621,7 @@ private function get_community_packages() { $this->avoid_composer_ca_bundle(); try { $community_packages = $this->package_index()->getPackages(); - } catch( Exception $e ) { + } catch ( Exception $e ) { WP_CLI::error( $e->getMessage() ); } } @@ -614,18 +640,20 @@ private function get_community_packages() { private function package_index() { static $package_index; - if ( !$package_index ) { + if ( ! $package_index ) { $config = new Config(); - $config->merge( array( - 'config' => array( - 'secure-http' => true, - 'home' => dirname( $this->get_composer_json_path() ), - ) - )); + $config->merge( + [ + 'config' => [ + 'secure-http' => true, + 'home' => dirname( $this->get_composer_json_path() ), + ], + ] + ); $config->setConfigSource( new JsonConfigSource( $this->get_composer_json() ) ); try { - $package_index = new ComposerRepository( array( 'url' => self::PACKAGE_INDEX_URL ), new NullIO, $config ); + $package_index = new ComposerRepository( [ 'url' => self::PACKAGE_INDEX_URL ], new NullIO(), $config ); } catch ( Exception $e ) { WP_CLI::error( $e->getMessage() ); } @@ -643,70 +671,74 @@ private function package_index() { */ private function show_packages( $context, $packages, $assoc_args ) { if ( 'list' === $context ) { - $default_fields = array( + $default_fields = [ 'name', 'authors', 'version', 'update', 'update_version', - ); - } else if ( 'browse' === $context ) { - $default_fields = array( + ]; + } elseif ( 'browse' === $context ) { + $default_fields = [ 'name', 'description', 'authors', 'version', - ); + ]; } - $defaults = array( + $defaults = [ 'fields' => implode( ',', $default_fields ), - 'format' => 'table' - ); + 'format' => 'table', + ]; $assoc_args = array_merge( $defaults, $assoc_args ); $composer = $this->get_composer(); - $list = array(); + $list = []; foreach ( $packages as $package ) { $name = $package->getPrettyName(); if ( isset( $list[ $name ] ) ) { $list[ $name ]['version'][] = $package->getPrettyVersion(); } else { - $package_output = array(); - $package_output['name'] = $package->getPrettyName(); + $package_output = []; + $package_output['name'] = $package->getPrettyName(); $package_output['description'] = $package->getDescription(); - $package_output['authors'] = implode( ', ', array_column( (array) $package->getAuthors(), 'name' ) ); - $package_output['version'] = array( $package->getPrettyVersion() ); - $update = 'none'; - $update_version = ''; + $package_output['authors'] = implode( ', ', array_column( (array) $package->getAuthors(), 'name' ) ); + $package_output['version'] = [ $package->getPrettyVersion() ]; + $update = 'none'; + $update_version = ''; if ( 'list' === $context ) { try { $latest = $this->find_latest_package( $package, $composer, null ); if ( $latest && $latest->getFullPrettyVersion() !== $package->getFullPrettyVersion() ) { - $update = 'available'; + $update = 'available'; $update_version = $latest->getPrettyVersion(); } } catch ( Exception $e ) { WP_CLI::warning( $e->getMessage() ); - $update = $update_version = 'error'; + $update = 'error'; + $update_version = $update; } } - $package_output['update'] = $update; + $package_output['update'] = $update; $package_output['update_version'] = $update_version; - $package_output['pretty_name'] = $package->getPrettyName(); // Deprecated but kept for BC with package-command 1.0.8. - $list[ $package_output['name'] ] = $package_output; + $package_output['pretty_name'] = $package->getPrettyName(); // Deprecated but kept for BC with package-command 1.0.8. + $list[ $package_output['name'] ] = $package_output; } } - $list = array_map( function( $package ){ - $package['version'] = implode( ', ', $package['version'] ); - return $package; - }, $list ); + $list = array_map( + function( $package ) { + $package['version'] = implode( ', ', $package['version'] ); + return $package; + }, + $list + ); ksort( $list ); if ( 'ids' === $assoc_args['format'] ) { $list = array_keys( $list ); } - WP_CLI\Utils\format_items( $assoc_args['format'], $list, $assoc_args['fields'] ); + Utils\format_items( $assoc_args['format'], $list, $assoc_args['fields'] ); } /** @@ -720,7 +752,7 @@ private function show_packages( $context, $packages, $assoc_args ) { private function get_package_by_shortened_identifier( $package_name ) { // Check the package index first, so we don't break existing behavior. $lc_package_name = strtolower( $package_name ); // For BC check. - foreach( $this->get_community_packages() as $package ) { + foreach ( $this->get_community_packages() as $package ) { if ( $package_name === $package->getPrettyName() ) { return $package; } @@ -731,17 +763,17 @@ private function get_package_by_shortened_identifier( $package_name ) { } // Check if the package exists on Packagist. - $url = "https://repo.packagist.org/p/{$package_name}.json"; + $url = "https://repo.packagist.org/p/{$package_name}.json"; $response = Utils\http_request( 'GET', $url ); if ( 20 === (int) substr( $response->status_code, 0, 2 ) ) { return $package_name; } // Fall back to GitHub URL if we had no match yet. - $url = "https://github.com/{$package_name}.git"; + $url = "https://github.com/{$package_name}.git"; $github_token = getenv( 'GITHUB_TOKEN' ); // Use GITHUB_TOKEN if available to avoid authorization failures or rate-limiting. - $headers = $github_token ? array( 'Authorization' => 'token ' . $github_token ) : array(); - $response = Utils\http_request( 'GET', $url, null /*data*/, $headers ); + $headers = $github_token ? [ 'Authorization' => 'token ' . $github_token ] : []; + $response = Utils\http_request( 'GET', $url, null /*data*/, $headers ); if ( 20 === (int) substr( $response->status_code, 0, 2 ) ) { return $url; } @@ -755,20 +787,21 @@ private function get_package_by_shortened_identifier( $package_name ) { private function get_installed_packages() { $composer = $this->get_composer(); - $repo = $composer->getRepositoryManager()->getLocalRepository(); - $existing = json_decode( file_get_contents( $this->get_composer_json_path() ), true ); - $installed_package_keys = ! empty( $existing['require'] ) ? array_keys( $existing['require'] ) : array(); + $repo = $composer->getRepositoryManager()->getLocalRepository(); + $existing = json_decode( file_get_contents( $this->get_composer_json_path() ), true ); + $installed_package_keys = ! empty( $existing['require'] ) ? array_keys( $existing['require'] ) : []; if ( empty( $installed_package_keys ) ) { - return array(); + return []; } // For use by legacy incorrect name check. $lc_installed_package_keys = array_map( 'strtolower', $installed_package_keys ); - $installed_packages = array(); - foreach( $repo->getCanonicalPackages() as $package ) { + $installed_packages = []; + foreach ( $repo->getCanonicalPackages() as $package ) { + $idx = array_search( $package->getName(), $lc_installed_package_keys, true ); // Use pretty name as it's case sensitive and what's in composer.json (or at least should be). if ( in_array( $package->getPrettyName(), $installed_package_keys, true ) ) { $installed_packages[] = $package; - } elseif ( false !== ( $idx = array_search( $package->getName(), $lc_installed_package_keys, true ) ) ) { // Legacy incorrect name check. + } elseif ( false !== $idx ) { // Legacy incorrect name check. WP_CLI::warning( sprintf( "Found package '%s' misnamed '%s' in '%s'.", $package->getPrettyName(), $installed_package_keys[ $idx ], $this->get_composer_json_path() ) ); $installed_packages[] = $package; } @@ -780,7 +813,7 @@ private function get_installed_packages() { * Gets an installed package by its name. */ private function get_installed_package_by_name( $package_name ) { - foreach( $this->get_installed_packages() as $package ) { + foreach ( $this->get_installed_packages() as $package ) { if ( $package_name === $package->getPrettyName() ) { return $package; } @@ -822,11 +855,11 @@ private static function get_package_name_and_version_from_dir_package( $dir_pack WP_CLI::error( sprintf( "Invalid package: no name in composer.json file '%s'.", $composer_file ) ); } $package_name = $composer_data['name']; - $version = 'dev-master'; + $version = 'dev-master'; if ( ! empty( $composer_data['version'] ) ) { $version = $composer_data['version']; } - return array( $package_name, $version ); + return [ $package_name, $version ]; } /** @@ -900,35 +933,35 @@ private function create_default_composer_json( $composer_path ) { $json_file = new JsonFile( $composer_path ); - $author = (object)array( - 'name' => 'WP-CLI', - 'email' => 'noreply@wpcli.org' - ); - - $repositories = (object)array( - 'wp-cli' => (object)array( - 'type' => 'composer', - 'url' => self::PACKAGE_INDEX_URL, - ), - ); - - $options = array( - 'name' => 'wp-cli/wp-cli', - 'description' => 'Installed community packages used by WP-CLI', - 'version' => self::get_wp_cli_version_composer(), - 'authors' => array( $author ), - 'homepage' => self::PACKAGE_INDEX_URL, - 'require' => new stdClass, - 'require-dev' => new stdClass, + $author = (object) [ + 'name' => 'WP-CLI', + 'email' => 'noreply@wpcli.org', + ]; + + $repositories = (object) [ + 'wp-cli' => (object) [ + 'type' => 'composer', + 'url' => self::PACKAGE_INDEX_URL, + ], + ]; + + $options = [ + 'name' => 'wp-cli/wp-cli', + 'description' => 'Installed community packages used by WP-CLI', + 'version' => self::get_wp_cli_version_composer(), + 'authors' => [ $author ], + 'homepage' => self::PACKAGE_INDEX_URL, + 'require' => new stdClass(), + 'require-dev' => new stdClass(), 'minimum-stability' => 'dev', - 'prefer-stable' => true, - 'license' => 'MIT', - 'repositories' => $repositories, - ); + 'prefer-stable' => true, + 'license' => 'MIT', + 'repositories' => $repositories, + ]; try { $json_file->write( $options ); - } catch( Exception $e ) { + } catch ( Exception $e ) { WP_CLI::error( $e->getMessage() ); } @@ -945,33 +978,33 @@ private function create_default_composer_json( $composer_path ) { * * @return PackageInterface|null */ - private function find_latest_package( PackageInterface $package, Composer $composer, $phpVersion, $minorOnly = false ) { + private function find_latest_package( PackageInterface $package, Composer $composer, $php_version, $minor_only = false ) { // find the latest version allowed in this pool - $name = $package->getPrettyName(); - $versionSelector = new VersionSelector($this->get_pool($composer)); - $stability = $composer->getPackage()->getMinimumStability(); - $flags = $composer->getPackage()->getStabilityFlags(); - if (isset($flags[$name])) { - $stability = array_search($flags[$name], BasePackage::$stabilities, true); + $name = $package->getPrettyName(); + $version_selector = new VersionSelector( $this->get_pool( $composer ) ); + $stability = $composer->getPackage()->getMinimumStability(); + $flags = $composer->getPackage()->getStabilityFlags(); + if ( isset( $flags[ $name ] ) ) { + $stability = array_search( $flags[ $name ], BasePackage::$stabilities, true ); } - $bestStability = $stability; - if ($composer->getPackage()->getPreferStable()) { - $bestStability = $package->getStability(); + $best_stability = $stability; + if ( $composer->getPackage()->getPreferStable() ) { + $best_stability = $package->getStability(); } - $targetVersion = null; - if (0 === strpos($package->getVersion(), 'dev-')) { - $targetVersion = $package->getVersion(); + $target_version = null; + if ( 0 === strpos( $package->getVersion(), 'dev-' ) ) { + $target_version = $package->getVersion(); } - if ($targetVersion === null && $minorOnly) { - $targetVersion = '^' . $package->getVersion(); + if ( null === $target_version && $minor_only ) { + $target_version = '^' . $package->getVersion(); } - return $versionSelector->findBestCandidate($name, $targetVersion, $phpVersion, $bestStability); + return $version_selector->findBestCandidate( $name, $target_version, $php_version, $best_stability ); } private function get_pool( Composer $composer ) { - if (!$this->pool) { - $this->pool = new Pool($composer->getPackage()->getMinimumStability(), $composer->getPackage()->getStabilityFlags()); - $this->pool->addRepository(new CompositeRepository($composer->getRepositoryManager()->getRepositories())); + if ( ! $this->pool ) { + $this->pool = new Pool( $composer->getPackage()->getMinimumStability(), $composer->getPackage()->getStabilityFlags() ); + $this->pool->addRepository( new CompositeRepository( $composer->getRepositoryManager()->getRepositories() ) ); } return $this->pool; } @@ -997,11 +1030,11 @@ private function is_git_repository( $package ) { private function check_git_package_name( $package_name, $version = '' ) { // Generate raw git URL of composer.json file. $raw_content_url = 'https://raw.githubusercontent.com/' . $package_name . '/' . $this->get_raw_git_version( $version ) . '/composer.json'; - $github_token = getenv( 'GITHUB_TOKEN' ); // Use GITHUB_TOKEN if available to avoid authorization failures or rate-limiting. - $headers = $github_token ? array( 'Authorization' => 'token ' . $github_token ) : array(); + $github_token = getenv( 'GITHUB_TOKEN' ); // Use GITHUB_TOKEN if available to avoid authorization failures or rate-limiting. + $headers = $github_token ? [ 'Authorization' => 'token ' . $github_token ] : []; - $response = WP_CLI\Utils\http_request( 'GET', $raw_content_url, null /*data*/, $headers ); - if ( 20 != substr( $response->status_code, 0, 2 ) ) { + $response = Utils\http_request( 'GET', $raw_content_url, null /*data*/, $headers ); + if ( 20 !== (int) substr( $response->status_code, 0, 2 ) ) { // Could not get composer.json. Possibly private so warn and return best guess from input (always xxx/xxx). WP_CLI::warning( sprintf( "Couldn't download composer.json file from '%s' (HTTP code %d). Presuming package name is '%s'.", $raw_content_url, $response->status_code, $package_name ) ); return $package_name; @@ -1039,7 +1072,8 @@ private function get_raw_git_version( $version ) { } // If Composer hash given then just use whatever's after it. - if ( false !== ( $hash_pos = strpos( $version, '#' ) ) ) { + $hash_pos = strpos( $version, '#' ); + if ( false !== $hash_pos ) { return substr( $version, $hash_pos + 1 ); } @@ -1049,7 +1083,7 @@ private function get_raw_git_version( $version ) { } // Ignore/strip any relative suffixes. - return str_replace( array( '^', '~' ), '', $version ); + return str_replace( [ '^', '~' ], '', $version ); } /** @@ -1060,7 +1094,7 @@ private function get_raw_git_version( $version ) { * @return string Release tag. */ private function get_github_latest_release_tag( $package_name ) { - $url = "https://api.github.com/repos/{$package_name}/releases/latest"; + $url = "https://api.github.com/repos/{$package_name}/releases/latest"; $response = Utils\http_request( 'GET', $url ); if ( 20 !== (int) substr( $response->status_code, 0, 2 ) ) { WP_CLI::warning( 'Could not guess stable version from GitHub repository, falling back to master branch' ); @@ -1087,8 +1121,8 @@ private function get_github_latest_release_tag( $package_name ) { * @return string Version constraint. */ private function guess_version_constraint_from_tag( $tag ) { - $matches = array(); - if ( 1 !== preg_match( "/(?:version|v)\s*((?:[0-9]+\.?)+)(?:-.*)/i", $tag, $matches ) ) { + $matches = []; + if ( 1 !== preg_match( '/(?:version|v)\s*((?:[0-9]+\.?)+)(?:-.*)/i', $tag, $matches ) ) { return $tag; } @@ -1103,14 +1137,18 @@ private function guess_version_constraint_from_tag( $tag ) { * Avoids authorization failures when accessing various sites. */ private function set_composer_auth_env_var() { - $changed = false; + $changed = false; $composer_auth = getenv( 'COMPOSER_AUTH' ); - if ( ! $composer_auth || ! ( $composer_auth = json_decode( $composer_auth, true /*assoc*/ ) ) || ! is_array( $composer_auth ) ) { - $composer_auth = array(); + if ( ! $composer_auth ) { + $composer_auth = json_decode( $composer_auth, true /*assoc*/ ); } - if ( ! isset( $composer_auth['github-oauth'] ) && ( $github_token = getenv( 'GITHUB_TOKEN' ) ) ) { - $composer_auth['github-oauth'] = array( 'github.com' => $github_token ); - $changed = true; + if ( ! $composer_auth || ! is_array( $composer_auth ) ) { + $composer_auth = []; + } + $github_token = getenv( 'GITHUB_TOKEN' ); + if ( ! isset( $composer_auth['github-oauth'] ) && $github_token ) { + $composer_auth['github-oauth'] = [ 'github.com' => $github_token ]; + $changed = true; } if ( $changed ) { putenv( 'COMPOSER_AUTH=' . json_encode( $composer_auth ) ); @@ -1135,8 +1173,8 @@ private function avoid_composer_ca_bundle() { */ private function get_composer_json_path_backup_decoded() { $composer_json_obj = $this->get_composer_json(); - $json_path = $composer_json_obj->getPath(); - $composer_backup = file_get_contents( $json_path ); + $json_path = $composer_json_obj->getPath(); + $composer_backup = file_get_contents( $json_path ); if ( false === $composer_backup ) { $error = error_get_last(); WP_CLI::error( sprintf( "Failed to read '%s': %s", $json_path, $error['message'] ) ); @@ -1147,7 +1185,7 @@ private function get_composer_json_path_backup_decoded() { WP_CLI::error( sprintf( "Failed to parse '%s' as json: %s", $json_path, $e->getMessage() ) ); } - return array( $json_path, $composer_backup, $composer_backup_decoded ); + return [ $json_path, $composer_backup, $composer_backup_decoded ]; } /** @@ -1164,14 +1202,15 @@ private function register_revert_shutdown_function( $json_path, $composer_backup $revert_fail_msg = "Failed to revert composer.json.\n"; $memory_msg = "WP-CLI ran out of memory. Please see https://bit.ly/wpclimem for further help.\n"; $memory_string = 'Allowed memory size of'; - $error_array = array( + $error_array = [ 'type' => 42, 'message' => 'Some random dummy string to take up memory', 'file' => 'Another random string, which would be a filename this time', 'line' => 314, - ); + ]; - register_shutdown_function( function () use ( + register_shutdown_function( + function () use ( $json_path, $composer_backup, &$revert, @@ -1180,19 +1219,22 @@ private function register_revert_shutdown_function( $json_path, $composer_backup $memory_msg, $memory_string, $error_array - ) { - if ( $revert ) { - if ( false === file_put_contents( $json_path, - $composer_backup ) ) { - fwrite( STDERR, $revert_fail_msg ); - } else { - fwrite( STDERR, $revert_msg ); + ) { + if ( $revert ) { + if ( false === file_put_contents( + $json_path, + $composer_backup + ) ) { + fwrite( STDERR, $revert_fail_msg ); + } else { + fwrite( STDERR, $revert_msg ); + } + } + $error_array = error_get_last(); + if ( false !== strpos( $error_array['message'], $memory_string ) ) { + fwrite( STDERR, $memory_msg ); } } - $error_array = error_get_last(); - if ( false !== strpos( $error_array['message'], $memory_string ) ) { - fwrite( STDERR, $memory_msg ); - } - } ); + ); } } diff --git a/tests/test-composer-json.php b/tests/test-composer-json.php index fba350e75..17997027d 100644 --- a/tests/test-composer-json.php +++ b/tests/test-composer-json.php @@ -8,10 +8,10 @@ class ComposerJsonTest extends PHPUnit_Framework_TestCase { - var $logger = null; - var $prev_logger = null; - var $prev_capture_exit = null; - var $temp_dir = null; + private $logger = null; + private $prev_logger = null; + private $prev_capture_exit = null; + private $temp_dir = null; public function setUp() { parent::setUp(); @@ -21,7 +21,7 @@ public function setUp() { $class_wp_cli_logger->setAccessible( true ); $this->prev_logger = $class_wp_cli_logger->getValue(); - $this->logger = new \WP_CLI\Loggers\Execution; + $this->logger = new \WP_CLI\Loggers\Execution(); WP_CLI::set_logger( $this->logger ); // Enable exit exception. @@ -48,11 +48,11 @@ public function tearDown() { parent::tearDown(); } - function test_create_default_composer_json() { + public function test_create_default_composer_json() { $create_default_composer_json = new \ReflectionMethod( 'Package_Command', 'create_default_composer_json' ); $create_default_composer_json->setAccessible( true ); - $package = new Package_Command; + $package = new Package_Command(); // Fail with bad directory. $exception = null; @@ -68,22 +68,22 @@ function test_create_default_composer_json() { // Succeed. $expected = $this->temp_dir . 'packages/composer.json'; - $actual = $create_default_composer_json->invoke( $package, $expected ); + $actual = $create_default_composer_json->invoke( $package, $expected ); $this->assertSame( $expected, $this->mac_safe_path( $actual ) ); $this->assertTrue( false !== strpos( file_get_contents( $actual ), 'wp-cli/wp-cli' ) ); unlink( $actual ); rmdir( dirname( $actual ) ); } - function test_get_composer_json_path() { - $env_test = getenv( 'WP_CLI_TEST_PACKAGE_GET_COMPOSER_JSON_PATH' ); - $env_home = getenv( 'HOME' ); + public function test_get_composer_json_path() { + $env_test = getenv( 'WP_CLI_TEST_PACKAGE_GET_COMPOSER_JSON_PATH' ); + $env_home = getenv( 'HOME' ); $env_wp_cli_packages_dir = getenv( 'WP_CLI_PACKAGES_DIR' ); $get_composer_json_path = new \ReflectionMethod( 'Package_Command', 'get_composer_json_path' ); $get_composer_json_path->setAccessible( true ); - $package = new Package_Command; + $package = new Package_Command(); putenv( 'WP_CLI_TEST_PACKAGE_GET_COMPOSER_JSON_PATH=1' ); putenv( 'HOME=' . $this->temp_dir ); @@ -91,7 +91,7 @@ function test_get_composer_json_path() { // Create in HOME. putenv( 'WP_CLI_PACKAGES_DIR' ); $expected = $this->temp_dir . '.wp-cli/packages/composer.json'; - $actual = $get_composer_json_path->invoke( $package ); + $actual = $get_composer_json_path->invoke( $package ); $this->assertSame( $expected, $this->mac_safe_path( $actual ) ); $this->assertTrue( false !== strpos( file_get_contents( $actual ), 'wp-cli/wp-cli' ) ); unlink( $actual ); @@ -101,7 +101,7 @@ function test_get_composer_json_path() { // Create in WP_CLI_PACKAGES_DIR. putenv( 'WP_CLI_PACKAGES_DIR=' . $this->temp_dir . 'packages' ); $expected = $this->temp_dir . 'packages/composer.json'; - $actual = $get_composer_json_path->invoke( $package ); + $actual = $get_composer_json_path->invoke( $package ); $this->assertSame( $expected, $this->mac_safe_path( $actual ) ); $this->assertTrue( false !== strpos( file_get_contents( $actual ), 'wp-cli/wp-cli' ) ); unlink( $actual ); @@ -123,8 +123,8 @@ function test_get_composer_json_path() { putenv( false === $env_wp_cli_packages_dir ? 'WP_CLI_PACKAGES_DIR' : "WP_CLI_PACKAGES_DIR=$env_wp_cli_packages_dir" ); } - function test_get_composer_json_path_backup_decoded() { - $env_test = getenv( 'WP_CLI_TEST_PACKAGE_GET_COMPOSER_JSON_PATH' ); + public function test_get_composer_json_path_backup_decoded() { + $env_test = getenv( 'WP_CLI_TEST_PACKAGE_GET_COMPOSER_JSON_PATH' ); $env_wp_cli_packages_dir = getenv( 'WP_CLI_PACKAGES_DIR' ); putenv( 'WP_CLI_TEST_PACKAGE_GET_COMPOSER_JSON_PATH=1' ); @@ -133,7 +133,7 @@ function test_get_composer_json_path_backup_decoded() { $get_composer_json_path_backup_decoded = new \ReflectionMethod( 'Package_Command', 'get_composer_json_path_backup_decoded' ); $get_composer_json_path_backup_decoded->setAccessible( true ); - $package = new Package_Command; + $package = new Package_Command(); // Fail with bad json. $expected = $this->temp_dir . 'packages/composer.json'; @@ -153,7 +153,7 @@ function test_get_composer_json_path_backup_decoded() { rmdir( dirname( $expected ) ); // Succeed with newly created. - $expected = $this->temp_dir . 'packages/composer.json'; + $expected = $this->temp_dir . 'packages/composer.json'; list( $actual, $content, $decoded ) = $get_composer_json_path_backup_decoded->invoke( $package ); $this->assertSame( $expected, $this->mac_safe_path( $actual ) ); $this->assertTrue( false !== strpos( file_get_contents( $actual ), 'wp-cli/wp-cli' ) ); From 18f5bd4e20c2bc959cc3bdde3d2a4bd2411ddd4f Mon Sep 17 00:00:00 2001 From: Thrijith Thankachan Date: Tue, 23 Apr 2019 20:34:17 +0530 Subject: [PATCH 3/3] PHPCS: Address feedbacks for package command --- src/Package_Command.php | 156 +++++++++++++++++++---------------- tests/test-composer-json.php | 8 +- 2 files changed, 88 insertions(+), 76 deletions(-) diff --git a/src/Package_Command.php b/src/Package_Command.php index 5ca78bfe0..6a391eb92 100644 --- a/src/Package_Command.php +++ b/src/Package_Command.php @@ -1,28 +1,29 @@ 'WP-CLI', + 'email' => 'noreply@wpcli.org', + ]; + + /** + * Default repository data used while creating default WP-CLI packages composer.json. + * @var array + */ + private $composer_type_package = [ + 'type' => 'composer', + 'url' => self::PACKAGE_INDEX_URL, + ]; + /** * Browses WP-CLI packages available for installation. * @@ -310,38 +330,39 @@ public function install( $args, $assoc_args ) { $json_manipulator->addLink( 'require', $package_name, $version, false /*sortPackages*/, true /*caseInsensitive*/ ); $json_manipulator->addConfigSetting( 'secure-http', true ); + $package_args = []; if ( $git_package ) { WP_CLI::log( sprintf( 'Registering %s as a VCS repository...', $git_package ) ); + $package_args = [ + 'type' => 'vcs', + 'url' => $git_package, + ]; $json_manipulator->addSubNode( 'repositories', $package_name, - [ - 'type' => 'vcs', - 'url' => $git_package, - ], + $package_args, true /*caseInsensitive*/ ); } elseif ( $dir_package ) { WP_CLI::log( sprintf( 'Registering %s as a path repository...', $dir_package ) ); + $package_args = [ + 'type' => 'path', + 'url' => $dir_package, + ]; $json_manipulator->addSubNode( 'repositories', $package_name, - [ - 'type' => 'path', - 'url' => $dir_package, - ], + $package_args, true /*caseInsensitive*/ ); } // If the composer file does not contain the current package index repository, refresh the repository definition. if ( empty( $composer_backup_decoded['repositories']['wp-cli']['url'] ) || self::PACKAGE_INDEX_URL !== $composer_backup_decoded['repositories']['wp-cli']['url'] ) { WP_CLI::log( 'Updating package index repository url...' ); + $package_args = $this->composer_type_package; $json_manipulator->addRepository( 'wp-cli', - [ - 'type' => 'composer', - 'url' => self::PACKAGE_INDEX_URL, - ] + $package_args ); } @@ -349,7 +370,7 @@ public function install( $args, $assoc_args ) { $composer = $this->get_composer(); // Set up the EventSubscriber - $event_subscriber = new \WP_CLI\PackageManagerEventSubscriber(); + $event_subscriber = new PackageManagerEventSubscriber(); $composer->getEventDispatcher()->addSubscriber( $event_subscriber ); // Set up the installer $install = Installer::create( new ComposerIO(), $composer ); @@ -482,7 +503,7 @@ public function update() { $composer = $this->get_composer(); // Set up the EventSubscriber - $event_subscriber = new \WP_CLI\PackageManagerEventSubscriber(); + $event_subscriber = new PackageManagerEventSubscriber(); $composer->getEventDispatcher()->addSubscriber( $event_subscriber ); // Set up the installer @@ -598,8 +619,9 @@ private function get_composer() { // Best to just pretend we're installing a package from ~/.wp-cli or similar chdir( pathinfo( $composer_path, PATHINFO_DIRNAME ) ); - // Prevent DateTime error/warning when no timezone set - // phpcs:ignore WordPress.WP.TimezoneChange.timezone_change_date_default_timezone_set, WordPress.PHP.NoSilencedErrors.Discouraged -- The package is loaded before WordPress load, For environments that don't have set time in php.ini. + // Prevent DateTime error/warning when no timezone set. + // Note: The package is loaded before WordPress load, For environments that don't have set time in php.ini. + // phpcs:ignore WordPress.WP.TimezoneChange.timezone_change_date_default_timezone_set,WordPress.PHP.NoSilencedErrors.Discouraged date_default_timezone_set( @date_default_timezone_get() ); $composer = Factory::create( new NullIO(), $composer_path ); @@ -641,15 +663,14 @@ private function package_index() { static $package_index; if ( ! $package_index ) { - $config = new Config(); - $config->merge( - [ - 'config' => [ - 'secure-http' => true, - 'home' => dirname( $this->get_composer_json_path() ), - ], - ] - ); + $config_args = [ + 'config' => [ + 'secure-http' => true, + 'home' => dirname( $this->get_composer_json_path() ), + ], + ]; + $config = new Config(); + $config->merge( $config_args ); $config->setConfigSource( new JsonConfigSource( $this->get_composer_json() ) ); try { @@ -728,8 +749,8 @@ private function show_packages( $context, $packages, $assoc_args ) { $list = array_map( function( $package ) { - $package['version'] = implode( ', ', $package['version'] ); - return $package; + $package['version'] = implode( ', ', $package['version'] ); + return $package; }, $list ); @@ -933,23 +954,15 @@ private function create_default_composer_json( $composer_path ) { $json_file = new JsonFile( $composer_path ); - $author = (object) [ - 'name' => 'WP-CLI', - 'email' => 'noreply@wpcli.org', - ]; - $repositories = (object) [ - 'wp-cli' => (object) [ - 'type' => 'composer', - 'url' => self::PACKAGE_INDEX_URL, - ], + 'wp-cli' => (object) $this->composer_type_package, ]; $options = [ 'name' => 'wp-cli/wp-cli', 'description' => 'Installed community packages used by WP-CLI', 'version' => self::get_wp_cli_version_composer(), - 'authors' => [ $author ], + 'authors' => [ (object) $this->author_data ], 'homepage' => self::PACKAGE_INDEX_URL, 'require' => new stdClass(), 'require-dev' => new stdClass(), @@ -1139,14 +1152,14 @@ private function guess_version_constraint_from_tag( $tag ) { private function set_composer_auth_env_var() { $changed = false; $composer_auth = getenv( 'COMPOSER_AUTH' ); - if ( ! $composer_auth ) { + if ( false !== $composer_auth ) { $composer_auth = json_decode( $composer_auth, true /*assoc*/ ); } - if ( ! $composer_auth || ! is_array( $composer_auth ) ) { + if ( empty( $composer_auth ) || ! is_array( $composer_auth ) ) { $composer_auth = []; } $github_token = getenv( 'GITHUB_TOKEN' ); - if ( ! isset( $composer_auth['github-oauth'] ) && $github_token ) { + if ( ! isset( $composer_auth['github-oauth'] ) && is_string( $github_token ) ) { $composer_auth['github-oauth'] = [ 'github.com' => $github_token ]; $changed = true; } @@ -1211,26 +1224,23 @@ private function register_revert_shutdown_function( $json_path, $composer_backup register_shutdown_function( function () use ( - $json_path, - $composer_backup, - &$revert, - $revert_msg, - $revert_fail_msg, - $memory_msg, - $memory_string, - $error_array + $json_path, + $composer_backup, + &$revert, + $revert_msg, + $revert_fail_msg, + $memory_msg, + $memory_string, + $error_array ) { if ( $revert ) { - if ( false === file_put_contents( - $json_path, - $composer_backup - ) ) { - fwrite( STDERR, $revert_fail_msg ); + if ( false === file_put_contents( $json_path, $composer_backup ) ) { + fwrite( STDERR, $revert_fail_msg ); } else { fwrite( STDERR, $revert_msg ); } } - $error_array = error_get_last(); + $error_array = error_get_last(); if ( false !== strpos( $error_array['message'], $memory_string ) ) { fwrite( STDERR, $memory_msg ); } diff --git a/tests/test-composer-json.php b/tests/test-composer-json.php index 17997027d..23d8ea7db 100644 --- a/tests/test-composer-json.php +++ b/tests/test-composer-json.php @@ -1,6 +1,8 @@ setAccessible( true ); $this->prev_logger = $class_wp_cli_logger->getValue(); - $this->logger = new \WP_CLI\Loggers\Execution(); + $this->logger = new Execution(); WP_CLI::set_logger( $this->logger ); // Enable exit exception. @@ -58,7 +60,7 @@ public function test_create_default_composer_json() { $exception = null; try { $actual = $create_default_composer_json->invoke( $package, '' ); - } catch ( \WP_CLI\ExitException $ex ) { + } catch ( ExitException $ex ) { $exception = $ex; } $this->assertTrue( null !== $exception ); @@ -142,7 +144,7 @@ public function test_get_composer_json_path_backup_decoded() { $exception = null; try { $actual = $get_composer_json_path_backup_decoded->invoke( $package ); - } catch ( \WP_CLI\ExitException $ex ) { + } catch ( ExitException $ex ) { $exception = $ex; } $this->assertTrue( null !== $exception );