Style sanitizer contains a piece of logic that doesn't seem to have any effect

[schlessera]

The style sanitizer contains piece of logic that stores "$indices_by_stylesheet_element_id". However, the entire array is only assembled, but never used.

amp-wp/includes/sanitizers/class-amp-style-sanitizer.php

Lines 2907 to 2910 in 7ba9f26

	// Keep track of the element IDs for the stylesheets.
	if ( $pending_stylesheet['node'] instanceof DOMElement && $pending_stylesheet['node']->hasAttribute( 'id' ) ) {
	$indices_by_stylesheet_element_id[ $pending_stylesheet['node']->getAttribute( 'id' ) ] = $pending_stylesheet_index;
	}

This entire snippet could just be removed, to save processing time and memory.

[schlessera]

Note: the initialization of the array needs to be removed as well:

amp-wp/includes/sanitizers/class-amp-style-sanitizer.php

Line 2901 in 7ba9f26

$indices_by_stylesheet_element_id = [];

[westonruter]

Good catch. This code should have been removed with #3187.

It used to be used here, being introduced in #2346:

amp-wp/includes/sanitizers/class-amp-style-sanitizer.php

Lines 3044 to 3082 in 1b38600

	$included_count -= $this->exclude_all_admin_bar_css_if_excessive( $indices_by_stylesheet_element_id );
	return $included_count;
	}
	/**
	* If the admin-bar CSS was excluded, make sure the admin-bar inline CSS is also excluded, and vice-versa.
	*
	* @param int[] $indices_by_stylesheet_element_id Lookup of stylesheet indices by stylesheet element ID.
	* @return int Number of excluded styles.
	*/
	private function exclude_all_admin_bar_css_if_excessive( $indices_by_stylesheet_element_id ) {
	$excluded_count = 0;
	$admin_bar_style_element_ids = [ 'admin-bar-css', 'admin-bar-inline-css' ];
	$should_exclude_admin_bar_inline_css = false;
	foreach ( $admin_bar_style_element_ids as $admin_bar_style_element_id ) {
	if ( ! isset( $indices_by_stylesheet_element_id[ $admin_bar_style_element_id ] ) ) {
	continue;
	}
	if ( false === $this->pending_stylesheets[ $indices_by_stylesheet_element_id[ $admin_bar_style_element_id ] ]['included'] ) {
	$should_exclude_admin_bar_inline_css = true;
	break;
	}
	}
	if ( $should_exclude_admin_bar_inline_css ) {
	foreach ( $admin_bar_style_element_ids as $admin_bar_style_element_id ) {
	$needs_exclusion = (
	isset( $indices_by_stylesheet_element_id[ $admin_bar_style_element_id ] )
	&&
	true === $this->pending_stylesheets[ $indices_by_stylesheet_element_id[ $admin_bar_style_element_id ] ]['included']
	);
	if ( $needs_exclusion ) {
	$this->pending_stylesheets[ $indices_by_stylesheet_element_id[ $admin_bar_style_element_id ] ]['included'] = false;
	$excluded_count++;
	}
	}
	}
	return $excluded_count;

[westonruter]

Curious: How did you find this logic was not being used?

[schlessera]

@westonruter PHPStorm actually shows this (at least with my current settings):

You can see that the code is grayed out, which means it is not being used. When you hover over it with the mouse, you'll get additional information.

[schlessera]

Oh, I just noticed that this actually comes from a paid addon to PHPStorm: https://kalessil.github.io/php-inspections-ultimate.html

I can heartily recommend this plugin, and @kalessil is extremely responsive if you hit issues or have ideas for additional checks.