Skip to content

Fix PHPStan undefined variable errors at level 3 #11189

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.

Sign up for GitHub

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

Open
huzaifaalmesbah wants to merge 6 commits into
base: trunk
Choose a base branch
from
+4 −1
Open

Fix PHPStan undefined variable errors at level 3 #11189

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
f86194a
Fix PHPStan undefined variable errors at level 3
huzaifaalmesbah Mar 6, 2026
8b93b41
Code Quality: Fix PHPStan undefined variable errors in ms-settings.php
huzaifaalmesbah Mar 7, 2026
30d971c
Merge branch 'trunk' into fix/phpstan-undefined-variables
huzaifaalmesbah Mar 7, 2026
9d9dd59
Code Quality: Fix PHPStan undefined variables using isset/empty checks
huzaifaalmesbah Mar 8, 2026
e6bbbfc
Apply alternative suggestion
huzaifaalmesbah Mar 9, 2026
eef6fa5
Apply alternative suggestion
huzaifaalmesbah Mar 9, 2026
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
4 changes: 3 additions & 1 deletion src/wp-includes/ms-settings.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,10 @@
* Use `get_current_network_id()` instead.
* @global bool $public Deprecated. Whether the site found on load is public.
* Use `get_site()->public` instead.
* @global string $table_prefix Database table prefix.
Copy link
Copy Markdown
Member

@westonruter westonruter Mar 7, 2026

Choose a reason for hiding this comment

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

Global populated in wp-config.php.

Copy link
Copy Markdown
Member

@westonruter westonruter Mar 8, 2026

Choose a reason for hiding this comment

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

Or not. It can get populated in sunrise.php.

* @global wpdb $wpdb WordPress database abstraction object.
Copy link
Copy Markdown
Member

@westonruter westonruter Mar 7, 2026

Choose a reason for hiding this comment

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

Global populated by require_wp_db().

*/
global $current_site, $current_blog, $domain, $path, $site_id, $public;
global $current_site, $current_blog, $domain, $path, $site_id, $public, $table_prefix, $wpdb;
Copy link
Copy Markdown
Member

@westonruter westonruter Mar 8, 2026

Choose a reason for hiding this comment

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

It seems we should remove $table_prefix from being marked as global here for the same reason as explained in wp-settings.php. We should perhaps do this instead of marking it as global:

--- a/src/wp-includes/ms-settings.php
+++ b/src/wp-includes/ms-settings.php
@@ -99,7 +99,9 @@ if ( ! isset( $current_site ) || ! isset( $current_blog ) ) {
 	wp_load_core_site_options( $site_id );
 }
 
-$wpdb->set_prefix( $table_prefix, false ); // $table_prefix can be set in sunrise.php.
+if ( isset( $table_prefix ) ) {
+	$wpdb->set_prefix( $table_prefix, false ); // $table_prefix can be set in sunrise.php.
+}
 $wpdb->set_blog_id( $current_blog->blog_id, $current_blog->site_id );
 $table_prefix       = $wpdb->get_blog_prefix();
 $_wp_switched_stack = array();

Note the reason given regarding sunrise.php. If $table_prefix is set in sunrise.php, and that file is included above via include_once, it could be that it isn't a global there. As I understand from the custom bootstrapping logic, this could accidentally override the actual $table_prefix global. Maybe there's no world in which this is an issue, but it seems safer to go with isset() checks than to declare these as global, at least this late in the release.

Copy link
Copy Markdown
Member

@westonruter westonruter Mar 8, 2026

Choose a reason for hiding this comment

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

To play it absolutely safe, and to not global $wpdb:

--- a/src/wp-includes/ms-settings.php
+++ b/src/wp-includes/ms-settings.php
@@ -99,9 +99,13 @@ if ( ! isset( $current_site ) || ! isset( $current_blog ) ) {
 	wp_load_core_site_options( $site_id );
 }
 
-$wpdb->set_prefix( $table_prefix, false ); // $table_prefix can be set in sunrise.php.
-$wpdb->set_blog_id( $current_blog->blog_id, $current_blog->site_id );
-$table_prefix       = $wpdb->get_blog_prefix();
+if ( isset( $wpdb ) ) {
+	if ( isset( $table_prefix ) ) {
+		$wpdb->set_prefix( $table_prefix, false ); // $table_prefix can be set in sunrise.php.
+	}
+	$wpdb->set_blog_id( $current_blog->blog_id, $current_blog->site_id );
+	$table_prefix = $wpdb->get_blog_prefix();
+}
 $_wp_switched_stack = array();
 $switched           = false;

Copy link
Copy Markdown
Member

@westonruter westonruter Mar 9, 2026

Choose a reason for hiding this comment

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

For reference, how assert() is used in 3rd-party libs bundled with core: https://github.com/search?q=repo%3AWordPress%2Fwordpress-develop+%2Fassert%5C%28%2F+language%3Aphp+path%3A%2F%5Esrc%5C%2F%2F&type=code


/** WP_Network class */
require_once ABSPATH . WPINC . '/class-wp-network.php';
Expand Down
4 changes: 3 additions & 1 deletion src/wp-settings.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,10 @@
* @global string[] $required_php_extensions The names of required PHP extensions.
* @global string $required_mysql_version The minimum required MySQL version string.
* @global string $wp_local_package Locale code of the package.
* @global array $wp_filter WordPress filter hooks.
* @global string $table_prefix Database table prefix.
*/
global $wp_version, $wp_db_version, $tinymce_version, $required_php_version, $required_php_extensions, $required_mysql_version, $wp_local_package;
global $wp_version, $wp_db_version, $tinymce_version, $required_php_version, $required_php_extensions, $required_mysql_version, $wp_local_package, $wp_filter, $table_prefix;
Comment thread
westonruter marked this conversation as resolved.
Outdated
Copy link
Copy Markdown
Member

@westonruter westonruter Mar 7, 2026

Choose a reason for hiding this comment

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

There's something curious below in how the $table_prefix is set:

wordpress-develop/src/wp-settings.php

Lines 140 to 147 in 30d971c

/**
* @since 3.3.0
*
* @global string $table_prefix The database table prefix.
*/
if ( ! isset( $GLOBALS['table_prefix'] ) ) {
$GLOBALS['table_prefix'] = $table_prefix;
}

This to me looks like dead code. But in fact, it is pretty new: 61ae275

It was developed for Core-63627.

I wonder if explicitly globalizing $table_prefix that this could break how this was intended to be used, if $table_prefix is loaded in a lexical scope somehow which is distinct from the $table_prefix which is also a global. I'm not sure how this would work, but it gives me pause.

@SergeyBiryukov can you shed some light on this as you made that commit?

It may be safer to do this patch instead:

diff --git a/src/wp-settings.php b/src/wp-settings.php
index 023cdccd5e..87fa935aa1 100644
--- a/src/wp-settings.php
+++ b/src/wp-settings.php
@@ -100,7 +100,7 @@ if ( WP_CACHE && apply_filters( 'enable_loading_advanced_cache_dropin', true ) &
 	include WP_CONTENT_DIR . '/advanced-cache.php';
 
 	// Re-initialize any hooks added manually by advanced-cache.php.
-	if ( $wp_filter ) {
+	if ( isset( $wp_filter ) ) {
 		$wp_filter = WP_Hook::build_preinitialized_hooks( $wp_filter );
 	}
 }
@@ -140,7 +140,7 @@ require_wp_db();
  *
  * @global string $table_prefix The database table prefix.
  */
-if ( ! isset( $GLOBALS['table_prefix'] ) ) {
+if ( ! isset( $GLOBALS['table_prefix'] ) && isset( $table_prefix ) ) {
 	$GLOBALS['table_prefix'] = $table_prefix;
 }

But let's get confirmation.

Copy link
Copy Markdown
Member

@westonruter westonruter Mar 7, 2026

Choose a reason for hiding this comment

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

Question raised: https://core.trac.wordpress.org/ticket/63627#comment:6

Copy link
Copy Markdown
Member

@westonruter westonruter Mar 8, 2026

Choose a reason for hiding this comment

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

And actually, for $wp_filter, this would be more correct:

--- a/src/wp-settings.php
+++ b/src/wp-settings.php
@@ -100,7 +100,7 @@ if ( WP_CACHE && apply_filters( 'enable_loading_advanced_cache_dropin', true ) &
 	include WP_CONTENT_DIR . '/advanced-cache.php';
 
 	// Re-initialize any hooks added manually by advanced-cache.php.
-	if ( $wp_filter ) {
+	if ( ! empty( $wp_filter ) ) {
 		$wp_filter = WP_Hook::build_preinitialized_hooks( $wp_filter );
 	}
 }

require ABSPATH . WPINC . '/version.php';
require ABSPATH . WPINC . '/compat-utf8.php';
require ABSPATH . WPINC . '/compat.php';
Expand Down
Loading