Update Core to comply to coding standards#8
Conversation
phpcs.xml.dist
Outdated
There was a problem hiding this comment.
The above 3 line comment should be removed, or updated to reference a suggested command line example, e.g. phpcbf --standard=phpcs.xml.dist --report-summary --report-source
| 'https://github.com/WordPress/gutenberg' ); ?></p> | ||
| 'https://github.com/WordPress/gutenberg' | ||
| ); | ||
| ?> |
There was a problem hiding this comment.
The indent doesn't seem right here.
| exit; | ||
|
|
||
| /** | ||
| /** |
There was a problem hiding this comment.
| 'timeout' => 120, | ||
| 'httpversion' => '1.1', | ||
| ) | ||
| ); |
There was a problem hiding this comment.
What rule are we following here? Should this be more like this:
$response = wp_remote_get(
admin_url( 'upgrade.php?step=1' ),
array(
'timeout' => 120,
'httpversion' => '1.1',
)
);There was a problem hiding this comment.
Yeah, that looks wrong. Can you report it to the WPCS repo?
There was a problem hiding this comment.
I can, but I wasn't sure if that's a rule we have because it's all over the code base now. Or should I create one for discussion first?
There was a problem hiding this comment.
If it doesn't look right, it's probably a bug. ;)
| $id, array( | ||
| 'send' => false, | ||
| 'delete' => true, | ||
| ) |
There was a problem hiding this comment.
See above, another example:
echo get_media_item(
$id,
array(
'send' => false,
'delete' => true,
)
);
src/wp-admin/edit-tag-form.php
Outdated
There was a problem hiding this comment.
This probably needs manual fixing, there's actually a space before the tab but now it's after the tab. 🤔
There was a problem hiding this comment.
| case 'pages': $('#page-filters').slideDown(); break; | ||
| } | ||
| }); | ||
| }); |
There was a problem hiding this comment.
Same as above, the extra whitespaces don't get removed, just moved after the tab.
| <?php | ||
| /* translators: %s: importer slug */ | ||
| printf( __( 'The %s importer is invalid or is not installed.' ), '<strong>' . esc_html( $_GET['invalid'] ) . '</strong>' ); | ||
| ?> |
tools/i18n/extract.php
Outdated
There was a problem hiding this comment.
Not sure if we have to update tools/i18n/ here too. 🤷♂️
Did you run the tests manually?
There was a problem hiding this comment.
That's the expected behaviour, but it looks a bit weird. Move the $comment_prefix declaration above $rules, or add a blank line between them.
Run what tests manually?
There was a problem hiding this comment.
I meant the whole tools/i18n directory.
The tests in https://core.trac.wordpress.org/browser/trunk/tools/i18n/t.
There was a problem hiding this comment.
Given that I've never opened that directory before... no. Why doesn't it run in Travis?
There was a problem hiding this comment.
I was never a fan of having that directory on core.trac at all. For me the canonical source is still https://i18n.trac.wordpress.org/browser/tools.
pento
force-pushed
the
41057-coding-standards
branch
from
313b7c3 to
7d9dc4f
Compare
…site-icon Custom properties, site-icon file
…WP_Scripts::localize()`.
This function was previously already problematic as it does not do proper input validation, and it has already received tweaks related to PHP 8.0 in [50408] / #52534, which also introduced a `_doing_it_wrong()` notice and added tests.
The short of it is:
* The function expects to receive an `array` for the `$l10n` parameter;
* ...but silently supported the parameter being passed as a `string`;
* ...and would expect PHP to gracefully handle everything else or throw appropriate warnings/errors.
In the previous fix, a `_doing_it_wrong()` notice was added for any non-array inputs. The function would also cause a PHP native "Cannot use a scalar value as an array" warning (PHP < 8.0) or error (PHP 8.0+) for all scalar values, except `false`.
PHP 8.1 deprecated autovivification from `false` to `array`, so now `false` starts throwing an "Automatic conversion of false to array is deprecated" notice.
By rights, the function should just throw an exception when a non-array/string input is received, but that would be a backward compatibility break.
So the current change will maintain the previous behavior, but will prevent both the "Cannot use a scalar value as an array" warning/error as well as the "Automatic conversion of false to array" deprecation notice for invalid inputs.
Invalid inputs ''will'' still receive a `_doing_it_wrong()` notice, which is the reason this fix is considered acceptable.
Includes:
* Adding a test passing an empty array.
* Adding a test to the data provider for a `null` input to make sure that the function will not throw a PHP 8.1 "passing null to non-nullable" notice.
This solves the following PHP 8.1 test error:
{{{
Tests_Dependencies_Scripts::test_wp_localize_script_data_formats with data set #8 (false, '[""]')
Automatic conversion of false to array is deprecated
/var/www/src/wp-includes/class.wp-scripts.php:514
/var/www/src/wp-includes/functions.wp-scripts.php:221
/var/www/tests/phpunit/tests/dependencies/scripts.php:1447
/var/www/vendor/bin/phpunit:123
}}}
Reference: [https://www.php.net/manual/en/migration81.deprecated.php#migration81.deprecated.core.autovivification-false PHP Manual: PHP 8.1 Deprecations: Autovivification from false].
Follow-up to [7970], [18464], [18490], [19217], [50408].
Props jrf, costdev.
See #55656.
git-svn-id: https://develop.svn.wordpress.org/trunk@54142 602fd350-edb4-49c9-b593-d223f7449a82
…updates Test updates.
4 participants
See the Trac ticket.