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

Enforce checks against redeclaration for functions and classes #52696

Merged
merged 30 commits into from Jul 20, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
6cccfb4
Add Gutenberg coding standards package.
anton-vlasenko Jul 12, 2023
81b0852
Fix contributors.
anton-vlasenko Jul 12, 2023
6ebaf07
Fix package names.
anton-vlasenko Jul 12, 2023
887fab8
Remove unneccessary section.
anton-vlasenko Jul 12, 2023
d91104d
Add Gutenberg coding standards package as a composer dependency.
anton-vlasenko Jul 12, 2023
3e7fa24
Add myself as a code owner of the coding standards package.
anton-vlasenko Jul 12, 2023
37a7273
Exclude gutenberg coding standards from PHPCS checks.
anton-vlasenko Jul 17, 2023
4e12b71
Add the rule to phpcs.xml.dist.
anton-vlasenko Jul 17, 2023
e2f7e90
Guard functions and classes.
anton-vlasenko Jul 17, 2023
1cc8f77
Guard functions and classes.
anton-vlasenko Jul 17, 2023
7db4995
Fix CS.
anton-vlasenko Jul 17, 2023
835399b
Guard functions and classes.
anton-vlasenko Jul 17, 2023
0db5b14
Guard functions and classes.
anton-vlasenko Jul 17, 2023
a448571
Guard functions and classes.
anton-vlasenko Jul 17, 2023
4c44f1d
Guard functions and classes.
anton-vlasenko Jul 17, 2023
3b3e147
Guard functions and classes.
anton-vlasenko Jul 17, 2023
eee0473
Guard functions and classes.
anton-vlasenko Jul 17, 2023
f9c8746
Guard functions and classes.
anton-vlasenko Jul 17, 2023
4f50af3
Guard functions and classes.
anton-vlasenko Jul 17, 2023
4681443
Guard functions and classes.
anton-vlasenko Jul 17, 2023
73c6aae
Guard functions and classes.
anton-vlasenko Jul 17, 2023
a9d3806
Guard functions and classes.
anton-vlasenko Jul 17, 2023
f3da181
Guard functions and classes.
anton-vlasenko Jul 17, 2023
686d3e0
Guard functions and classes.
anton-vlasenko Jul 17, 2023
51e7ebe
Revert code style changes back.
anton-vlasenko Jul 17, 2023
ad92156
Fix cs errors.
anton-vlasenko Jul 18, 2023
42ec0e9
Move the package to `test/php`.
anton-vlasenko Jul 20, 2023
27dc135
Don't whitelist "WP.+Gutenberg" classes.
anton-vlasenko Jul 20, 2023
a088af5
Update code owners.
anton-vlasenko Jul 20, 2023
4457480
Fix indents.
anton-vlasenko Jul 20, 2023
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions .github/CODEOWNERS
Validating CODEOWNERS rules …
Expand Up @@ -82,6 +82,7 @@
/packages/scripts @gziolo @ntwb @nerrad @ajitbohra @ryanwelcher
/packages/stylelint-config @ntwb
/test/e2e @kevin940726 @Mamaduka
/test/php/gutenberg-coding-standards @anton-vlasenko

# UI Components
/packages/components @ajitbohra
Expand Down
12 changes: 11 additions & 1 deletion composer.json
Expand Up @@ -32,8 +32,18 @@
"wp-coding-standards/wpcs": "^2.2",
"sirbrillig/phpcs-variable-analysis": "^2.8",
"spatie/phpunit-watcher": "^1.23",
"yoast/phpunit-polyfills": "^1.0"
"yoast/phpunit-polyfills": "^1.0",
"gutenberg/gutenberg-coding-standards": "@dev"
},
"repositories": [
{
"type": "path",
"url": "./test/php/gutenberg-coding-standards",
"options": {
"symlink": false
}
}
],
"require": {
"composer/installers": "~1.0"
},
Expand Down
4 changes: 4 additions & 0 deletions lib/class-wp-duotone-gutenberg.php
Expand Up @@ -32,6 +32,10 @@
* @since 6.3.0
*/

if ( class_exists( 'WP_Duotone_Gutenberg' ) ) {
return;
}

/**
* Manages duotone block supports and global styles.
*
Expand Down
4 changes: 4 additions & 0 deletions lib/class-wp-theme-json-data-gutenberg.php
Expand Up @@ -6,6 +6,10 @@
* @since 6.1.0
*/

if ( class_exists( 'WP_Theme_JSON_Data_Gutenberg' ) ) {
return;
}

/**
* Class to provide access to update a theme.json structure.
*/
Expand Down
4 changes: 4 additions & 0 deletions lib/class-wp-theme-json-gutenberg.php
Expand Up @@ -6,6 +6,10 @@
* @since 5.8.0
*/

if ( class_exists( 'WP_Theme_JSON_Gutenberg' ) ) {
return;
}

/**
* Class that encapsulates the processing of structures that adhere to the theme.json spec.
*
Expand Down
4 changes: 4 additions & 0 deletions lib/class-wp-theme-json-resolver-gutenberg.php
Expand Up @@ -6,6 +6,10 @@
* @since 5.8.0
*/

if ( class_exists( 'WP_Theme_JSON_Resolver_Gutenberg' ) ) {
return;
}

/**
* Class that abstracts the processing of the different data sources
* for site-level config and offers an API to work with them.
Expand Down
Expand Up @@ -7,6 +7,10 @@
* @since 6.2.0
*/

if ( class_exists( 'WP_HTML_Attribute_Token' ) ) {
return;
}

/**
* Data structure for the attribute token that allows to drastically improve performance.
*
Expand Down
4 changes: 4 additions & 0 deletions lib/compat/wordpress-6.2/html-api/class-wp-html-span.php
Expand Up @@ -7,6 +7,10 @@
* @since 6.2.0
*/

if ( class_exists( 'WP_HTML_Span' ) ) {
return;
}

/**
* Represents a textual span inside an HTML document.
*
Expand Down
Expand Up @@ -26,6 +26,10 @@
* @since 6.2.0
*/

if ( class_exists( 'WP_HTML_Tag_Processor' ) ) {
return;
}

/**
* Modifies attributes in an HTML document for tags matching a query.
*
Expand Down
Expand Up @@ -7,6 +7,10 @@
* @since 6.2.0
*/

if ( class_exists( 'WP_HTML_Text_Replacement' ) ) {
return;
}

/**
* Data structure used to replace existing content from start to end that allows to drastically improve performance.
*
Expand Down
69 changes: 36 additions & 33 deletions lib/compat/wordpress-6.2/rest-api.php
Expand Up @@ -91,43 +91,46 @@ function gutenberg_modify_rest_sidebars_response( $response ) {
}
add_filter( 'rest_prepare_sidebar', 'gutenberg_modify_rest_sidebars_response' );


/**
* Add the `block_types` value to the `pattern-directory-item` schema.
*
* @since 6.2.0 Added 'block_types' property.
*/
function add_block_pattern_block_types_schema() {
register_rest_field(
'pattern-directory-item',
'block_types',
array(
'schema' => array(
'description' => __( 'The block types which can use this pattern.', 'gutenberg' ),
'type' => 'array',
'uniqueItems' => true,
'items' => array( 'type' => 'string' ),
'context' => array( 'view', 'embed' ),
),
)
);
if ( ! function_exists( 'add_block_pattern_block_types_schema' ) ) {
/**
* Add the `block_types` value to the `pattern-directory-item` schema.
*
* @since 6.2.0 Added 'block_types' property.
*/
function add_block_pattern_block_types_schema() {
register_rest_field(
'pattern-directory-item',
'block_types',
array(
'schema' => array(
'description' => __( 'The block types which can use this pattern.', 'gutenberg' ),
'type' => 'array',
'uniqueItems' => true,
'items' => array( 'type' => 'string' ),
'context' => array( 'view', 'embed' ),
),
)
);
}
}
add_filter( 'rest_api_init', 'add_block_pattern_block_types_schema' );


/**
* Add the `block_types` value into the API response.
*
* @since 6.2.0 Added 'block_types' property.
*
* @param WP_REST_Response $response The response object.
* @param object $raw_pattern The unprepared pattern.
*/
function filter_block_pattern_response( $response, $raw_pattern ) {
$data = $response->get_data();
$data['block_types'] = array_map( 'sanitize_text_field', $raw_pattern->meta->wpop_block_types );
$response->set_data( $data );
return $response;
if ( ! function_exists( 'filter_block_pattern_response' ) ) {
/**
* Add the `block_types` value into the API response.
*
* @since 6.2.0 Added 'block_types' property.
*
* @param WP_REST_Response $response The response object.
* @param object $raw_pattern The unprepared pattern.
*/
function filter_block_pattern_response( $response, $raw_pattern ) {
$data = $response->get_data();
$data['block_types'] = array_map( 'sanitize_text_field', $raw_pattern->meta->wpop_block_types );
$response->set_data( $data );
return $response;
}
}
add_filter( 'rest_prepare_block_pattern', 'filter_block_pattern_response', 10, 2 );

Expand Down
32 changes: 17 additions & 15 deletions lib/compat/wordpress-6.3/kses.php
Expand Up @@ -7,21 +7,23 @@
* @package gutenberg
*/

/**
* Mark CSS safe if it contains grid functions
*
* This function should not be backported to core.
*
* @param bool $allow_css Whether the CSS is allowed.
* @param string $css_test_string The CSS to test.
*/
function allow_grid_functions_in_styles( $allow_css, $css_test_string ) {
if ( preg_match(
'/^grid-template-columns:\s*repeat\([0-9,a-z-\s\(\)]*\)$/',
$css_test_string
) ) {
return true;
if ( ! function_exists( 'allow_grid_functions_in_styles' ) ) {
/**
* Mark CSS safe if it contains grid functions
*
* This function should not be backported to core.
*
* @param bool $allow_css Whether the CSS is allowed.
* @param string $css_test_string The CSS to test.
*/
function allow_grid_functions_in_styles( $allow_css, $css_test_string ) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This one doesn't exist in core, because we patched kses directly. I guess it could be prefixed with gutenberg instead and then it wouldn't need the function_exists check? (there may be other functions in lib/ to which the same applies)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@tellthemachines Hmm, I'm not as knowledgeable as you in the lib/compat stuff.
So, I'm asking myself, could renaming allow_filter_in_styles() to gutenberg_allow_grid_functions_in_styles() break backward compatibility?

Honestly, I would prefer if this is resolved within the scope of another PR, as I'm tryting to keep this PR focused on enforcing CI checks against redeclaraions.

Copy link
Contributor

@azaozz azaozz Jul 19, 2023

Choose a reason for hiding this comment

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

I guess it could be prefixed with gutenberg instead and then it wouldn't need the function_exists check?

Been wondering about that too. Imho as a "golden rule of thumb" it would probably be safer to always do function_exists()/class_exists() in the plugin's code, no matter what! Scrapping WP betas because of old versions of Gutenberg having conflicting function/class names is no fun.

So perhaps absolutely all functions and classes in /lib should always be wrapped in a _exists, no matter what. This could eventually "teach" some contributors about the relation WP <=> plugin too :)

Copy link
Contributor

Choose a reason for hiding this comment

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

That's a good point! I don't think renaming would break back compat because this function only exists in the plugin, and there are no matches other than gutenberg in the wp directory. But yeah, it doesn't really matter and if leaving it as is can have an educational value, I'm happy with that!

if ( preg_match(
'/^grid-template-columns:\s*repeat\([0-9,a-z-\s\(\)]*\)$/',
$css_test_string
) ) {
return true;
}
return $allow_css;
}
return $allow_css;
}
add_filter( 'safecss_filter_attr_allow_css', 'allow_grid_functions_in_styles', 10, 2 );
14 changes: 8 additions & 6 deletions lib/compat/wordpress-6.3/rest-api.php
Expand Up @@ -52,12 +52,13 @@ function gutenberg_update_templates_template_parts_rest_controller( $args, $post
}
add_filter( 'register_post_type_args', 'gutenberg_update_templates_template_parts_rest_controller', 10, 2 );

/**
* Add the `modified` value to the `wp_template` schema.
*
* @since 6.3.0 Added 'modified' property and response value.
*/
function add_modified_wp_template_schema() {
if ( ! function_exists( 'add_modified_wp_template_schema' ) ) {
/**
* Add the `modified` value to the `wp_template` schema.
*
* @since 6.3.0 Added 'modified' property and response value.
*/
function add_modified_wp_template_schema() {
register_rest_field(
array( 'wp_template', 'wp_template_part' ),
'modified',
Expand All @@ -80,6 +81,7 @@ function add_modified_wp_template_schema() {
},
)
);
}
}
add_filter( 'rest_api_init', 'add_modified_wp_template_schema' );

Expand Down
37 changes: 20 additions & 17 deletions lib/compat/wordpress-6.3/theme-previews.php
Expand Up @@ -51,14 +51,15 @@ function gutenberg_attach_theme_preview_middleware() {
);
}

/**
* Temporary function to add a live preview button to block themes.
* Remove when https://core.trac.wordpress.org/ticket/58190 lands.
*/
function add_live_preview_button() {
global $pagenow;
if ( 'themes.php' === $pagenow ) {
?>
if ( ! function_exists( 'add_live_preview_button' ) ) {
/**
* Temporary function to add a live preview button to block themes.
* Remove when https://core.trac.wordpress.org/ticket/58190 lands.
*/
function add_live_preview_button() {
global $pagenow;
if ( 'themes.php' === $pagenow ) {
?>
<script type="text/javascript">
jQuery( document ).ready( function() {
addLivePreviewButton();
Expand Down Expand Up @@ -95,21 +96,23 @@ function addLivePreviewButton() {
});
}
</script>
<?php
<?php
}
}

}

/**
* Adds a nonce for the theme activation link.
*/
function block_theme_activate_nonce() {
$nonce_handle = 'switch-theme_' . gutenberg_get_theme_preview_path();
?>
if ( ! function_exists( 'block_theme_activate_nonce' ) ) {
/**
* Adds a nonce for the theme activation link.
*/
function block_theme_activate_nonce() {
$nonce_handle = 'switch-theme_' . gutenberg_get_theme_preview_path();
?>
<script type="text/javascript">
window.WP_BLOCK_THEME_ACTIVATE_NONCE = '<?php echo wp_create_nonce( $nonce_handle ); ?>';
</script>
<?php
<?php
}
}

/**
Expand Down
4 changes: 4 additions & 0 deletions lib/experimental/class--wp-editors.php
Expand Up @@ -8,6 +8,10 @@

// phpcs:disable PEAR.NamingConventions.ValidClassName.StartWithCapital

if ( class_exists( '_WP_Editors' ) ) {
return;
}

/**
* Placeholder class.
* Used to disable loading of TinyMCE assets.
Expand Down
Expand Up @@ -6,6 +6,10 @@
* @subpackage REST_API
*/

if ( class_exists( 'WP_REST_Block_Editor_Settings_Controller' ) ) {
return;
}

/**
* Core class used to retrieve the block editor settings via the REST API.
*
Expand Down
4 changes: 4 additions & 0 deletions lib/experimental/class-wp-rest-customizer-nonces.php
Expand Up @@ -5,6 +5,10 @@
* @package gutenberg
*/

if ( class_exists( 'WP_Rest_Customizer_Nonces' ) ) {
return;
}

/**
* Class that returns the customizer "save" nonce that's required for the
* batch save operation using the customizer API endpoint.
Expand Down
Expand Up @@ -6,6 +6,10 @@
* @subpackage Interactivity API
*/

if ( class_exists( 'WP_Directive_Context' ) ) {
return;
}

/**
* This is a data structure to hold the current context.
*
Expand Down
Expand Up @@ -6,6 +6,10 @@
* @subpackage Interactivity API
*/

if ( class_exists( 'WP_Directive_Processor' ) ) {
return;
}

/**
* This processor is built on top of the HTML Tag Processor and augments its
* capabilities to process the Interactivity API directives.
Expand Down
Expand Up @@ -9,6 +9,10 @@
* @subpackage Interactivity API
*/

if ( class_exists( 'WP_Interactivity_Store' ) ) {
return;
}

/**
* Manages the initial state of the Interactivity API store in the server and
* its serialization so it can be restored in the browser upon hydration.
Expand Down