Skip to content

Commit d0cceb5

Browse files
westonruterschlessera
westonruter
and
schlessera
committed
Identify the theme/plugin responsible for calling amp_is_available() too early (#5289)
Co-authored-by: Alain Schlesser <[email protected]>
1 parent 203d301 commit d0cceb5

36 files changed

+1690
-316
lines changed

‎.phpcs.xml.dist‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,7 @@
100100
<config name="testVersion" value="5.6-"/>
101101
<rule ref="PHPCompatibilityWP">
102102
<exclude-pattern>bin/*</exclude-pattern>
103+
<exclude-pattern>tests/php/src/PhpStan/*</exclude-pattern>
103104
</rule>
104105

105106
<rule ref="Generic.Arrays.DisallowLongArraySyntax.Found">

‎.phpstorm.meta.php‎

Lines changed: 33 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -2,28 +2,37 @@
22

33
namespace PHPSTORM_META {
44

5-
override( \AmpProject\AmpWP\Services::get(), map( [
6-
'admin.analytics_menu' => \AmpProject\AmpWP\Admin\AnalyticsOptionsSubmenu::class,
7-
'admin.google_fonts' => \AmpProject\AmpWP\Admin\GoogleFonts::class,
8-
'admin.onboarding_menu' => \AmpProject\AmpWP\Admin\OnboardingWizardSubmenu::class,
9-
'admin.onboarding_wizard' => \AmpProject\AmpWP\Admin\OnboardingWizardSubmenuPage::class,
10-
'admin.options_menu' => \AmpProject\AmpWP\Admin\OptionsMenu::class,
11-
'admin.polyfills' => \AmpProject\AmpWP\Admin\Polyfills::class,
12-
'amp_slug_customization_watcher' => \AmpProject\AmpWP\AmpSlugCustomizationWatcher::class,
13-
'css_transient_cache.ajax_handler' => \AmpProject\AmpWP\Admin\ReenableCssTransientCachingAjaxAction::class,
14-
'css_transient_cache.monitor' => \AmpProject\AmpWP\BackgroundTask\MonitorCssTransientCaching::class,
15-
'dev_tools.user_access' => \AmpProject\AmpWP\Admin\DevToolsUserAccess::class,
16-
'error_page' => \AmpProject\AmpWP\ErrorPage::class,
17-
'extra_theme_and_plugin_headers' => \AmpProject\AmpWP\ExtraThemeAndPluginHeaders::class,
18-
'mobile_redirection' => \AmpProject\AmpWP\MobileRedirection::class,
19-
'obsolete_block_attribute_remover' => \AmpProject\AmpWP\ObsoleteBlockAttributeRemover::class,
20-
'plugin_activation_notice' => \AmpProject\AmpWP\Admin\PluginActivationNotice::class,
21-
'plugin_registry' => \AmpProject\AmpWP\PluginRegistry::class,
22-
'plugin_suppression' => \AmpProject\AmpWP\PluginSuppression::class,
23-
'reader_theme_loader' => \AmpProject\AmpWP\ReaderThemeLoader::class,
24-
'rest.options_controller' => \AmpProject\AmpWP\OptionsRESTController::class,
25-
'server_timing' => \AmpProject\AmpWP\Instrumentation\ServerTiming::class,
26-
'site_health_integration' => \AmpProject\AmpWP\Admin\SiteHealth::class,
27-
'validated_url_stylesheet_gc' => \AmpProject\AmpWP\BackgroundTask\ValidatedUrlStylesheetDataGarbageCollection::class,
28-
] ) );
5+
override(
6+
\AmpProject\AmpWP\Services::get(),
7+
8+
// TODO: I'd like to use AmpWpPlugin::SERVICES directly here but it doesn't seem to work.
9+
map( [
10+
'admin.analytics_menu' => \AmpProject\AmpWP\Admin\AnalyticsOptionsSubmenu::class,
11+
'admin.google_fonts' => \AmpProject\AmpWP\Admin\GoogleFonts::class,
12+
'admin.onboarding_menu' => \AmpProject\AmpWP\Admin\OnboardingWizardSubmenu::class,
13+
'admin.onboarding_wizard' => \AmpProject\AmpWP\Admin\OnboardingWizardSubmenuPage::class,
14+
'admin.options_menu' => \AmpProject\AmpWP\Admin\OptionsMenu::class,
15+
'admin.polyfills' => \AmpProject\AmpWP\Admin\Polyfills::class,
16+
'amp_slug_customization_watcher' => \AmpProject\AmpWP\AmpSlugCustomizationWatcher::class,
17+
'css_transient_cache.ajax_handler' => \AmpProject\AmpWP\Admin\ReenableCssTransientCachingAjaxAction::class,
18+
'css_transient_cache.monitor' => \AmpProject\AmpWP\BackgroundTask\MonitorCssTransientCaching::class,
19+
'dev_tools.callback_reflection' => \AmpProject\AmpWP\DevTools\CallbackReflection::class,
20+
'dev_tools.error_page' => \AmpProject\AmpWP\DevTools\ErrorPage::class,
21+
'dev_tools.file_reflection' => \AmpProject\AmpWP\DevTools\FileReflection::class,
22+
'dev_tools.likely_culprit_detector' => \AmpProject\AmpWP\DevTools\LikelyCulpritDetector::class,
23+
'dev_tools.user_access' => \AmpProject\AmpWP\DevTools\UserAccess::class,
24+
'extra_theme_and_plugin_headers' => \AmpProject\AmpWP\ExtraThemeAndPluginHeaders::class,
25+
'injector' => \AmpProject\AmpWP\Infrastructure\Injector::class,
26+
'mobile_redirection' => \AmpProject\AmpWP\MobileRedirection::class,
27+
'obsolete_block_attribute_remover' => \AmpProject\AmpWP\ObsoleteBlockAttributeRemover::class,
28+
'plugin_activation_notice' => \AmpProject\AmpWP\Admin\PluginActivationNotice::class,
29+
'plugin_registry' => \AmpProject\AmpWP\PluginRegistry::class,
30+
'plugin_suppression' => \AmpProject\AmpWP\PluginSuppression::class,
31+
'reader_theme_loader' => \AmpProject\AmpWP\ReaderThemeLoader::class,
32+
'rest.options_controller' => \AmpProject\AmpWP\OptionsRESTController::class,
33+
'server_timing' => \AmpProject\AmpWP\Instrumentation\ServerTiming::class,
34+
'site_health_integration' => \AmpProject\AmpWP\Admin\SiteHealth::class,
35+
'validated_url_stylesheet_gc' => \AmpProject\AmpWP\BackgroundTask\ValidatedUrlStylesheetDataGarbageCollection::class,
36+
] )
37+
);
2938
}

‎.travis.yml‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -134,11 +134,11 @@ jobs:
134134

135135
- name: PHP unit tests (5.6, WordPress 5.0)
136136
php: "5.6"
137-
env: WP_VERSION=5.0 DEV_LIB_ONLY=phpunit,phpsyntax INSTALL_PWA_PLUGIN=1
137+
env: WP_VERSION=5.0 DEV_LIB_ONLY=phpunit,phpsyntax INSTALL_PWA_PLUGIN=1 PATH_EXCLUDES_PATTERN='^(.*/)?(vendor|bower_components|node_modules|PhpStan)/.*'
138138

139139
- name: PHP unit tests w/ external-http (5.6, WordPress 4.9)
140140
php: "5.6"
141-
env: WP_VERSION=4.9 DEV_LIB_ONLY=phpunit,phpsyntax PHPUNIT_EXTRA_SUITE=external-http
141+
env: WP_VERSION=4.9 DEV_LIB_ONLY=phpunit,phpsyntax PHPUNIT_EXTRA_SUITE=external-http PATH_EXCLUDES_PATTERN='^(.*/)?(vendor|bower_components|node_modules|PhpStan)/.*'
142142

143143
- name: PHP unit tests (8.0, WordPress trunk)
144144
php: "nightly"

‎includes/admin/class-amp-template-customizer.php‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,6 @@ protected function __construct( WP_Customize_Manager $wp_customize, ReaderThemeL
7575
* @return AMP_Template_Customizer Instance.
7676
*/
7777
public static function init( WP_Customize_Manager $wp_customize ) {
78-
/** @var ReaderThemeLoader $reader_theme_loader */
7978
$reader_theme_loader = Services::get( 'reader_theme_loader' );
8079

8180
$self = new self( $wp_customize, $reader_theme_loader );

‎includes/amp-helper-functions.php‎

Lines changed: 49 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
use AmpProject\AmpWP\Icon;
1111
use AmpProject\AmpWP\Option;
1212
use AmpProject\AmpWP\QueryVar;
13+
use AmpProject\AmpWP\Services;
1314

1415
/**
1516
* Handle activation of plugin.
@@ -413,24 +414,59 @@ function amp_is_available() {
413414
return false;
414415
}
415416

416-
$warn = function () {
417-
static $warned = false;
418-
if ( $warned ) {
417+
$warn = static function () {
418+
static $already_warned_sources = [];
419+
420+
$likely_culprit_detector = Services::get( 'dev_tools.likely_culprit_detector' );
421+
422+
$closest_source = $likely_culprit_detector->analyze_backtrace();
423+
$closest_source_identifier = $closest_source['type'] . ':' . $closest_source['name'];
424+
if ( in_array( $closest_source_identifier, $already_warned_sources, true ) ) {
419425
return;
420426
}
427+
421428
$message = sprintf(
422-
/* translators: %1$s: amp_is_available(), %2$s: amp_is_request(), %3$s: is_amp_endpoint(), %4$s: the current action, %5$s: the wp action, %6$s: the WP_Query class, %7$s: the amp_skip_post() function */
423-
__( '%1$s (or %2$s, formerly %3$s) was called too early and so it will not work properly. WordPress is currently doing the "%4$s" action. Calling this function before the "%5$s" action means it will not have access to %6$s and the queried object to determine if it is an AMP response, thus neither the "%7$s" filter nor the AMP enabled toggle will be considered.', 'amp' ),
424-
'amp_is_available()',
425-
'amp_is_request()',
426-
'is_amp_endpoint()',
427-
current_action(),
428-
'wp',
429-
'WP_Query',
430-
'amp_skip_post()'
429+
/* translators: 1: amp_is_available() function, 2: amp_is_request() function, 3: is_amp_endpoint() function */
430+
__( '%1$s (or %2$s, formerly %3$s) was called too early and so it will not work properly.', 'amp' ),
431+
'`amp_is_available()`',
432+
'`amp_is_request()`',
433+
'`is_amp_endpoint()`'
431434
);
435+
436+
$message .= ' ' . sprintf(
437+
/* translators: 1: the current hook, 2: the wp action, 4: the WP_Query class, 4: the amp_skip_post() function */
438+
__( 'WordPress is currently doing the %1$s hook. Calling this function before the %2$s action means it will not have access to %3$s and the queried object to determine if it is an AMP response, thus neither the %4$s filter nor the AMP enabled toggle will be considered.', 'amp' ),
439+
'`' . current_action() . '`',
440+
'`wp`',
441+
'`WP_Query`',
442+
'`amp_skip_post()`'
443+
);
444+
445+
if ( ! empty( $closest_source['type'] ) && ! empty( $closest_source['name'] ) ) {
446+
$translated_string = false;
447+
448+
switch ( $closest_source['type'] ) {
449+
case 'plugin':
450+
/* translators: placeholder is the slug of the plugin */
451+
$translated_string = __( 'It appears the plugin with slug %s is responsible; please contact the author.', 'amp' );
452+
break;
453+
case 'mu-plugin':
454+
/* translators: placeholder is the slug of the must-use plugin */
455+
$translated_string = __( 'It appears the must-use plugin with slug %s is responsible; please contact the author.', 'amp' );
456+
break;
457+
case 'theme':
458+
/* translators: placeholder is the slug of the theme */
459+
$translated_string = __( 'It appears the theme with slug %s is responsible; please contact the author.', 'amp' );
460+
break;
461+
}
462+
463+
if ( $translated_string ) {
464+
$message .= ' ' . sprintf( $translated_string, '`' . $closest_source['name'] . '`' );
465+
}
466+
}
467+
432468
_doing_it_wrong( 'amp_is_available', esc_html( $message ), '2.0.0' );
433-
$warned = true;
469+
$already_warned_sources[] = $closest_source_identifier;
434470
};
435471

436472
// Make sure the parse_request action has triggered before trying to read from the REST_REQUEST constant, which is set during rest_api_loaded().

‎includes/class-amp-theme-support.php‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
*/
77

88
use AmpProject\Amp;
9-
use AmpProject\AmpWP\ErrorPage;
9+
use AmpProject\AmpWP\DevTools\ErrorPage;
1010
use AmpProject\AmpWP\ExtraThemeAndPluginHeaders;
1111
use AmpProject\AmpWP\Option;
1212
use AmpProject\AmpWP\QueryVar;
@@ -1843,8 +1843,7 @@ public static function finish_output_buffering( $response ) {
18431843
$title = __( 'Failed to prepare AMP page', 'amp' );
18441844
$message = __( 'A PHP error occurred while trying to prepare the AMP response. This may not be caused by the AMP plugin but by some other active plugin or the current theme. You will need to review the error details to determine the source of the error.', 'amp' );
18451845

1846-
/** @var ErrorPage $error_page */
1847-
$error_page = Services::get( 'error_page' );
1846+
$error_page = Services::get( 'dev_tools.error_page' );
18481847

18491848
$error_page
18501849
->with_title( $title )

‎includes/validation/class-amp-validated-url-post-type.php‎

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
* @package AMP
66
*/
77

8-
use AmpProject\AmpWP\Admin\DevToolsUserAccess;
8+
use AmpProject\AmpWP\DevTools\UserAccess;
99
use AmpProject\AmpWP\Admin\OptionsMenu;
1010
use AmpProject\AmpWP\Icon;
1111
use AmpProject\AmpWP\PluginRegistry;
@@ -141,7 +141,6 @@ class AMP_Validated_URL_Post_Type {
141141
public static function register() {
142142
add_action( 'amp_plugin_update', [ __CLASS__, 'handle_plugin_update' ] );
143143

144-
/** @var DevToolsUserAccess $dev_tools_user_access */
145144
$dev_tools_user_access = Services::get( 'dev_tools.user_access' );
146145

147146
// Show in the admin menu if dev tools are enabled for the user or if the user is on any dev tools screen.
@@ -270,7 +269,6 @@ public static function handle_plugin_update( $old_version ) {
270269
public static function add_admin_hooks() {
271270
add_action( 'admin_enqueue_scripts', [ __CLASS__, 'enqueue_post_list_screen_scripts' ] );
272271

273-
/** @var DevToolsUserAccess $dev_tools_user_access */
274272
$dev_tools_user_access = Services::get( 'dev_tools.user_access' );
275273

276274
if ( $dev_tools_user_access->is_user_enabled() ) {
@@ -1077,7 +1075,6 @@ public static function get_recent_validation_errors_by_source( $count = 100 ) {
10771075
* @return array Environment.
10781076
*/
10791077
public static function get_validated_environment() {
1080-
/** @var PluginRegistry $plugin_registry */
10811078
$plugin_registry = Services::get( 'plugin_registry' );
10821079

10831080
$theme = [];
@@ -1378,8 +1375,7 @@ public static function render_sources_column( $sources, $post_id ) {
13781375
$active_theme = $validated_environment['theme'];
13791376
}
13801377

1381-
$output = [];
1382-
/** @var PluginRegistry $plugin_registry */
1378+
$output = [];
13831379
$plugin_registry = Services::get( 'plugin_registry' );
13841380
foreach ( wp_array_slice_assoc( $sources, [ 'plugin', 'mu-plugin' ] ) as $type => $slugs ) {
13851381
$plugin_names = [];

‎includes/validation/class-amp-validation-error-taxonomy.php‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
* @package AMP
66
*/
77

8-
use AmpProject\AmpWP\Admin\DevToolsUserAccess;
8+
use AmpProject\AmpWP\DevTools\UserAccess;
99
use AmpProject\AmpWP\Icon;
1010
use AmpProject\AmpWP\PluginRegistry;
1111
use AmpProject\AmpWP\Services;
@@ -227,7 +227,6 @@ class AMP_Validation_Error_Taxonomy {
227227
* @return void
228228
*/
229229
public static function register() {
230-
/** @var DevToolsUserAccess $dev_tools_user_access */
231230
$dev_tools_user_access = Services::get( 'dev_tools.user_access' );
232231

233232
// Show in the admin menu if dev tools are enabled for the user or if the user is on any dev tools screen.
@@ -2360,7 +2359,6 @@ private static function get_file_editor_url( $source ) {
23602359
// Fall back to using the theme/plugin editors if no external editor is offered.
23612360
if ( ! $edit_url ) {
23622361
if ( 'plugin' === $source['type'] && current_user_can( 'edit_plugins' ) ) {
2363-
/** @var PluginRegistry $plugin_registry */
23642362
$plugin_registry = Services::get( 'plugin_registry' );
23652363
$plugin = $plugin_registry->get_plugin_from_slug( $source['name'] );
23662364
if ( $plugin ) {
@@ -2414,7 +2412,6 @@ private static function render_source_name( $name, $type ) {
24142412
}
24152413
break;
24162414
case 'plugin':
2417-
/** @var PluginRegistry $plugin_registry */
24182415
$plugin_registry = Services::get( 'plugin_registry' );
24192416
$plugin = $plugin_registry->get_plugin_from_slug( $name );
24202417
if ( $plugin && ! empty( $plugin['data']['Name'] ) ) {

0 commit comments

Comments
 (0)