Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve check-url command output #3929

Open
wants to merge 5 commits into
base: update/amp-validate-requests
Choose a base branch
from

Conversation

schlessera
Copy link
Collaborator

Summary

Adds the following:

  • multiple output formats that can be selected via --format=<format>: json, yaml, table, count
  • tabular format improves output for human consumption
  • adds filtering via --<field>=<value>
  • allows selection of fields to display via --fields=<fields>
  • can return a single column via --field=<field>
  • uses exit codes for scripting purposes

Depends on #3898

Checklist

  • My pull request is addressing an open issue (please create one otherwise).
  • My code is tested and passes existing tests.
  • My code follows the Engineering Guidelines (updates are often made to the guidelines, check it out periodically).

@googlebot googlebot added the cla: yes Signed the Google CLA label Dec 14, 2019
@schlessera
Copy link
Collaborator Author

Noticed a bug in the WP-CLI formatting code while working on this: wp-cli/wp-cli#5330

@schlessera
Copy link
Collaborator Author

⚠️ Rebasing the PR to resolve the merge conflict now.

Adds the following:
- multiple output formats that can be selected via `--format=<format>`: `json`, `yaml`, `table`, `count`
- tabular format improves output for human consumption
- adds filtering via `--<field>=<value>`
- allows selection of fields to display via `--fields=<fields>`
- can return a single column via `--field=<field>`
- uses exit codes for scripting purposes
@schlessera
Copy link
Collaborator Author

We still don't have functional tests for the commands, so I don't think it is worth the effort to write tests for the command itself right now. We should look into #3076 at some point to improve on this.

Copy link
Contributor

@kienstra kienstra left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good overall

Hi @schlessera,
The new formats look great, this is a really nice improvement. In my local, there was a PHP notice with --format=table. There are steps to reproduce below.

--format=json

format-json

--format=count

format-count

--format=yaml

format-yaml

--format=table

 wp amp validation check-url /with-script/ --format=table
PHP Notice:  Array to string conversion in /srv/www/loc/public_html/wp-content/plugins/amp/vendor/wp-cli/wp-cli/php/WP_CLI/Formatter.php on line 218
PHP Stack trace:
PHP   1. {main}() /usr/local/bin/wp:0
PHP   2. include() /usr/local/bin/wp:4
PHP   3. include() phar:///usr/local/bin/wp/php/boot-phar.php:11
PHP   4. WP_CLI\bootstrap() phar:///usr/local/bin/wp/vendor/wp-cli/wp-cli/php/wp-cli.php:27
PHP   5. WP_CLI\Bootstrap\LaunchRunner->process($state = class WP_CLI\Bootstrap\BootstrapState { private $state = array () }) phar:///usr/local/bin/wp/vendor/wp-cli/wp-cli/php/bootstrap.php:74
PHP   6. WP_CLI\Runner->start() phar:///usr/local/bin/wp/vendor/wp-cli/wp-cli/php/WP_CLI/Bootstrap/LaunchRunner.php:23
PHP   7. WP_CLI\Runner->run_command_and_exit($help_exit_warning = *uninitialized*) phar:///usr/local/bin/wp/vendor/wp-cli/wp-cli/php/WP_CLI/Runner.php:1160
PHP   8. WP_CLI\Runner->run_command($args = array (0 => 'amp', 1 => 'validation', 2 => 'check-url', 3 => '/with-script/'), $assoc_args = array ('format' => 'table'), $options = *uninitialized*) phar:///usr/local/bin/wp/vendor/wp-cli/wp-cli/php/WP_CLI/Runner.php:394
PHP   9. WP_CLI\Dispatcher\Subcommand->invoke($args = array (0 => '/with-script/'), $assoc_args = array ('format' => 'table'), $extra_args = array ()) phar:///usr/local/bin/wp/vendor/wp-cli/wp-cli/php/WP_CLI/Runner.php:371
PHP  10. call_user_func:{phar:///usr/local/bin/wp/vendor/wp-cli/wp-cli/php/WP_CLI/Dispatcher/Subcommand.php:451}(class Closure { public $static = array ('callable' => array (0 => 'AMP_CLI_Validation_Command', 1 => 'check_url')); public $parameter = array ('$args' => '<required>', '$assoc_args' => '<required>') }, array (0 => '/with-script/'), array ('format' => 'table')) phar:///usr/local/bin/wp/vendor/wp-cli/wp-cli/php/WP_CLI/Dispatcher/Subcommand.php:451
PHP  11. WP_CLI\Dispatcher\CommandFactory::WP_CLI\Dispatcher\{closure:phar:///usr/local/bin/wp/vendor/wp-cli/wp-cli/php/WP_CLI/Dispatcher/CommandFactory.php:95-102}($args = array (0 => '/with-script/'), $assoc_args = array ('format' => 'table')) phar:///usr/local/bin/wp/vendor/wp-cli/wp-cli/php/WP_CLI/Dispatcher/Subcommand.php:451
PHP  12. call_user_func:{phar:///usr/local/bin/wp/vendor/wp-cli/wp-cli/php/WP_CLI/Dispatcher/CommandFactory.php:98}(array (0 => class AMP_CLI_Validation_Command { public $wp_cli_progress = NULL; public $total_errors = 0; public $unaccepted_errors = 0; public $number_crawled = 0; public $force_crawl_urls = FALSE; public $include_conditionals = array (); public $limit_type_validate_count = NULL; public $validity_by_type = array () }, 1 => 'check_url'), array (0 => '/with-script/'), array ('format' => 'table')) phar:///usr/local/bin/wp/vendor/wp-cli/wp-cli/php/WP_CLI/Dispatcher/CommandFactory.php:98
PHP  13. AMP_CLI_Validation_Command->check_url($args = array (0 => '/with-script/'), $assoc_args = array ('format' => 'table')) phar:///usr/local/bin/wp/vendor/wp-cli/wp-cli/php/WP_CLI/Dispatcher/CommandFactory.php:98
PHP  14. WP_CLI\Formatter->display_items($items = array (0 => array ('node_name' => 'script', 'parent_name' => 'div', 'code' => '\033[31mDISALLOWED_TAG\033[0m', 'type' => 'js_error', 'node_attributes' => '[]', 'text' => 'doThis()', 'sources' => '1.  [{"hook":"the_content","filter":true,"post_id":5206,"post_type":"post","sources":[{"type":"core","name":"wp-includes","file":"class-wp-embed.php","line":59,"function":"WP_Embed::run_shortcode"},{"type":"core","name":"wp-includes","file":"class-wp-embed.php","line":404,"function":"WP_Embed::autoembed"},{"type":"core","name":"wp-includes","file":"blocks.php","line":308,"function":"do_blocks"},{"type":"core","name":"wp-includes","file":"formatting.php","line":51,"function":"wptexturize"},{"type":"core","name":"wp-includes","file":"formatting.php","line":455,"function":"wpautop"},{"type":"core","name":"wp-includes","file":"formatting.php","line":838,"function":"shortcode_unautop"},{"type":"core","name":"wp-includes","file":"post-template.php","line":1649,"function":"prepend_attachment"},{"type":"core","name":"wp-includes","file":"media.php","line":1432,"function":"wp_make_content_images_responsive"},{"type":"core","name":"wp-includes","file":"formatting.php","line":5690,"function":"wp_staticize_emoji"},{"type":"plugin","name":"amp","file":"includes/sanitizers/class-amp-core-theme-sanitizer.php","line":500,"function":"{closure}"},{"type":"core","name":"wp-includes","file":"formatting.php","line":5323,"function":"capital_P_dangit"},{"type":"core","name":"wp-includes","file":"shortcodes.php","line":177,"function":"do_shortcode"},{"type":"core","name":"wp-includes","file":"formatting.php","line":3324,"function":"convert_smilies"}]}] | 2.  [{"block_name":"core/html","post_id":5206,"block_content_index":0}]', 'sanitized' => '\033[32;1myes\033[0m')), $ascii_pre_colorized = TRUE) /srv/www/loc/public_html/wp-content/plugins/amp/includes/cli/class-amp-cli-validation-command.php:435
PHP  15. WP_CLI\Formatter->find_item_key($item = array ('node_name' => 'script', 'parent_name' => 'div', 'code' => '\033[31mDISALLOWED_TAG\033[0m', 'type' => 'js_error', 'node_attributes' => '[]', 'text' => 'doThis()', 'sources' => '1.  [{"hook":"the_content","filter":true,"post_id":5206,"post_type":"post","sources":[{"type":"core","name":"wp-includes","file":"class-wp-embed.php","line":59,"function":"WP_Embed::run_shortcode"},{"type":"core","name":"wp-includes","file":"class-wp-embed.php","line":404,"function":"WP_Embed::autoembed"},{"type":"core","name":"wp-includes","file":"blocks.php","line":308,"function":"do_blocks"},{"type":"core","name":"wp-includes","file":"formatting.php","line":51,"function":"wptexturize"},{"type":"core","name":"wp-includes","file":"formatting.php","line":455,"function":"wpautop"},{"type":"core","name":"wp-includes","file":"formatting.php","line":838,"function":"shortcode_unautop"},{"type":"core","name":"wp-includes","file":"post-template.php","line":1649,"function":"prepend_attachment"},{"type":"core","name":"wp-includes","file":"media.php","line":1432,"function":"wp_make_content_images_responsive"},{"type":"core","name":"wp-includes","file":"formatting.php","line":5690,"function":"wp_staticize_emoji"},{"type":"plugin","name":"amp","file":"includes/sanitizers/class-amp-core-theme-sanitizer.php","line":500,"function":"{closure}"},{"type":"core","name":"wp-includes","file":"formatting.php","line":5323,"function":"capital_P_dangit"},{"type":"core","name":"wp-includes","file":"shortcodes.php","line":177,"function":"do_shortcode"},{"type":"core","name":"wp-includes","file":"formatting.php","line":3324,"function":"convert_smilies"}]}] | 2.  [{"block_name":"core/html","post_id":5206,"block_content_index":0}]', 'sanitized' => '\033[32;1myes\033[0m'), $field = 'node_name') /srv/www/loc/public_html/wp-content/plugins/amp/vendor/wp-cli/wp-cli/php/WP_CLI/Formatter.php:76
PHP Notice:  Array to string conversion in /srv/www/loc/public_html/wp-content/plugins/amp/vendor/wp-cli/wp-cli/php/WP_CLI/Formatter.php on line 218
PHP Stack trace:
PHP   1. {main}() /usr/local/bin/wp:0
PHP   2. include() /usr/local/bin/wp:4
PHP   3. include() phar:///usr/local/bin/wp/php/boot-phar.php:11
PHP   4. WP_CLI\bootstrap() phar:///usr/local/bin/wp/vendor/wp-cli/wp-cli/php/wp-cli.php:27
PHP   5. WP_CLI\Bootstrap\LaunchRunner->process($state = class WP_CLI\Bootstrap\BootstrapState { private $state = array () }) phar:///usr/local/bin/wp/vendor/wp-cli/wp-cli/php/bootstrap.php:74
PHP   6. WP_CLI\Runner->start() phar:///usr/local/bin/wp/vendor/wp-cli/wp-cli/php/WP_CLI/Bootstrap/LaunchRunner.php:23
PHP   7. WP_CLI\Runner->run_command_and_exit($help_exit_warning = *uninitialized*) phar:///usr/local/bin/wp/vendor/wp-cli/wp-cli/php/WP_CLI/Runner.php:1160
PHP   8. WP_CLI\Runner->run_command($args = array (0 => 'amp', 1 => 'validation', 2 => 'check-url', 3 => '/with-script/'), $assoc_args = array ('format' => 'table'), $options = *uninitialized*) phar:///usr/local/bin/wp/vendor/wp-cli/wp-cli/php/WP_CLI/Runner.php:394
PHP   9. WP_CLI\Dispatcher\Subcommand->invoke($args = array (0 => '/with-script/'), $assoc_args = array ('format' => 'table'), $extra_args = array ()) phar:///usr/local/bin/wp/vendor/wp-cli/wp-cli/php/WP_CLI/Runner.php:371
PHP  10. call_user_func:{phar:///usr/local/bin/wp/vendor/wp-cli/wp-cli/php/WP_CLI/Dispatcher/Subcommand.php:451}(class Closure { public $static = array ('callable' => array (0 => 'AMP_CLI_Validation_Command', 1 => 'check_url')); public $parameter = array ('$args' => '<required>', '$assoc_args' => '<required>') }, array (0 => '/with-script/'), array ('format' => 'table')) phar:///usr/local/bin/wp/vendor/wp-cli/wp-cli/php/WP_CLI/Dispatcher/Subcommand.php:451
PHP  11. WP_CLI\Dispatcher\CommandFactory::WP_CLI\Dispatcher\{closure:phar:///usr/local/bin/wp/vendor/wp-cli/wp-cli/php/WP_CLI/Dispatcher/CommandFactory.php:95-102}($args = array (0 => '/with-script/'), $assoc_args = array ('format' => 'table')) phar:///usr/local/bin/wp/vendor/wp-cli/wp-cli/php/WP_CLI/Dispatcher/Subcommand.php:451
PHP  12. call_user_func:{phar:///usr/local/bin/wp/vendor/wp-cli/wp-cli/php/WP_CLI/Dispatcher/CommandFactory.php:98}(array (0 => class AMP_CLI_Validation_Command { public $wp_cli_progress = NULL; public $total_errors = 0; public $unaccepted_errors = 0; public $number_crawled = 0; public $force_crawl_urls = FALSE; public $include_conditionals = array (); public $limit_type_validate_count = NULL; public $validity_by_type = array () }, 1 => 'check_url'), array (0 => '/with-script/'), array ('format' => 'table')) phar:///usr/local/bin/wp/vendor/wp-cli/wp-cli/php/WP_CLI/Dispatcher/CommandFactory.php:98
PHP  13. AMP_CLI_Validation_Command->check_url($args = array (0 => '/with-script/'), $assoc_args = array ('format' => 'table')) phar:///usr/local/bin/wp/vendor/wp-cli/wp-cli/php/WP_CLI/Dispatcher/CommandFactory.php:98
PHP  14. WP_CLI\Formatter->display_items($items = array (0 => array ('node_name' => 'script', 'parent_name' => 'div', 'code' => '\033[31mDISALLOWED_TAG\033[0m', 'type' => 'js_error', 'node_attributes' => '[]', 'text' => 'doThis()', 'sources' => '1.  [{"hook":"the_content","filter":true,"post_id":5206,"post_type":"post","sources":[{"type":"core","name":"wp-includes","file":"class-wp-embed.php","line":59,"function":"WP_Embed::run_shortcode"},{"type":"core","name":"wp-includes","file":"class-wp-embed.php","line":404,"function":"WP_Embed::autoembed"},{"type":"core","name":"wp-includes","file":"blocks.php","line":308,"function":"do_blocks"},{"type":"core","name":"wp-includes","file":"formatting.php","line":51,"function":"wptexturize"},{"type":"core","name":"wp-includes","file":"formatting.php","line":455,"function":"wpautop"},{"type":"core","name":"wp-includes","file":"formatting.php","line":838,"function":"shortcode_unautop"},{"type":"core","name":"wp-includes","file":"post-template.php","line":1649,"function":"prepend_attachment"},{"type":"core","name":"wp-includes","file":"media.php","line":1432,"function":"wp_make_content_images_responsive"},{"type":"core","name":"wp-includes","file":"formatting.php","line":5690,"function":"wp_staticize_emoji"},{"type":"plugin","name":"amp","file":"includes/sanitizers/class-amp-core-theme-sanitizer.php","line":500,"function":"{closure}"},{"type":"core","name":"wp-includes","file":"formatting.php","line":5323,"function":"capital_P_dangit"},{"type":"core","name":"wp-includes","file":"shortcodes.php","line":177,"function":"do_shortcode"},{"type":"core","name":"wp-includes","file":"formatting.php","line":3324,"function":"convert_smilies"}]}] | 2.  [{"block_name":"core/html","post_id":5206,"block_content_index":0}]', 'sanitized' => '\033[32;1myes\033[0m')), $ascii_pre_colorized = TRUE) /srv/www/loc/public_html/wp-content/plugins/amp/includes/cli/class-amp-cli-validation-command.php:435
PHP  15. WP_CLI\Formatter->find_item_key($item = array ('node_name' => 'script', 'parent_name' => 'div', 'code' => '\033[31mDISALLOWED_TAG\033[0m', 'type' => 'js_error', 'node_attributes' => '[]', 'text' => 'doThis()', 'sources' => '1.  [{"hook":"the_content","filter":true,"post_id":5206,"post_type":"post","sources":[{"type":"core","name":"wp-includes","file":"class-wp-embed.php","line":59,"function":"WP_Embed::run_shortcode"},{"type":"core","name":"wp-includes","file":"class-wp-embed.php","line":404,"function":"WP_Embed::autoembed"},{"type":"core","name":"wp-includes","file":"blocks.php","line":308,"function":"do_blocks"},{"type":"core","name":"wp-includes","file":"formatting.php","line":51,"function":"wptexturize"},{"type":"core","name":"wp-includes","file":"formatting.php","line":455,"function":"wpautop"},{"type":"core","name":"wp-includes","file":"formatting.php","line":838,"function":"shortcode_unautop"},{"type":"core","name":"wp-includes","file":"post-template.php","line":1649,"function":"prepend_attachment"},{"type":"core","name":"wp-includes","file":"media.php","line":1432,"function":"wp_make_content_images_responsive"},{"type":"core","name":"wp-includes","file":"formatting.php","line":5690,"function":"wp_staticize_emoji"},{"type":"plugin","name":"amp","file":"includes/sanitizers/class-amp-core-theme-sanitizer.php","line":500,"function":"{closure}"},{"type":"core","name":"wp-includes","file":"formatting.php","line":5323,"function":"capital_P_dangit"},{"type":"core","name":"wp-includes","file":"shortcodes.php","line":177,"function":"do_shortcode"},{"type":"core","name":"wp-includes","file":"formatting.php","line":3324,"function":"convert_smilies"}]}] | 2.  [{"block_name":"core/html","post_id":5206,"block_content_index":0}]', 'sanitized' => '\033[32;1myes\033[0m'), $field = 'parent_name') /srv/www/loc/public_html/wp-content/plugins/amp/vendor/wp-cli/wp-cli/php/WP_CLI/Formatter.php:76
PHP Notice:  Array to string conversion in /srv/www/loc/public_html/wp-content/plugins/amp/vendor/wp-cli/wp-cli/php/WP_CLI/Formatter.php on line 218
PHP Stack trace:
PHP   1. {main}() /usr/local/bin/wp:0
PHP   2. include() /usr/local/bin/wp:4
PHP   3. include() phar:///usr/local/bin/wp/php/boot-phar.php:11
PHP   4. WP_CLI\bootstrap() phar:///usr/local/bin/wp/vendor/wp-cli/wp-cli/php/wp-cli.php:27
PHP   5. WP_CLI\Bootstrap\LaunchRunner->process($state = class WP_CLI\Bootstrap\BootstrapState { private $state = array () }) phar:///usr/local/bin/wp/vendor/wp-cli/wp-cli/php/bootstrap.php:74
PHP   6. WP_CLI\Runner->start() phar:///usr/local/bin/wp/vendor/wp-cli/wp-cli/php/WP_CLI/Bootstrap/LaunchRunner.php:23
PHP   7. WP_CLI\Runner->run_command_and_exit($help_exit_warning = *uninitialized*) phar:///usr/local/bin/wp/vendor/wp-cli/wp-cli/php/WP_CLI/Runner.php:1160
PHP   8. WP_CLI\Runner->run_command($args = array (0 => 'amp', 1 => 'validation', 2 => 'check-url', 3 => '/with-script/'), $assoc_args = array ('format' => 'table'), $options = *uninitialized*) phar:///usr/local/bin/wp/vendor/wp-cli/wp-cli/php/WP_CLI/Runner.php:394
PHP   9. WP_CLI\Dispatcher\Subcommand->invoke($args = array (0 => '/with-script/'), $assoc_args = array ('format' => 'table'), $extra_args = array ()) phar:///usr/local/bin/wp/vendor/wp-cli/wp-cli/php/WP_CLI/Runner.php:371
PHP  10. call_user_func:{phar:///usr/local/bin/wp/vendor/wp-cli/wp-cli/php/WP_CLI/Dispatcher/Subcommand.php:451}(class Closure { public $static = array ('callable' => array (0 => 'AMP_CLI_Validation_Command', 1 => 'check_url')); public $parameter = array ('$args' => '<required>', '$assoc_args' => '<required>') }, array (0 => '/with-script/'), array ('format' => 'table')) phar:///usr/local/bin/wp/vendor/wp-cli/wp-cli/php/WP_CLI/Dispatcher/Subcommand.php:451
PHP  11. WP_CLI\Dispatcher\CommandFactory::WP_CLI\Dispatcher\{closure:phar:///usr/local/bin/wp/vendor/wp-cli/wp-cli/php/WP_CLI/Dispatcher/CommandFactory.php:95-102}($args = array (0 => '/with-script/'), $assoc_args = array ('format' => 'table')) phar:///usr/local/bin/wp/vendor/wp-cli/wp-cli/php/WP_CLI/Dispatcher/Subcommand.php:451
PHP  12. call_user_func:{phar:///usr/local/bin/wp/vendor/wp-cli/wp-cli/php/WP_CLI/Dispatcher/CommandFactory.php:98}(array (0 => class AMP_CLI_Validation_Command { public $wp_cli_progress = NULL; public $total_errors = 0; public $unaccepted_errors = 0; public $number_crawled = 0; public $force_crawl_urls = FALSE; public $include_conditionals = array (); public $limit_type_validate_count = NULL; public $validity_by_type = array () }, 1 => 'check_url'), array (0 => '/with-script/'), array ('format' => 'table')) phar:///usr/local/bin/wp/vendor/wp-cli/wp-cli/php/WP_CLI/Dispatcher/CommandFactory.php:98
PHP  13. AMP_CLI_Validation_Command->check_url($args = array (0 => '/with-script/'), $assoc_args = array ('format' => 'table')) phar:///usr/local/bin/wp/vendor/wp-cli/wp-cli/php/WP_CLI/Dispatcher/CommandFactory.php:98
PHP  14. WP_CLI\Formatter->display_items($items = array (0 => array ('node_name' => 'script', 'parent_name' => 'div', 'code' => '\033[31mDISALLOWED_TAG\033[0m', 'type' => 'js_error', 'node_attributes' => '[]', 'text' => 'doThis()', 'sources' => '1.  [{"hook":"the_content","filter":true,"post_id":5206,"post_type":"post","sources":[{"type":"core","name":"wp-includes","file":"class-wp-embed.php","line":59,"function":"WP_Embed::run_shortcode"},{"type":"core","name":"wp-includes","file":"class-wp-embed.php","line":404,"function":"WP_Embed::autoembed"},{"type":"core","name":"wp-includes","file":"blocks.php","line":308,"function":"do_blocks"},{"type":"core","name":"wp-includes","file":"formatting.php","line":51,"function":"wptexturize"},{"type":"core","name":"wp-includes","file":"formatting.php","line":455,"function":"wpautop"},{"type":"core","name":"wp-includes","file":"formatting.php","line":838,"function":"shortcode_unautop"},{"type":"core","name":"wp-includes","file":"post-template.php","line":1649,"function":"prepend_attachment"},{"type":"core","name":"wp-includes","file":"media.php","line":1432,"function":"wp_make_content_images_responsive"},{"type":"core","name":"wp-includes","file":"formatting.php","line":5690,"function":"wp_staticize_emoji"},{"type":"plugin","name":"amp","file":"includes/sanitizers/class-amp-core-theme-sanitizer.php","line":500,"function":"{closure}"},{"type":"core","name":"wp-includes","file":"formatting.php","line":5323,"function":"capital_P_dangit"},{"type":"core","name":"wp-includes","file":"shortcodes.php","line":177,"function":"do_shortcode"},{"type":"core","name":"wp-includes","file":"formatting.php","line":3324,"function":"convert_smilies"}]}] | 2.  [{"block_name":"core/html","post_id":5206,"block_content_index":0}]', 'sanitized' => '\033[32;1myes\033[0m')), $ascii_pre_colorized = TRUE) /srv/www/loc/public_html/wp-content/plugins/amp/includes/cli/class-amp-cli-validation-command.php:435
PHP  15. WP_CLI\Formatter->find_item_key($item = array ('node_name' => 'script', 'parent_name' => 'div', 'code' => '\033[31mDISALLOWED_TAG\033[0m', 'type' => 'js_error', 'node_attributes' => '[]', 'text' => 'doThis()', 'sources' => '1.  [{"hook":"the_content","filter":true,"post_id":5206,"post_type":"post","sources":[{"type":"core","name":"wp-includes","file":"class-wp-embed.php","line":59,"function":"WP_Embed::run_shortcode"},{"type":"core","name":"wp-includes","file":"class-wp-embed.php","line":404,"function":"WP_Embed::autoembed"},{"type":"core","name":"wp-includes","file":"blocks.php","line":308,"function":"do_blocks"},{"type":"core","name":"wp-includes","file":"formatting.php","line":51,"function":"wptexturize"},{"type":"core","name":"wp-includes","file":"formatting.php","line":455,"function":"wpautop"},{"type":"core","name":"wp-includes","file":"formatting.php","line":838,"function":"shortcode_unautop"},{"type":"core","name":"wp-includes","file":"post-template.php","line":1649,"function":"prepend_attachment"},{"type":"core","name":"wp-includes","file":"media.php","line":1432,"function":"wp_make_content_images_responsive"},{"type":"core","name":"wp-includes","file":"formatting.php","line":5690,"function":"wp_staticize_emoji"},{"type":"plugin","name":"amp","file":"includes/sanitizers/class-amp-core-theme-sanitizer.php","line":500,"function":"{closure}"},{"type":"core","name":"wp-includes","file":"formatting.php","line":5323,"function":"capital_P_dangit"},{"type":"core","name":"wp-includes","file":"shortcodes.php","line":177,"function":"do_shortcode"},{"type":"core","name":"wp-includes","file":"formatting.php","line":3324,"function":"convert_smilies"}]}] | 2.  [{"block_name":"core/html","post_id":5206,"block_content_index":0}]', 'sanitized' => '\033[32;1myes\033[0m'), $field = 'code') /srv/www/loc/public_html/wp-content/plugins/amp/vendor/wp-cli/wp-cli/php/WP_CLI/Formatter.php:76
PHP Notice:  Array to string conversion in /srv/www/loc/public_html/wp-content/plugins/amp/vendor/wp-cli/wp-cli/php/WP_CLI/Formatter.php on line 218
PHP Stack trace:
PHP   1. {main}() /usr/local/bin/wp:0
PHP   2. include() /usr/local/bin/wp:4
PHP   3. include() phar:///usr/local/bin/wp/php/boot-phar.php:11
PHP   4. WP_CLI\bootstrap() phar:///usr/local/bin/wp/vendor/wp-cli/wp-cli/php/wp-cli.php:27
PHP   5. WP_CLI\Bootstrap\LaunchRunner->process($state = class WP_CLI\Bootstrap\BootstrapState { private $state = array () }) phar:///usr/local/bin/wp/vendor/wp-cli/wp-cli/php/bootstrap.php:74
PHP   6. WP_CLI\Runner->start() phar:///usr/local/bin/wp/vendor/wp-cli/wp-cli/php/WP_CLI/Bootstrap/LaunchRunner.php:23
PHP   7. WP_CLI\Runner->run_command_and_exit($help_exit_warning = *uninitialized*) phar:///usr/local/bin/wp/vendor/wp-cli/wp-cli/php/WP_CLI/Runner.php:1160
PHP   8. WP_CLI\Runner->run_command($args = array (0 => 'amp', 1 => 'validation', 2 => 'check-url', 3 => '/with-script/'), $assoc_args = array ('format' => 'table'), $options = *uninitialized*) phar:///usr/local/bin/wp/vendor/wp-cli/wp-cli/php/WP_CLI/Runner.php:394
PHP   9. WP_CLI\Dispatcher\Subcommand->invoke($args = array (0 => '/with-script/'), $assoc_args = array ('format' => 'table'), $extra_args = array ()) phar:///usr/local/bin/wp/vendor/wp-cli/wp-cli/php/WP_CLI/Runner.php:371
PHP  10. call_user_func:{phar:///usr/local/bin/wp/vendor/wp-cli/wp-cli/php/WP_CLI/Dispatcher/Subcommand.php:451}(class Closure { public $static = array ('callable' => array (0 => 'AMP_CLI_Validation_Command', 1 => 'check_url')); public $parameter = array ('$args' => '<required>', '$assoc_args' => '<required>') }, array (0 => '/with-script/'), array ('format' => 'table')) phar:///usr/local/bin/wp/vendor/wp-cli/wp-cli/php/WP_CLI/Dispatcher/Subcommand.php:451
PHP  11. WP_CLI\Dispatcher\CommandFactory::WP_CLI\Dispatcher\{closure:phar:///usr/local/bin/wp/vendor/wp-cli/wp-cli/php/WP_CLI/Dispatcher/CommandFactory.php:95-102}($args = array (0 => '/with-script/'), $assoc_args = array ('format' => 'table')) phar:///usr/local/bin/wp/vendor/wp-cli/wp-cli/php/WP_CLI/Dispatcher/Subcommand.php:451
PHP  12. call_user_func:{phar:///usr/local/bin/wp/vendor/wp-cli/wp-cli/php/WP_CLI/Dispatcher/CommandFactory.php:98}(array (0 => class AMP_CLI_Validation_Command { public $wp_cli_progress = NULL; public $total_errors = 0; public $unaccepted_errors = 0; public $number_crawled = 0; public $force_crawl_urls = FALSE; public $include_conditionals = array (); public $limit_type_validate_count = NULL; public $validity_by_type = array () }, 1 => 'check_url'), array (0 => '/with-script/'), array ('format' => 'table')) phar:///usr/local/bin/wp/vendor/wp-cli/wp-cli/php/WP_CLI/Dispatcher/CommandFactory.php:98
PHP  13. AMP_CLI_Validation_Command->check_url($args = array (0 => '/with-script/'), $assoc_args = array ('format' => 'table')) phar:///usr/local/bin/wp/vendor/wp-cli/wp-cli/php/WP_CLI/Dispatcher/CommandFactory.php:98
PHP  14. WP_CLI\Formatter->display_items($items = array (0 => array ('node_name' => 'script', 'parent_name' => 'div', 'code' => '\033[31mDISALLOWED_TAG\033[0m', 'type' => 'js_error', 'node_attributes' => '[]', 'text' => 'doThis()', 'sources' => '1.  [{"hook":"the_content","filter":true,"post_id":5206,"post_type":"post","sources":[{"type":"core","name":"wp-includes","file":"class-wp-embed.php","line":59,"function":"WP_Embed::run_shortcode"},{"type":"core","name":"wp-includes","file":"class-wp-embed.php","line":404,"function":"WP_Embed::autoembed"},{"type":"core","name":"wp-includes","file":"blocks.php","line":308,"function":"do_blocks"},{"type":"core","name":"wp-includes","file":"formatting.php","line":51,"function":"wptexturize"},{"type":"core","name":"wp-includes","file":"formatting.php","line":455,"function":"wpautop"},{"type":"core","name":"wp-includes","file":"formatting.php","line":838,"function":"shortcode_unautop"},{"type":"core","name":"wp-includes","file":"post-template.php","line":1649,"function":"prepend_attachment"},{"type":"core","name":"wp-includes","file":"media.php","line":1432,"function":"wp_make_content_images_responsive"},{"type":"core","name":"wp-includes","file":"formatting.php","line":5690,"function":"wp_staticize_emoji"},{"type":"plugin","name":"amp","file":"includes/sanitizers/class-amp-core-theme-sanitizer.php","line":500,"function":"{closure}"},{"type":"core","name":"wp-includes","file":"formatting.php","line":5323,"function":"capital_P_dangit"},{"type":"core","name":"wp-includes","file":"shortcodes.php","line":177,"function":"do_shortcode"},{"type":"core","name":"wp-includes","file":"formatting.php","line":3324,"function":"convert_smilies"}]}] | 2.  [{"block_name":"core/html","post_id":5206,"block_content_index":0}]', 'sanitized' => '\033[32;1myes\033[0m')), $ascii_pre_colorized = TRUE) /srv/www/loc/public_html/wp-content/plugins/amp/includes/cli/class-amp-cli-validation-command.php:435
PHP  15. WP_CLI\Formatter->find_item_key($item = array ('node_name' => 'script', 'parent_name' => 'div', 'code' => '\033[31mDISALLOWED_TAG\033[0m', 'type' => 'js_error', 'node_attributes' => '[]', 'text' => 'doThis()', 'sources' => '1.  [{"hook":"the_content","filter":true,"post_id":5206,"post_type":"post","sources":[{"type":"core","name":"wp-includes","file":"class-wp-embed.php","line":59,"function":"WP_Embed::run_shortcode"},{"type":"core","name":"wp-includes","file":"class-wp-embed.php","line":404,"function":"WP_Embed::autoembed"},{"type":"core","name":"wp-includes","file":"blocks.php","line":308,"function":"do_blocks"},{"type":"core","name":"wp-includes","file":"formatting.php","line":51,"function":"wptexturize"},{"type":"core","name":"wp-includes","file":"formatting.php","line":455,"function":"wpautop"},{"type":"core","name":"wp-includes","file":"formatting.php","line":838,"function":"shortcode_unautop"},{"type":"core","name":"wp-includes","file":"post-template.php","line":1649,"function":"prepend_attachment"},{"type":"core","name":"wp-includes","file":"media.php","line":1432,"function":"wp_make_content_images_responsive"},{"type":"core","name":"wp-includes","file":"formatting.php","line":5690,"function":"wp_staticize_emoji"},{"type":"plugin","name":"amp","file":"includes/sanitizers/class-amp-core-theme-sanitizer.php","line":500,"function":"{closure}"},{"type":"core","name":"wp-includes","file":"formatting.php","line":5323,"function":"capital_P_dangit"},{"type":"core","name":"wp-includes","file":"shortcodes.php","line":177,"function":"do_shortcode"},{"type":"core","name":"wp-includes","file":"formatting.php","line":3324,"function":"convert_smilies"}]}] | 2.  [{"block_name":"core/html","post_id":5206,"block_content_index":0}]', 'sanitized' => '\033[32;1myes\033[0m'), $field = 'type') /srv/www/loc/public_html/wp-content/plugins/amp/vendor/wp-cli/wp-cli/php/WP_CLI/Formatter.php:76
PHP Notice:  Array to string conversion in /srv/www/loc/public_html/wp-content/plugins/amp/vendor/wp-cli/wp-cli/php/WP_CLI/Formatter.php on line 218
...

Steps to reproduce

  1. Create a new post, with a title like 'With Script'
  2. Add a Custom HTML block
  3. Add a script to it, like <script>doThis()</script>
  4. Publish the post, and look at the permalink, like /with-script
  5. Run wp amp validation check-url /with-script/ --format=table
  6. Expected: there is no PHP notice (I think)
  7. Actual: there's a PHP notice, as above. The notice still appeared with Xdebug off, but it was much less verbose.

@kienstra
Copy link
Contributor

kienstra commented Dec 17, 2019

Still, that PHP notice could just be from my local. The AMP plugin is the only active plugin.

php --version shows PHP 7.2.25-1+ubuntu18.04.1+deb.sury.org+1 (cli) (built: Nov 28 2019 07:42:26) ( NTS )

And wp --version shows WP-CLI 2.4.0.

$errors = $this->flatten_validation_errors( $response['results'] );

if ( 'json' === $assoc_args['format'] ) {
WP_CLI::line( wp_json_encode( $errors, JSON_PRETTY_PRINT | JSON_UNESCAPED_SLASHES ) );
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If json I think it should output the entire $response here, as there is more data in the $response than just the resultant $errors.

Suggested change
WP_CLI::line( wp_json_encode( $errors, JSON_PRETTY_PRINT | JSON_UNESCAPED_SLASHES ) );
WP_CLI::line( wp_json_encode( $response, JSON_PRETTY_PRINT | JSON_UNESCAPED_SLASHES ) );

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As such, this if ( 'json' === $assoc_args['format'] ) condition should be moved above the if ( empty( $response['results'] ) ) check.

if ( $response instanceof WP_Error ) {
WP_CLI::error( $response );
}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's go ahead and store the results in the amp_validated_url post type by adding something like this:

$validated_url_post = AMP_Validated_URL_Post_Type::store_validation_errors(
    $response['results'],
    $response['url'],
    wp_array_slice_assoc( $response, [ 'object' ] )
);
if ( $validated_url_post instanceof WP_Error ) {
    WP_CLI::error( $validated_url_post );
}

The URL to the results in the admin can then be another format that can be returned:

if ( 'url' === $assoc_args['format'] ) {
    WP_CLI::line( get_permalink( $validated_url_post ) );
    exit( 0 );
}

And in general, at the very end of the method here prior to exit there can be printing to STDERR the admin URL, which would allow the JSON results to be returned in addition to the validated URL screen admin URL.

@westonruter
Copy link
Member

Additional data that would need to be included here is what is obtained from the stylesheets as introduced on the AMP validated URL screen in #4026.

@westonruter
Copy link
Member

@schlessera Please create an issue for this and connect this PR with it.

@schlessera schlessera added the WS:Core Work stream for Plugin core label Aug 18, 2020
@CLAassistant
Copy link

CLAassistant commented Oct 8, 2020

CLA assistant check
All committers have signed the CLA.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes Signed the Google CLA Validation WP-CLI WS:Core Work stream for Plugin core
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants