Skip to content

Commit

Permalink
[Boost] Clean up linting issues in the minify feature (#29352)
Browse files Browse the repository at this point in the history 
* Fix linting issues in minify module

* changelog

---------

Co-authored-by: Mark George <[email protected]>
  • Loading branch information
@thingalon
thingalon and Mark George authored Mar 8, 2023
1 parent 12fbcf8 commit 0712aed
Show file tree
Hide file tree
Showing 8 changed files with 150 additions and 94 deletions.
2 changes: 0 additions & 2 deletions projects/plugins/boost/.phpcsignore
View file

This file was deleted.

25 changes: 15 additions & 10 deletions projects/plugins/boost/app/features/optimizations/minify/Concatenate_CSS.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,16 @@

use WP_Styles;

// Disable complaints about enqueuing stylesheets, as this class alters the way enqueuing them works.
// phpcs:disable WordPress.WP.EnqueuedResources.NonEnqueuedStylesheet

class Concatenate_CSS extends WP_Styles {
private $dependency_path_mapping;
private $old_styles;

public $allow_gzip_compression;

function __construct( $styles ) {
public function __construct( $styles ) {
if ( empty( $styles ) || ! ( $styles instanceof WP_Styles ) ) {
$this->old_styles = new WP_Styles();
} else {
Expand All @@ -32,7 +35,7 @@ function __construct( $styles ) {
);
}

function do_items( $handles = false, $group = false ) {
public function do_items( $handles = false, $group = false ) {
$handles = false === $handles ? $this->queue : (array) $handles;
$stylesheets = array();
$siteurl = apply_filters( 'page_optimize_site_url', $this->base_url );
Expand All @@ -53,11 +56,11 @@ function do_items( $handles = false, $group = false ) {
// http://core.trac.wordpress.org/attachment/ticket/16827/colors-hacked-fixed.diff
// http://core.trac.wordpress.org/ticket/20729
$css_url = $obj->src;
if ( 'colors' == $obj->handle && true === $css_url ) {
if ( 'colors' === $obj->handle && true === $css_url ) {
$css_url = wp_style_loader_src( $css_url, $obj->handle );
}

$css_url_parsed = parse_url( $obj->src );
$css_url_parsed = wp_parse_url( $obj->src );
$extra = $obj->extra;

// Don't concat by default
Expand Down Expand Up @@ -141,9 +144,9 @@ function do_items( $handles = false, $group = false ) {
unset( $this->to_do[ $key ] );
}

foreach ( $stylesheets as $idx => $stylesheets_group ) {
foreach ( $stylesheets as $_idx => $stylesheets_group ) {
foreach ( $stylesheets_group as $media => $css ) {
if ( 'noconcat' == $media ) {
if ( 'noconcat' === $media ) {
foreach ( $css as $handle ) {
if ( $this->do_item( $handle, $group ) ) {
$this->done[] = $handle;
Expand All @@ -165,6 +168,7 @@ function do_items( $handles = false, $group = false ) {
$path_str = "$path_str?m=$mtime";

if ( $this->allow_gzip_compression ) {
// phpcs:ignore WordPress.PHP.DiscouragedPHPFunctions.obfuscation_base64_encode
$path_64 = base64_encode( gzcompress( $path_str ) );
if ( strlen( $path_str ) > ( strlen( $path_64 ) + 1 ) ) {
$path_str = '-' . $path_64;
Expand All @@ -187,6 +191,7 @@ function do_items( $handles = false, $group = false ) {
$style_tag = apply_filters( 'page_optimize_style_loader_tag', $style_tag, $handles, $href, $media );
$style_tag = apply_filters( 'style_loader_tag', $style_tag, $handles, $href, $media );

// phpcs:ignore WordPress.Security.EscapeOutput.OutputNotEscaped
echo $style_tag . "\n";

array_map( array( $this, 'print_inline_style' ), array_keys( $css ) );
Expand All @@ -196,19 +201,19 @@ function do_items( $handles = false, $group = false ) {
return $this->done;
}

function __isset( $key ) {
public function __isset( $key ) {
return isset( $this->old_styles->$key );
}

function __unset( $key ) {
public function __unset( $key ) {
unset( $this->old_styles->$key );
}

function &__get( $key ) {
public function &__get( $key ) {
return $this->old_styles->$key;
}

function __set( $key, $value ) {
public function __set( $key, $value ) {
$this->old_styles->$key = $value;
}
}
31 changes: 20 additions & 11 deletions projects/plugins/boost/app/features/optimizations/minify/Concatenate_JS.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,16 @@

use WP_Scripts;

// Disable complaints about enqueuing scripts, as this class alters the way enqueuing them works.
// phpcs:disable WordPress.WP.EnqueuedResources.NonEnqueuedScript

class Concatenate_JS extends WP_Scripts {
private $dependency_path_mapping;
private $old_scripts;

public $allow_gzip_compression;

function __construct( $scripts ) {
public function __construct( $scripts ) {
if ( empty( $scripts ) || ! ( $scripts instanceof WP_Scripts ) ) {
$this->old_scripts = new WP_Scripts();
} else {
Expand Down Expand Up @@ -52,7 +55,9 @@ protected function has_inline_content( $handle ) {
return false;
}

function do_items( $handles = false, $group = false ) {
public function do_items( $handles = false, $group = false ) {
global $wp_filesystem;

$handles = false === $handles ? $this->queue : (array) $handles;
$javascripts = array();
$siteurl = apply_filters( 'page_optimize_site_url', $this->base_url );
Expand All @@ -62,7 +67,7 @@ function do_items( $handles = false, $group = false ) {
$using_strict = false;
foreach ( $this->to_do as $key => $handle ) {
$script_is_strict = false;
if ( in_array( $handle, $this->done ) || ! isset( $this->registered[ $handle ] ) ) {
if ( in_array( $handle, $this->done, true ) || ! isset( $this->registered[ $handle ] ) ) {
continue;
}

Expand All @@ -85,7 +90,7 @@ function do_items( $handles = false, $group = false ) {

$obj = $this->registered[ $handle ];
$js_url = $obj->src;
$js_url_parsed = parse_url( $js_url );
$js_url_parsed = wp_parse_url( $js_url );

// Don't concat by default
$do_concat = false;
Expand Down Expand Up @@ -131,7 +136,7 @@ function do_items( $handles = false, $group = false ) {
}
$do_concat = false;
$script_is_strict = true;
} elseif ( $do_concat && preg_match_all( '/^[\',"]use strict[\',"];/Uims', file_get_contents( $js_realpath ), $matches ) ) {
} elseif ( $do_concat && preg_match_all( '/^[\',"]use strict[\',"];/Uims', $wp_filesystem->get_contents( $js_realpath ), $matches ) ) {
// Skip third-party scripts that use Strict Mode
if ( defined( 'WP_DEBUG' ) && WP_DEBUG ) {
echo sprintf( "\n<!-- No Concat JS %s => Has Strict Mode (Third-Party) -->\n", esc_html( $handle ) );
Expand Down Expand Up @@ -196,11 +201,11 @@ function do_items( $handles = false, $group = false ) {
}

foreach ( $javascripts as $js_array ) {
if ( 'do_item' == $js_array['type'] ) {
if ( 'do_item' === $js_array['type'] ) {
if ( $this->do_item( $js_array['handle'], $group ) ) {
$this->done[] = $js_array['handle'];
}
} elseif ( 'concat' == $js_array['type'] ) {
} elseif ( 'concat' === $js_array['type'] ) {
array_map( array( $this, 'print_extra_script' ), $js_array['handles'] );

if ( isset( $js_array['paths'] ) && count( $js_array['paths'] ) > 1 ) {
Expand All @@ -218,6 +223,7 @@ function do_items( $handles = false, $group = false ) {
$path_str = "$path_str?m=$mtime";

if ( $this->allow_gzip_compression ) {
// phpcs:ignore WordPress.PHP.DiscouragedPHPFunctions.obfuscation_base64_encode
$path_64 = base64_encode( gzcompress( $path_str ) );
if ( strlen( $path_str ) > ( strlen( $path_64 ) + 1 ) ) {
$path_str = '-' . $path_64;
Expand All @@ -234,6 +240,7 @@ function do_items( $handles = false, $group = false ) {
// Print before/after scripts from wp_inline_scripts() and concatenated script tag
if ( isset( $js_array['extras']['before'] ) ) {
foreach ( $js_array['extras']['before'] as $inline_before ) {
// phpcs:ignore WordPress.Security.EscapeOutput.OutputNotEscaped
echo $inline_before;
}
}
Expand All @@ -257,11 +264,13 @@ function do_items( $handles = false, $group = false ) {
$tag = apply_filters( 'script_loader_tag', $tag, $js_array['handles'][0], $href );
}

// phpcs:ignore WordPress.Security.EscapeOutput.OutputNotEscaped
echo $tag;
}

if ( isset( $js_array['extras']['after'] ) ) {
foreach ( $js_array['extras']['after'] as $inline_after ) {
// phpcs:ignore WordPress.Security.EscapeOutput.OutputNotEscaped
echo $inline_after;
}
}
Expand All @@ -273,19 +282,19 @@ function do_items( $handles = false, $group = false ) {
return $this->done;
}

function __isset( $key ) {
public function __isset( $key ) {
return isset( $this->old_scripts->$key );
}

function __unset( $key ) {
public function __unset( $key ) {
unset( $this->old_scripts->$key );
}

function &__get( $key ) {
public function &__get( $key ) {
return $this->old_scripts->$key;
}

function __set( $key, $value ) {
public function __set( $key, $value ) {
$this->old_scripts->$key = $value;
}
}
20 changes: 9 additions & 11 deletions projects/plugins/boost/app/features/optimizations/minify/Dependency_Path_Mapping.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,6 @@

namespace Automattic\Jetpack_Boost\Features\Optimizations\Minify;

use Automattic\Jetpack_Boost\Features\Optimizations\Minify\Config;

/**
* This is a class to map script and style URLs to local filesystem paths.
* This is necessary when we are deciding what we can concatenate and when
Expand All @@ -21,7 +19,7 @@ class Dependency_Path_Mapping {
public $plugin_uri_path = null;
public $plugin_dir = null;

function __construct(
public function __construct(
// Expose URLs and DIRs for unit test
$site_url = null, // default site URL is determined dynamically
$site_dir = null,
Expand All @@ -39,33 +37,33 @@ function __construct(
}
$site_url = trailingslashit( $site_url );
$this->site_url = $site_url;
$this->site_uri_path = parse_url( $site_url, PHP_URL_PATH );
$this->site_uri_path = wp_parse_url( $site_url, PHP_URL_PATH );
$this->site_dir = trailingslashit( $site_dir );

// Only resolve content URLs if they are under the site URL
if ( $this->is_internal_uri( $content_url ) ) {
$this->content_uri_path = parse_url( trailingslashit( $content_url ), PHP_URL_PATH );
$this->content_uri_path = wp_parse_url( trailingslashit( $content_url ), PHP_URL_PATH );
$this->content_dir = trailingslashit( $content_dir );
}

// Only resolve plugin URLs if they are under the site URL
if ( $this->is_internal_uri( $plugin_url ) ) {
$this->plugin_uri_path = parse_url( trailingslashit( $plugin_url ), PHP_URL_PATH );
$this->plugin_uri_path = wp_parse_url( trailingslashit( $plugin_url ), PHP_URL_PATH );
$this->plugin_dir = trailingslashit( $plugin_dir );
}
}

/**
* Given the full URL of a script/style dependency, return its local filesystem path.
*/
function dependency_src_to_fs_path( $src ) {
public function dependency_src_to_fs_path( $src ) {
if ( ! $this->is_internal_uri( $src ) ) {
// If a URI is not internal, we can have no confidence
// we are resolving to the correct file.
return false;
}

$src_parts = parse_url( $src );
$src_parts = wp_parse_url( $src );
if ( false === $src_parts ) {
return false;
}
Expand All @@ -89,7 +87,7 @@ function dependency_src_to_fs_path( $src ) {
/**
* Given a URI path of a script/style resource, return its local filesystem path.
*/
function uri_path_to_fs_path( $uri_path ) {
public function uri_path_to_fs_path( $uri_path ) {
if ( 1 === preg_match( '#(?:^|/)\.\.?(?:/|$)#', $uri_path ) ) {
// Reject relative paths
return false;
Expand Down Expand Up @@ -118,7 +116,7 @@ function uri_path_to_fs_path( $uri_path ) {
*
* This method helps ensure we only resolve to local FS paths.
*/
function is_internal_uri( $uri ) {
public function is_internal_uri( $uri ) {
if ( jetpack_boost_page_optimize_starts_with( '/', $uri ) && ! jetpack_boost_page_optimize_starts_with( '//', $uri ) ) {
// Absolute paths are internal because they are based on the site dir (typically ABSPATH),
// and this looks like an absolute path.
Expand All @@ -135,7 +133,7 @@ function is_internal_uri( $uri ) {
*
* Does not handle relative paths.
*/
static function is_descendant_uri( $dir_path, $candidate ) {
public static function is_descendant_uri( $dir_path, $candidate ) {
// Ensure a trailing slash to avoid false matches like
// "/wp-content/resource" being judged a descendant of "/wp".
$dir_path = trailingslashit( $dir_path );
Expand Down
11 changes: 8 additions & 3 deletions projects/plugins/boost/app/features/optimizations/minify/Minify.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,16 +3,19 @@
namespace Automattic\Jetpack_Boost\Features\Optimizations\Minify;

use Automattic\Jetpack_Boost\Contracts\Feature;
use Automattic\Jetpack_Boost\Features\Optimizations\Minify\Config;

// Allow overriding WordPress globals, as that is necessary to taking over script output.
// phpcs:disable WordPress.WP.GlobalVariablesOverride.Prohibited

class Minify implements Feature {
// @todo - handle PHP constants.

public function setup() {
require 'functions-helpers.php';
require __DIR__ . '/functions-helpers.php';

// TODO: Make concat URL dir configurable
if ( isset( $_SERVER['REQUEST_URI'] ) && '/_static/' === substr( $_SERVER['REQUEST_URI'], 0, 9 ) ) {
// phpcs:ignore WordPress.Security.ValidatedSanitizedInput.InputNotSanitized
if ( isset( $_SERVER['REQUEST_URI'] ) && '/_static/' === substr( wp_unslash( $_SERVER['REQUEST_URI'] ), 0, 9 ) ) {
require_once __DIR__ . '/service.php';
exit;
}
Expand Down Expand Up @@ -46,6 +49,8 @@ public function init_concatenate() {
return;
}

jetpack_boost_init_filesystem();

if ( jetpack_boost_page_optimize_should_concat_js() || jetpack_boost_page_optimize_load_mode_js() ) {
global $wp_scripts;

Expand Down
Loading

0 comments on commit 0712aed

Please sign in to comment.