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

Implement Enqueued_Scripts_Scope_Check #485

Merged
merged 3 commits into from
Jul 2, 2024
Merged
Show file tree
Hide file tree
Changes from all 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
276 changes: 276 additions & 0 deletions includes/Checker/Checks/Enqueued_Scripts_Scope_Check.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,276 @@
<?php
/**
* Class WordPress\Plugin_Check\Checker\Checks\Enqueued_Scripts_Scope_Check
*
* @package plugin-check
*/

namespace WordPress\Plugin_Check\Checker\Checks;

use Exception;
use WordPress\Plugin_Check\Checker\Check_Categories;
use WordPress\Plugin_Check\Checker\Check_Result;
use WordPress\Plugin_Check\Checker\Preparations\Demo_Posts_Creation_Preparation;
use WordPress\Plugin_Check\Checker\With_Shared_Preparations;
use WordPress\Plugin_Check\Traits\Amend_Check_Result;
use WordPress\Plugin_Check\Traits\Stable_Check;
use WordPress\Plugin_Check\Traits\URL_Aware;

/**
* Check if a script is present in all URLs.
*
* @since 1.0.2
*/
class Enqueued_Scripts_Scope_Check extends Abstract_Runtime_Check implements With_Shared_Preparations {

use Amend_Check_Result;
use Stable_Check;
use URL_Aware;

/**
* List of viewable post types.
*
* @since 1.0.2
* @var array
*/
private $viewable_post_types;

/**
* List of plugin scripts.
*
* @since 1.0.2
* @var array
*/
private $plugin_scripts = array();

/**
* Plugin script counter.
*
* @since 1.0.2
* @var array
*/
private $plugin_script_count = array();

/**
* Gets the categories for the check.
*
* Every check must have at least one category.
*
* @since 1.0.2
*
* @return array The categories for the check.
*/
public function get_categories() {
return array( Check_Categories::CATEGORY_PERFORMANCE );

Check warning on line 64 in includes/Checker/Checks/Enqueued_Scripts_Scope_Check.php

View check run for this annotation

Codecov / codecov/patch

includes/Checker/Checks/Enqueued_Scripts_Scope_Check.php#L63-L64

Added lines #L63 - L64 were not covered by tests
}

/**
* Runs this preparation step for the environment and returns a cleanup function.
*
* @since 1.0.2
*
* @return callable Cleanup function to revert any changes made here.
*
* @throws Exception Thrown when preparation fails.
*/
public function prepare() {
$orig_scripts = isset( $GLOBALS['wp_scripts'] ) ? $GLOBALS['wp_scripts'] : null;
Copy link
Member

Choose a reason for hiding this comment

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

This can be simplified to:

Suggested change
$orig_scripts = isset( $GLOBALS['wp_scripts'] ) ? $GLOBALS['wp_scripts'] : null;
$orig_scripts = $GLOBALS['wp_scripts'] ?? null;

But not required.


// Backup the original values for the global state.
$this->backup_globals();

return function () use ( $orig_scripts ) {
if ( is_null( $orig_scripts ) ) {
unset( $GLOBALS['wp_scripts'] );
} else {
$GLOBALS['wp_scripts'] = $orig_scripts;

Check warning on line 86 in includes/Checker/Checks/Enqueued_Scripts_Scope_Check.php

View check run for this annotation

Codecov / codecov/patch

includes/Checker/Checks/Enqueued_Scripts_Scope_Check.php#L86

Added line #L86 was not covered by tests
}

$this->restore_globals();
};
}

/**
* Returns an array of shared preparations for the check.
*
* @since 1.0.2
*
* @return array Returns a map of $class_name => $constructor_args pairs. If the class does not
* need any constructor arguments, it would just be an empty array.
*/
public function get_shared_preparations() {
$demo_posts = array_map(
static function ( $post_type ) {
return array(
'post_title' => "Demo {$post_type} post",
Copy link
Member

Choose a reason for hiding this comment

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

Advised by PhpStorm:

Suggested change
'post_title' => "Demo {$post_type} post",
'post_title' => "Demo $post_type post",

'post_content' => 'Test content',
'post_type' => $post_type,
'post_status' => 'publish',
);
},
$this->get_viewable_post_types()
);

return array(
Demo_Posts_Creation_Preparation::class => array( $demo_posts ),
);
}

/**
* Runs the check on the plugin and amends results.
*
* @since 1.0.2
*
* @param Check_Result $result The check results to amend and the plugin context.
*/
public function run( Check_Result $result ) {
$urls = $this->get_urls();
$this->run_for_urls(
$urls,
function () use ( $result ) {
$this->check_url( $result );
}
);

if ( ! empty( $this->plugin_scripts ) ) {
$url_count = count( $urls );
foreach ( $this->plugin_scripts as $plugin_script ) {
if ( isset( $plugin_script['count'] ) && ( $url_count === $plugin_script['count'] ) ) {
$this->add_result_warning_for_file(
$result,
__( 'This script is being loaded in all contexts.', 'plugin-check' ),
Copy link
Member

Choose a reason for hiding this comment

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

Why is it a problem if a script is loaded in all contexts?

Copy link
Member Author

Choose a reason for hiding this comment

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

It's not always a problem, but often times this is done in error and not actually on purpose. This can also be often observed in the wp-admin, when a plugin loads JS on every page instead of only its own settings page.

This check is aimed to detect the former, but of course it can't be bulletproof. Hence this is a warning and not an error.

'EnqueuedScriptsScope',
$plugin_script['path']
);
}
}
}
}

/**
* Gets the list of URLs to run this check for.
*
* @since 1.0.2
*
* @return array List of URL strings (either full URLs or paths).
*
* @throws Exception Thrown when a post type URL cannot be retrieved.
*/
protected function get_urls() {
$urls = array( home_url( '/' ), get_search_link(), get_author_posts_url( 1 ) );
foreach ( $this->get_viewable_post_types() as $post_type ) {
$args = array(
'posts_per_page' => 1,
'post_type' => $post_type,
'post_status' => array( 'publish', 'inherit' ),
'ignore_sticky_posts' => true,
'no_found_rows' => true,
'lazy_load_term_meta' => false,
'update_post_meta_cache' => false,
'update_post_term_cache' => false,
);

$the_query = new \WP_Query( $args );
Copy link
Member

Choose a reason for hiding this comment

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

If use WP_Query is added to the top:

Suggested change
$the_query = new \WP_Query( $args );
$the_query = new WP_Query( $args );


if ( $the_query->have_posts() ) {
while ( $the_query->have_posts() ) {
$the_query->the_post();

$urls[] = get_permalink();
$post = get_post();
$taxonomy_names = get_post_taxonomies( $post );
Comment on lines +177 to +181
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
$the_query->the_post();
$urls[] = get_permalink();
$post = get_post();
$taxonomy_names = get_post_taxonomies( $post );
$the_query->next_post();
$urls[] = get_permalink( $the_query->post );
$taxonomy_names = get_post_taxonomies( $the_query->post );

foreach ( $taxonomy_names as $taxonomy_name ) {
if ( ! is_taxonomy_viewable( $taxonomy_name ) ) {
continue;

Check warning on line 184 in includes/Checker/Checks/Enqueued_Scripts_Scope_Check.php

View check run for this annotation

Codecov / codecov/patch

includes/Checker/Checks/Enqueued_Scripts_Scope_Check.php#L184

Added line #L184 was not covered by tests
}

$terms = get_the_terms( $post, $taxonomy_name );
if ( ! is_array( $terms ) ) {
continue;
}
foreach ( $terms as $term ) {
$term_link = get_term_link( $term );
if ( ! is_wp_error( $term_link ) ) {
$urls[] = $term_link;
}
}
}
}
} else {
throw new Exception(
sprintf(

Check warning on line 201 in includes/Checker/Checks/Enqueued_Scripts_Scope_Check.php

View check run for this annotation

Codecov / codecov/patch

includes/Checker/Checks/Enqueued_Scripts_Scope_Check.php#L200-L201

Added lines #L200 - L201 were not covered by tests
/* translators: %s: The Post Type name. */
__( 'Unable to retrieve post URL for post type: %s', 'plugin-check' ),
$post_type
)
);

Check warning on line 206 in includes/Checker/Checks/Enqueued_Scripts_Scope_Check.php

View check run for this annotation

Codecov / codecov/patch

includes/Checker/Checks/Enqueued_Scripts_Scope_Check.php#L203-L206

Added lines #L203 - L206 were not covered by tests
}

/* Restore original Post Data */
wp_reset_postdata();
Comment on lines +208 to +210
Copy link
Member

Choose a reason for hiding this comment

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

Now unneeded if we don't call the_post() above.

Suggested change
/* Restore original Post Data */
wp_reset_postdata();

}

return $urls;
}

/**
* Amends the given result by running the check for the given URL.
*
* @since 1.0.2
*
* @param Check_Result $result The check result to amend, including the plugin context to check.
*
* @throws Exception Thrown when the check fails with a critical error (unrelated to any errors detected as part of
* the check).
*/
protected function check_url( Check_Result $result ) {
// Reset the wp_scripts instance.
unset( $GLOBALS['wp_scripts'] );

/*
* Run the 'wp_enqueue_script' action, wrapped in an output buffer in case of any callbacks printing scripts
* directly. This is discouraged, but some plugins or themes are still doing it.
*/
$wp_scripts = wp_scripts();
ob_start();
wp_enqueue_scripts();
$wp_scripts->do_head_items();
$wp_scripts->do_footer_items();
ob_get_clean();

foreach ( $wp_scripts->done as $handle ) {
$script = $wp_scripts->registered[ $handle ];

if ( ! $script->src || strpos( $script->src, $result->plugin()->url() ) !== 0 ) {
continue;

Check warning on line 245 in includes/Checker/Checks/Enqueued_Scripts_Scope_Check.php

View check run for this annotation

Codecov / codecov/patch

includes/Checker/Checks/Enqueued_Scripts_Scope_Check.php#L245

Added line #L245 was not covered by tests
}

$script_path = str_replace( $result->plugin()->url(), $result->plugin()->path(), $script->src );

if ( ! isset( $this->plugin_script_count[ $handle ] ) ) {
$this->plugin_script_count[ $handle ] = 0;
}
$this->plugin_script_count[ $handle ] += 1;

$this->plugin_scripts[ $handle ] = array(
'path' => $script_path,
'count' => $this->plugin_script_count[ $handle ],
);
}
}

/**
* Returns an array of viewable post types.
*
* @since 1.0.2
*
* @return array Array of viewable post type slugs.
*/
private function get_viewable_post_types() {
if ( ! is_array( $this->viewable_post_types ) ) {
$this->viewable_post_types = array_filter( get_post_types(), 'is_post_type_viewable' );
}

return $this->viewable_post_types;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
console.log( 'Hello World' );
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
<?php
/**
* File contains errors for the enqueued scripts scope check.
*/

add_action(
'wp_enqueue_scripts',
function() {
wp_enqueue_script(
'script',
plugin_dir_url( __FILE__ ) . 'script.js',
array(),
'1.0'
);
if ( is_home() ) {
wp_enqueue_script(
'home-script',
plugin_dir_url( __FILE__ ) . 'home.js',
array(),
'1.0'
);
}
}
);
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
console.log( 'Hello World' );
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
<?php
/**
* The file does not contain any errors or notices for the enqueued scripts scope check.
*/

add_action(
'wp_enqueue_scripts',
function() {
if ( is_home() ) {
wp_enqueue_script(
'home-script',
plugin_dir_url( __FILE__ ) . 'script.js',
array(),
'1.0'
);
}
}
);
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
console.log( 'Hello World' );
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
<?php
/**
* Tests for the Enqueued_Scripts_Scope_Check class.
*
* @package plugin-check
*/

namespace phpunit\tests\Checker\Checks;

use WordPress\Plugin_Check\Checker\Checks\Enqueued_Scripts_Scope_Check;
use WordPress\Plugin_Check\Test_Utils\TestCase\Runtime_Check_UnitTestCase;

class Enqueued_Scripts_Scope_Check_Tests extends Runtime_Check_UnitTestCase {

public function test_run_with_errors() {
// Load the test plugin.
require UNIT_TESTS_PLUGIN_DIR . 'test-plugin-enqueued-scripts-scope-check-with-error/load.php';

$check = new Enqueued_Scripts_Scope_Check();
$context = $this->get_context( WP_PLUGIN_CHECK_MAIN_FILE );
$results = $this->run_check( $check, $context );

$errors = $results->get_errors();
$warnings = $results->get_warnings();

$this->assertEmpty( $errors );
$this->assertNotEmpty( $warnings );

$this->assertEquals( 0, $results->get_error_count() );
$this->assertEquals( 1, $results->get_warning_count() );
}

public function test_run_without_errors() {
// Load the test plugin.
require UNIT_TESTS_PLUGIN_DIR . 'test-plugin-enqueued-scripts-scope-check-without-error/load.php';

$check = new Enqueued_Scripts_Scope_Check();
$context = $this->get_context( WP_PLUGIN_CHECK_MAIN_FILE );
$results = $this->run_check( $check, $context );

$errors = $results->get_errors();
$warnings = $results->get_warnings();

$this->assertEmpty( $errors );
$this->assertEmpty( $warnings );

$this->assertEquals( 0, $results->get_error_count() );
$this->assertEquals( 0, $results->get_warning_count() );
}
}
Loading