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

Janitorial: Disparate cleanup tasks #40200

Merged
merged 29 commits into from
Nov 18, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
c157034
Remove Phan suppression now that PHPDoc is correct in supported WP ve…
tbradsha Nov 14, 2024
01bb8ae
Remove WP 6.5 polyfill (see #38428)
tbradsha Nov 14, 2024
6e6eab8
Remove is_callable check for wp_trigger_error()
tbradsha Nov 14, 2024
d7458f0
setMethods → onlyMethods
tbradsha Nov 14, 2024
3240990
static → const property
tbradsha Nov 14, 2024
4f83422
Add type hinting
tbradsha Nov 14, 2024
0674faf
Throw exception instead of triggering deprecation notice
tbradsha Nov 14, 2024
3bdaac9
Remove now-unneeded CSS
tbradsha Nov 14, 2024
7842f7d
Updated comment
tbradsha Nov 14, 2024
69aed20
Add future todo
tbradsha Nov 14, 2024
565292b
Remove fallbacks
tbradsha Nov 14, 2024
1ed885c
Add changelogs
tbradsha Nov 14, 2024
460e14b
Remove composer downgrade logic
tbradsha Nov 14, 2024
17b7094
CRM requires PHP 7.4 already
tbradsha Nov 14, 2024
5fd5720
Super Cache has previously required WP 6.5
tbradsha Nov 14, 2024
f1f7025
Vaultpress has previous required WP 5.2
tbradsha Nov 14, 2024
e66c7aa
Get realpath of temp dir
tbradsha Nov 15, 2024
d143ebe
Fix changelogger tests
tbradsha Nov 15, 2024
6355fae
Add changelogs
tbradsha Nov 15, 2024
a4aeab1
Fix typo
tbradsha Nov 15, 2024
a9a6936
Adjust comment
tbradsha Nov 15, 2024
2a6feee
Simplify logic
tbradsha Nov 15, 2024
b0d8609
See if this sates tests
tbradsha Nov 15, 2024
0dd6d17
Jetpack::is_connection_ready has been around in its current state sin…
tbradsha Nov 15, 2024
a6fefc3
Remove checks for more long-present calls
tbradsha Nov 15, 2024
a792185
Clean up Phan suppressions
tbradsha Nov 15, 2024
9fbfc42
Update changelog
tbradsha Nov 15, 2024
6c32ce9
Update changelog
tbradsha Nov 15, 2024
82bfe8e
Remove old comment
tbradsha Nov 15, 2024
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
5 changes: 0 additions & 5 deletions .github/actions/tool-setup/action.yml
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,6 @@ runs:
# Read tool versions
. .github/versions.sh

if [[ "${INPUT_PHP:-$PHP_VERSION}" == 7.[01] ]]; then
printf "Downgrading composer for PHP %s\n\n" "${INPUT_PHP:-$PHP_VERSION}"
COMPOSER_VERSION=2.2.24
fi

printf "\n\e[1mSelected tool versions\e[0m\n"
echo " PHP: ${INPUT_PHP:-$PHP_VERSION}"
echo "php-version=${INPUT_PHP:-$PHP_VERSION}" >> "$GITHUB_OUTPUT"
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
Significance: major
Type: removed

Remove JSX runtime polyfill, now that we've dropped support for WordPress < 6.6.
6 changes: 0 additions & 6 deletions projects/packages/assets/src/class-assets.php
Original file line number Diff line number Diff line change
Expand Up @@ -537,12 +537,6 @@ public static function wp_default_scripts_hook( $wp_scripts ) {
$wp_scripts->add( 'wp-jp-i18n-state', false, array( 'wp-deprecated', $handle ) );
$wp_scripts->add_inline_script( 'wp-jp-i18n-state', 'wp.deprecated( "wp-jp-i18n-state", { alternative: "wp-jp-i18n-loader" } );' );
$wp_scripts->add_inline_script( 'wp-jp-i18n-state', 'wp.jpI18nState = wp.jpI18nLoader.state;' );

// Register the React JSX runtime script - used as a polyfill until we can update JSX transforms. See https://github.com/Automattic/jetpack/issues/38424.
// @todo Remove this when we drop support for WordPress 6.5, as well as the script inclusion in test_wp_default_scripts_hook.
$jsx_url = self::normalize_path( plugins_url( '../build/react-jsx-runtime.js', __FILE__ ) );
$wp_scripts->add( 'react-jsx-runtime', $jsx_url, array( 'react' ), '18.3.1', true );
$wp_scripts->add_data( 'react-jsx-runtime', 'group', 1 );
}

// endregion .
Expand Down
15 changes: 4 additions & 11 deletions projects/packages/assets/tests/php/test-assets.php
Original file line number Diff line number Diff line change
Expand Up @@ -755,8 +755,7 @@ function ( $value ) use ( $value_sets, $i ) {
return $funcs;
};

// @todo: Remove `react-jsx-runtime` from the list of dependencies once we drop support for WordPress 6.5 and remove the dependency from wp_default_scripts_hook.
$mock->expects( $this->exactly( 3 ) )->method( 'add' )
$mock->expects( $this->exactly( 2 ) )->method( 'add' )
->with(
...$with_consecutive(
array(
Expand All @@ -767,12 +766,7 @@ function ( $value ) use ( $value_sets, $i ) {
),
array( 'wp-i18n' ),
),
array( 'wp-jp-i18n-state', false, array( 'wp-deprecated', 'wp-jp-i18n-loader' ) ),
array(
'react-jsx-runtime',
'http://www.example.com/wp-content/plugins/jetpack/packages/assets/build/react-jsx-runtime.js',
array( 'react' ),
)
array( 'wp-jp-i18n-state', false, array( 'wp-deprecated', 'wp-jp-i18n-loader' ) )
)
);
$mock->expects( $this->exactly( 3 ) )->method( 'add_inline_script' )
Expand All @@ -783,11 +777,10 @@ function ( $value ) use ( $value_sets, $i ) {
array( 'wp-jp-i18n-state', 'wp.jpI18nState = wp.jpI18nLoader.state;' )
)
);
$mock->expects( $this->exactly( 2 ) )->method( 'add_data' )
$mock->expects( $this->once() )->method( 'add_data' )
->with(
...$with_consecutive(
array( 'wp-jp-i18n-loader', 'group', 1 ),
array( 'react-jsx-runtime', 'group', 1 )
array( 'wp-jp-i18n-loader', 'group', 1 )
)
);

Expand Down
19 changes: 0 additions & 19 deletions projects/packages/assets/webpack.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -61,23 +61,4 @@ module.exports = [
} ),
],
},
{
entry: {
'react-jsx-runtime': {
import: 'react/jsx-runtime',
},
},
output: {
...jetpackWebpackConfig.output,
path: path.resolve( './build' ),
filename: 'react-jsx-runtime.js',
library: {
name: 'ReactJSXRuntime',
type: 'window',
},
},
externals: {
react: 'React',
},
},
];
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
Significance: major
Type: removed

Remove support for WordPress 6.5 and earlier.
24 changes: 3 additions & 21 deletions projects/packages/autoloader/src/class-latest-autoloader-guard.php
Original file line number Diff line number Diff line change
Expand Up @@ -131,41 +131,23 @@ public function check_for_conflicting_autoloaders() {
foreach ( $composer_autoloader->getClassMap() as $classname => $path ) {
if ( $jetpack_autoloader_loader->find_class_file( $classname ) ) {
$msg = "A Composer autoloader is registered with a higher priority than the Jetpack Autoloader and would also handle some of the classes we handle (e.g. $classname => $path). This may cause strange and confusing problems.";
// @todo Remove the is_callable check once we drop support for WP 6.5.
if ( is_callable( 'wp_trigger_error' ) ) {
wp_trigger_error( '', $msg );
} else {
// phpcs:ignore WordPress.PHP.DevelopmentFunctions.error_log_trigger_error
trigger_error( $msg );
}
wp_trigger_error( '', $msg );
continue 2;
}
}
foreach ( $composer_autoloader->getPrefixesPsr4() as $prefix => $paths ) {
if ( isset( $prefixes[ $prefix ] ) ) {
$path = array_pop( $paths );
$msg = "A Composer autoloader is registered with a higher priority than the Jetpack Autoloader and would also handle some of the namespaces we handle (e.g. $prefix => $path). This may cause strange and confusing problems.";
// @todo Remove the is_callable check once we drop support for WP 6.5.
if ( is_callable( 'wp_trigger_error' ) ) {
wp_trigger_error( '', $msg );
} else {
// phpcs:ignore WordPress.PHP.DevelopmentFunctions.error_log_trigger_error
trigger_error( $msg );
}
wp_trigger_error( '', $msg );
continue 2;
}
}
foreach ( $composer_autoloader->getPrefixes() as $prefix => $paths ) {
if ( isset( $prefixes[ $prefix ] ) ) {
$path = array_pop( $paths );
$msg = "A Composer autoloader is registered with a higher priority than the Jetpack Autoloader and would also handle some of the namespaces we handle (e.g. $prefix => $path). This may cause strange and confusing problems.";
// @todo Remove the is_callable check once we drop support for WP 6.5.
if ( is_callable( 'wp_trigger_error' ) ) {
wp_trigger_error( '', $msg );
} else {
// phpcs:ignore WordPress.PHP.DevelopmentFunctions.error_log_trigger_error
trigger_error( $msg );
}
wp_trigger_error( '', $msg );
continue 2;
}
}
Expand Down
8 changes: 1 addition & 7 deletions projects/packages/autoloader/src/class-php-autoloader.php
Original file line number Diff line number Diff line change
Expand Up @@ -88,13 +88,7 @@ public static function load_class( $class_name ) {
) {
// phpcs:ignore WordPress.PHP.DevelopmentFunctions.error_log_wp_debug_backtrace_summary -- This is a debug log message.
$msg = "Jetpack Autoloader: Autoloading `$class_name` before the plugins_loaded hook may cause strange and confusing problems. " . wp_debug_backtrace_summary( '', 1 );
// @todo Remove the is_callable check once we drop support for WP 6.5.
if ( is_callable( 'wp_trigger_error' ) ) {
wp_trigger_error( '', $msg );
} else {
// phpcs:ignore WordPress.PHP.DevelopmentFunctions.error_log_trigger_error
trigger_error( $msg );
}
wp_trigger_error( '', $msg );
}

require $file;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
Significance: patch
Type: removed
Comment: Additional cleanup of old PHP + WP version support.


Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
Significance: minor
Type: deprecated

Using getChangesBySubheading() with a param now throws an exception.
15 changes: 7 additions & 8 deletions projects/packages/changelogger/lib/ChangelogEntry.php
Original file line number Diff line number Diff line change
Expand Up @@ -268,31 +268,30 @@ public function appendChange( ChangeEntry $change ) {
/**
* Get the changes grouped by subheading.
*
* @param string|null $subheading Subheading to retrieve. @deprecated since 4.2.0. Do `->getChangesBySubheading()[ $subheading ]` instead.
* @param string|null $subheading Subheading to retrieve. @deprecated since 4.2.0; now throws error. Do `->getChangesBySubheading()[ $subheading ]` instead.
* @return array<string,ChangeEntry[]> Change entries by subheading.
* Returns just the `ChangeEntry[]` if the deprecated `$subheading` parameter is used.
* @throws \InvalidArgumentException If the deprecated `$subheading` parameter is used.
*/
public function getChangesBySubheading( $subheading = null ) {
if ( $subheading !== null ) {
// @todo Sometime for a major version bump, have this throw instead. Then remove it for a major bump after that.
// phpcs:ignore WordPress.PHP.DevelopmentFunctions.error_log_trigger_error
trigger_error( 'Passing a value for $subheading is deprecated. Do `->getChangesBySubheading()[ $subheading ]` instead.', E_USER_DEPRECATED );
// @todo Remove this next major version bump.
throw new \InvalidArgumentException( 'Passing a value for $subheading is deprecated. Do `->getChangesBySubheading()[ $subheading ]` instead.' );
}

$ret = array();
foreach ( $this->changes as $entry ) {
$ret[ $entry->getSubheading() ][] = $entry;
}

return null === $subheading
? $ret
: ( $ret[ $subheading ] ?? array() );
return $ret;
}

/**
* Return data for serializing to JSON.
*
* @return array
*
* @todo Once we drop support for PHP <7.4, we should remove the attribute and add a `mixed` return type to match JsonSerializable.
*/
#[\ReturnTypeWillChange]
public function jsonSerialize() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -98,12 +98,15 @@ public function testChanges() {

$this->assertSame( array(), $entry->getChanges() );
$this->assertSame( array(), $entry->getChangesBySubheading() );
error_clear_last();
$this->assertSame( array(), @$entry->getChangesBySubheading( 'B' ) );
$err = error_get_last();
$this->assertNotNull( $err );
$this->assertSame( E_USER_DEPRECATED, $err['type'] );
$this->assertSame( 'Passing a value for $subheading is deprecated. Do `->getChangesBySubheading()[ $subheading ]` instead.', $err['message'] );

try {
$entry->getChangesBySubheading( 'B' );
} catch ( \InvalidArgumentException $e ) {
$this->assertSame(
'Passing a value for $subheading is deprecated. Do `->getChangesBySubheading()[ $subheading ]` instead.',
$e->getMessage()
);
}

$this->assertSame( $entry, $entry->setChanges( $changes ) );
$this->assertSame( $changes, $entry->getChanges() );
Expand All @@ -116,13 +119,6 @@ public function testChanges() {
$entry->getChangesBySubheading()
);

error_clear_last();
$this->assertSame( array( $changes[1], $changes[2] ), @$entry->getChangesBySubheading( 'B' ) );
$err = error_get_last();
$this->assertNotNull( $err );
$this->assertSame( E_USER_DEPRECATED, $err['type'] );
$this->assertSame( 'Passing a value for $subheading is deprecated. Do `->getChangesBySubheading()[ $subheading ]` instead.', $err['message'] );

$c1 = new ChangeEntry(
array(
'subheading' => 'B',
Expand Down Expand Up @@ -155,13 +151,6 @@ public function testChanges() {
$entry->getChangesBySubheading()
);

error_clear_last();
$this->assertSame( array( $c2, $changes[1], $c1, $changes[2] ), @$entry->getChangesBySubheading( 'B' ) );
$err = error_get_last();
$this->assertNotNull( $err );
$this->assertSame( E_USER_DEPRECATED, $err['type'] );
$this->assertSame( 'Passing a value for $subheading is deprecated. Do `->getChangesBySubheading()[ $subheading ]` instead.', $err['message'] );

$entry = new ChangelogEntry( '1.0' );
$this->assertSame( $entry, $entry->appendChange( $c1 )->appendChange( $c2 )->appendChange( $c3 ) );
$this->assertSame( array( $c1, $c2, $c3 ), $entry->getChanges() );
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ public function testRunCommand( $cmd, $options, $expectExitCode, $expectStdout,
* Data provider for testRunCommand.
*/
public function provideRunCommand() {
$tmp = sys_get_temp_dir();
$tmp = realpath( sys_get_temp_dir() );
tbradsha marked this conversation as resolved.
Show resolved Hide resolved

return array(
'true' => array(
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
Significance: patch
Type: removed
Comment: Additional cleanup of old PHP + WP version support.


Original file line number Diff line number Diff line change
Expand Up @@ -987,8 +987,7 @@ public static function update_custom_css_data( $args ) {
public static function editor_max_image_size( $dims, $size = 'medium', $context = null ) {
list( $width, $height ) = $dims;

// @phan-suppress-next-line PhanUndeclaredClassInCallable
if ( class_exists( 'Jetpack' ) && is_callable( 'Jetpack::get_content_width' ) && 'large' === $size && 'edit' === $context ) {
if ( class_exists( 'Jetpack' ) && 'large' === $size && 'edit' === $context ) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like a reasonable change, but doesn't seem to have anything to do with the PHP 7.2 change. Same for the similar stuff below.

I wonder if we even need the class_exists( 'Jetpack' ) test? This seems to have been added in dd3b1d8 to try fixing static analysis.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed PR title.

As for whether we need it, I wasn't confident enough in the load order and knowledge of the environment to know if there's an edge case where the class might not exist. 😓

// @phan-suppress-next-line PhanUndeclaredClassMethod
$width = Jetpack::get_content_width();
}
Expand Down Expand Up @@ -1029,8 +1028,7 @@ public static function set_content_width() {
return;
}

// @phan-suppress-next-line PhanUndeclaredClassInCallable
if ( class_exists( 'Jetpack' ) && is_callable( 'Jetpack::get_content_width' ) ) {
if ( class_exists( 'Jetpack' ) ) {
// @phan-suppress-next-line PhanUndeclaredClassMethod
$GLOBALS['content_width'] = Jetpack::get_content_width();
}
Expand Down Expand Up @@ -1078,8 +1076,7 @@ public static function load_revision_php() {
* Enable CSS module.
*/
public static function custom_css_loaded() {
// @phan-suppress-next-line PhanUndeclaredClassInCallable
if ( class_exists( 'Jetpack' ) && is_callable( 'Jetpack::enable_module_configurable' ) ) {
if ( class_exists( 'Jetpack' ) ) {
// @phan-suppress-next-line PhanUndeclaredClassMethod
Jetpack::enable_module_configurable( __FILE__ );
}
Expand Down Expand Up @@ -1252,4 +1249,4 @@ public function subvalue() { // phpcs:ignore MediaWiki.Usage.NestedFunctions.Nes
}
}
}
endif;
endif;
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@
* @return bool
*/
function is_jetpack_connected() {
// @phan-suppress-next-line PhanUndeclaredClassMethod, PhanUndeclaredClassInCallable
return class_exists( 'Jetpack' ) && is_callable( 'Jetpack::is_connection_ready' ) && Jetpack::is_connection_ready();
// @phan-suppress-next-line PhanUndeclaredClassMethod
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like a reasonable change, but doesn't seem to have anything to do with the PHP 7.2 change.

In this case, I don't see any indication as to why the is_callable was added in a4dea90, although Phan may again be the reason.

The tricky part with adding a stub is that Simple has its own Jetpack class (fbhepr%2Skers%2Sjcpbz%2Sjc%2Qpbagrag%2Szh%2Qcyhtvaf%2Swrgcnpx%2Spynff.wrgcnpx.cuc%3Se%3Q37575q28-og) which doesn't entirely match Jetpack's, so whether we point to Jetpack's or make a stub from Simple's it'd be wrong for some environments. :sigh:

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated the PR title. Good to know about the Simple stub! At least in this case it still should be safe.

return class_exists( 'Jetpack' ) && Jetpack::is_connection_ready();
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,3 @@
width: 100%;
}
}

// Fix so datetime picker doesn't have horizontal scrollbar.
// This is a global fix, unscoped, and not very nice.
// @todo: It won't be necessary once https://github.com/WordPress/gutenberg/pull/18235/files gets merged.
.components-datetime {
padding: 8px;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
Significance: patch
Type: removed
Comment: Additional cleanup of old PHP + WP version support.


Original file line number Diff line number Diff line change
Expand Up @@ -49,10 +49,9 @@ public function tear_down() {
* Check if the nudge contains the proper url and message copy.
*/
public function test_it_generates_proper_url_and_nudge() {
// @phan-suppress-next-line PhanDeprecatedFunction -- Keep using setMethods until we drop PHP 7.0 support.
$manager = $this->getMockBuilder( Atomic_Additional_CSS_Manager::class )
->setConstructorArgs( array( 'foo.com' ) )
->setMethods( array( 'get_plan_name' ) )
->onlyMethods( array( 'get_plan_name' ) )
->getMock();

$manager->method( 'get_plan_name' )->willReturn( 'Business' );
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,10 +40,9 @@ public function set_up() {
* Check if the manager constructs the proper url and copy message.
*/
public function test_it_generates_proper_url_and_nudge() {
// @phan-suppress-next-line PhanDeprecatedFunction -- Keep using setMethods until we drop PHP 7.0 support.
$manager = $this->getMockBuilder( WPCOM_Additional_CSS_Manager::class )
->setConstructorArgs( array( 'foo.com' ) )
->setMethods( array( 'get_plan' ) )
->onlyMethods( array( 'get_plan' ) )
->getMock();

$manager->method( 'get_plan' )->willReturn(
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
Significance: patch
Type: removed
Comment: Additional cleanup of old PHP + WP version support.


3 changes: 1 addition & 2 deletions projects/packages/status/src/class-status.php
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,7 @@ public function is_offline_mode() {
/**
* Filters Jetpack's offline mode.
*
* @see https://jetpack.com/support/development-mode/
* @todo Update documentation ^^.
* @see https://jetpack.com/support/offline-mode/
anomiex marked this conversation as resolved.
Show resolved Hide resolved
*
* @since 1.3.0
*
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
Significance: patch
Type: removed
Comment: Additional cleanup of old PHP + WP version support.


Loading
Loading