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

Add wp-cli integration #458

Merged
Merged
Show file tree
Hide file tree
Changes from 14 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -50,5 +50,23 @@ add_filter( 'tc_skip_development_directories', '__return_true' );

To add more directories to the paths where other files are excluded then add them to the array through the `tc_common_dev_directories` filter.

### Usage with wp-cli

To use with [wp-cli](https://wp-cli.org/), ensure the theme check plugin is active and `wp-cli` is installed. The `theme-check` subcommand is added to `wp-cli` and can be used as follows:

`wp theme-check run [<theme>] [--format=<format>]`

matiasbenedetto marked this conversation as resolved.
Show resolved Hide resolved
#### Options
| Option | Accepts | Required | Default
| -- | -- | -- | -- |
| `theme` | The slug of the theme to check | No | Current theme slug
| `format` | `cli` or `json` | No | `cli`

#### Examples
`wp theme-check run`
`wp theme-check run twentytwentyfour`
`wp theme-check run --format=json`
`wp theme-check run twentytwentyfour --format=json`

## Contributors
Otto42, pross, The theme review team
4 changes: 4 additions & 0 deletions theme-check.php
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,10 @@ class ThemeCheckMain {
function __construct() {
add_action( 'admin_init', array( $this, 'tc_i18n' ) );
add_action( 'admin_menu', array( $this, 'themecheck_add_page' ) );

if ( defined( 'WP_CLI' ) && WP_CLI ) {
include 'wp-cli/class-theme-check-cli.php';
}
}

function tc_i18n() {
Expand Down
160 changes: 160 additions & 0 deletions wp-cli/class-theme-check-cli.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,160 @@
<?php

if ( defined( 'WP_CLI' ) && WP_CLI ) {
$parent_dir = dirname( __DIR__ );
require_once $parent_dir . '/checkbase.php';
require_once $parent_dir . '/main.php';
WP_CLI::add_command( 'theme-check', 'Theme_Check_Command' );
}

class Theme_Check_Command extends WP_CLI_Command {
/**
* Run a theme check on the specified theme or the current theme.
*
* ## OPTIONS
* [<theme>]
* : The slug of the theme to check. If not provided, checks the current theme.
*
* [--format=<format>]
matiasbenedetto marked this conversation as resolved.
Show resolved Hide resolved
* : Render output in a particular format.
* ---
* default: cli
* options:
* - cli
* - json
* ---
*
* ## EXAMPLES
matiasbenedetto marked this conversation as resolved.
Show resolved Hide resolved
* # Check the current active theme
* wp theme-check run
*
* # Check a specific theme
* wp theme-check run twentytwentyfour
*
* # Check the current active theme and output results as JSON
* wp theme-check run --format=json
*
* # Check a specific theme and output results as JSON
* wp theme-check run twentytwentyfour --format=json
*
* @param array $args Indexed array of positional arguments.
* @param array $assoc_args Associative array of options.
* @return void
*/
public function run( $args, $assoc_args ) {
$format = \WP_CLI\Utils\get_flag_value( $assoc_args, 'format', 'cli' );

// Get the current theme
$current_theme = wp_get_theme();
$current_theme_slug = $current_theme->get_stylesheet();

// Use the provided theme slug if available, otherwise use the current theme
$check_theme_slug = ! empty( $args[0] ) ? $args[0] : $current_theme_slug;

// Get the theme
$theme = wp_get_theme( $check_theme_slug );

if ( ! $theme->exists() ) {
if ( $format === 'json' ) {
$json_output = array(
'completed' => false,
'result' => "Error: Theme '{$check_theme_slug}' not found.",
'messages' => array(),
);
WP_CLI::line( wp_json_encode( $json_output, JSON_PRETTY_PRINT ) );
return;
}
WP_CLI::error( "Theme '{$check_theme_slug}' not found." );
}

// Run the checks.
$success = run_themechecks_against_theme( $theme, $check_theme_slug );

if ( $format === 'json' ) {
if ( ! $success ) {
$json_output = array(
'completed' => false,
'result' => "Error: Theme check failed for {$check_theme_slug}.",
'messages' => array(),
);
WP_CLI::line( wp_json_encode( $json_output, JSON_PRETTY_PRINT ) );
return;
}
$this->display_themechecks_as_json( $check_theme_slug );
} else {
if ( ! $success ) {
WP_CLI::error( "Theme check failed for {$check_theme_slug}." );
}
$this->display_themechecks_in_cli( $check_theme_slug );
Copy link
Contributor

Choose a reason for hiding this comment

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

Execution stops if a WP_CLI::error is thrown, so the errors are not being displayed. Could this line be moved above the $success check?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'm not sure, i'm seeing this logic here:

// If the check process failed (if the checks were not able to run properly) raise an error. There's nothing else to do.
if ( ! $success ) {
WP_CLI::error( "Theme check failed for {$check_theme_slug}." );
}
// If the checking process run print in the console the checks. This will also print the things marked as an error in the theme.
$this->display_themechecks_in_cli( $check_theme_slug );

Could you explain why you think we should move it?

Copy link
Contributor

Choose a reason for hiding this comment

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

// If the check process failed (if the checks were not able to run properly) raise an error. There's nothing else to do.

I don't think this is correct. $success is the result of run_themechecks_against_theme which returns false if the theme fails a check.

Could you explain why you think we should move it?

Using twentytwentyfive, here is the output currently:

Error: Theme check failed for twentytwentyfive.

Moving call to display_themechecks_in_cli above the conditional, the result is:

Success: Theme check completed for twentytwentyfive.

REQUIRED: phpcs.xml.dist XML file found. This file must not be in the production version of the theme.

WARNING: assets/images/image-from-rawpixel-id-2222755.webp is 997.6 KB in size. Large file sizes have a negative impact on website performance and loading time. Compress images before using them.

WARNING: assets/images/image-from-rawpixel-id-2224378.webp is 879.6 KB in size. Large file sizes have a negative impact on website performance and loading time. Compress images before using them.

WARNING: assets/images/location.webp is 778.1 KB in size. Large file sizes have a negative impact on website performance and loading time. Compress images before using them.

WARNING: assets/images/poster-image-background.webp is 574.7 KB in size. Large file sizes have a negative impact on website performance and loading time. Compress images before using them.

INFO Themes that use the tag accessibility-ready will need to undergo an accessibility review. See https://make.wordpress.org/themes/handbook/review/accessibility/

WARNING: Your theme appears to be in the wrong directory for the theme name. The directory name must match the slug of the theme. This theme's correct slug and text-domain is twenty-twenty-five. (If this is a child theme, you can ignore this error.)

INFO: Only one text-domain is being used in this theme. Make sure it matches the theme's slug correctly so that the theme will be compatible with WordPress.org language packs. The domain found is twentytwentyfive.

RECOMMENDED: Requires PHP is recommended to have major and minor versions only (e.g. 7.4). Patch version is not needed (e.g. 7.4.1).

Error: Theme check failed for twentytwentyfive.

I think the latter is more useful because it reports what checks failed (the first one marked REQUIRED).

Also I am not sure we should throw an error here if $success isn't true, because the theme check successfully ran, but the result was it failed the checks.

Copy link
Member

Choose a reason for hiding this comment

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

This command's focus should be on pass/fail check for a theme, rather than run is successfully completed or not. And exit code should be returned based on that I believe.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think I could implement your feedback, please see: #458 (comment)

}
}

/**
* Process theme check messages.
*
* @return array Processed messages.
*/
private function process_themecheck_messages() {
global $themechecks;
$messages = array();

foreach ( $themechecks as $check ) {
if ( $check instanceof themecheck ) {
$error = $check->getError();
$error = (array) $error;
if ( ! empty( $error ) ) {
$messages = array_merge( $messages, $error );
}
}
}

$processed_messages = array_map(
function( $message ) {
return html_entity_decode( wp_strip_all_tags( $message ), ENT_QUOTES, 'UTF-8' );
},
$messages
);

return $processed_messages;
}

/**
* Display the theme checks in the CLI.
*
* @param string $slug The slug of the theme to display the checks for.
* @return void
*/
private function display_themechecks_in_cli( $slug ) {
$processed_messages = $this->process_themecheck_messages();

WP_CLI::success( "Theme check completed for {$slug}." );

if ( empty( $processed_messages ) ) {
WP_CLI::line( 'No issues found.' );
return;
}

foreach ( $processed_messages as $message ) {
WP_CLI::line( '' );
WP_CLI::line( $message );
}
}

/**
* Display the theme checks in JSON format.
*
* @param string $slug The slug of the theme to display the checks for.
* @return void
*/
private function display_themechecks_as_json( $slug ) {
$processed_messages = $this->process_themecheck_messages();

$json_output = array(
'completed' => true,
'result' => "Theme check completed for {$slug}.",
'messages' => $processed_messages,
);

WP_CLI::line( wp_json_encode( $json_output, JSON_PRETTY_PRINT ) );
}
}
Loading