Skip to content

Commit

Permalink
Replaces _doing_it_wrong() with trigger_error().
Browse files Browse the repository at this point in the history 
Per Ozz, this is an incorrect usage of doing it wrong.
It should not be used for input validation. Use `trigger_error()`
instead.
  • Loading branch information
@hellofromtonya
hellofromtonya committed Oct 14, 2021
1 parent a0c55e9 commit 1d02ed4
Show file tree
Hide file tree
Showing 4 changed files with 55 additions and 57 deletions.
33 changes: 7 additions & 26 deletions src/wp-includes/webfonts-api/class-wp-webfonts-schema-validator.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -73,11 +73,7 @@ private function is_provider_valid() {
// @todo check if provider is registered.

if ( empty( $this->webfont['provider'] ) || ! is_string( $this->webfont['provider'] ) ) {
_doing_it_wrong(
'register_webfonts',
__( 'Webfont must define a string "provider".' ),
'5.9.0'
);
trigger_error( __( 'Webfont provider must be a non-empty string.' ) );

return false;
}
Expand All @@ -94,11 +90,7 @@ private function is_provider_valid() {
*/
private function is_font_family_valid() {
if ( empty( $this->webfont['fontFamily'] ) || ! is_string( $this->webfont['fontFamily'] ) ) {
_doing_it_wrong(
'register_webfonts',
__( 'Webfont must define a string "fontFamily".' ),
'5.9.0'
);
trigger_error( __( 'Webfont font family must be a non-empty string.' ) );

return false;
}
Expand All @@ -115,25 +107,18 @@ private function is_font_family_valid() {
*/
private function is_font_style_valid() {
if ( empty( $this->webfont['fontStyle'] ) || ! is_string( $this->webfont['fontStyle'] ) ) {
_doing_it_wrong(
'register_webfonts',
__( 'Webfont must define a string "fontStyle".' ),
'5.9.0'
);

trigger_error( __( 'Webfont font style must be a non-empty string.' ) );
return false;
}

if ( ! $this->is_font_style_value_valid( $this->webfont['fontStyle'] ) ) {
_doing_it_wrong(
'register_webfonts',
trigger_error(
sprintf(
/* translators: 1: Slant angle, 2: Given font style. */
/* translators: 1: Slant angle, 2: Given font style. */
__( 'Webfont font style must be normal, italic, oblique, or oblique %1$s. Given: %2$s.' ),
'<angle>',
$this->webfont['fontStyle']
),
'5.9.0'
)
);

return false;
Expand Down Expand Up @@ -170,11 +155,7 @@ private function is_font_style_value_valid( $font_style ) {
private function is_font_weight_valid() {
// @todo validate the value.
if ( empty( $this->webfont['fontWeight'] ) || ! is_string( $this->webfont['fontWeight'] ) ) {
_doing_it_wrong(
'register_webfonts',
__( 'Webfont must define a string "fontWeight".' ),
'5.9.0'
);
trigger_error( __( 'Webfont font weight must be a non-empty string.' ) );

return false;
}
Expand Down
28 changes: 15 additions & 13 deletions tests/phpunit/tests/webfonts-api/wpWebfontsController.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
class Tests_Webfonts_API_wpWebfontsController extends WP_UnitTestCase {
private static $webfonts;

public static function wpSetUpBeforeClass() {
public static function set_up_before_class() {
require_once ABSPATH . WPINC . '/webfonts-api/class-wp-webfonts-schema-validator.php';
require_once ABSPATH . WPINC . '/webfonts-api/class-wp-webfonts-registry.php';
require_once ABSPATH . WPINC . '/webfonts-api/class-wp-webfonts-provider-registry.php';
Expand Down Expand Up @@ -51,19 +51,19 @@ public function test_register_webfonts_with_empty_schema() {
}

/**
* @covers WP_Webfonts_Controller::register_webfonts
* @covers WP_Webfonts_Controller::register_webfont
* @covers WP_Webfonts_Controller::register_webfonts
* @covers WP_Webfonts_Controller::register_webfont
*
* @dataProvider data_register_webfonts_with_invalid_schema
*
* @param array $webfonts Webfonts input.
* @param array $expected Exptected registered webfonts.
* @param array $expected Expected registered webfonts.
*/
public function test_register_webfonts_with_invalid_schema( array $webfonts, array $expected ) {
$controller = $this->get_controller();

$this->setExpectedIncorrectUsage( 'register_webfonts' );
public function test_register_webfonts_with_invalid_schema( array $webfonts, $expected_message, array $expected ) {
$this->expectNotice();
$this->expectNoticeMessage( $expected_message );

$controller = $this->get_controller();
$controller->register_webfonts( $webfonts );

$this->assertSame( $expected, $controller->get_webfonts() );
Expand All @@ -80,7 +80,7 @@ public function test_register_webfonts_with_invalid_schema( array $webfonts, arr
public function data_register_webfonts_with_invalid_schema() {
return array(
'provider: invalid type' => array(
'webfonts' => array(
'webfonts' => array(
array(
'provider' => null,
'fontFamily' => 'Open Sans',
Expand All @@ -94,7 +94,8 @@ public function data_register_webfonts_with_invalid_schema() {
'fontWeight' => '700',
),
),
'expected' => array(
'expected_message' => 'Webfont provider must be a non-empty string.',
'expected' => array(
'open-sans.italic.700' => array(
'provider' => 'google',
'fontFamily' => 'Open Sans',
Expand All @@ -104,7 +105,7 @@ public function data_register_webfonts_with_invalid_schema() {
),
),
'font family: invalid key' => array(
'webfonts' => array(
'webfonts' => array(
array(
'provider' => 'google',
'fontFamily' => 'Roboto',
Expand All @@ -118,7 +119,8 @@ public function data_register_webfonts_with_invalid_schema() {
'fontWeight' => '400',
),
),
'expected' => array(
'expected_message' => 'Webfont font family must be a non-empty string.',
'expected' => array(
'roboto.normal.900' => array(
'provider' => 'google',
'fontFamily' => 'Roboto',
Expand Down Expand Up @@ -191,7 +193,7 @@ public function test_get_webfonts_by_font_family() {
}

/**
* @covers WP_Webfonts_Controller::get_webfonts_by_font_family
* @covers WP_Webfonts_Controller::get_webfonts_by_font_family
*
* @dataProvider data_get_by_font_family_with_invalid_input
*
Expand Down
2 changes: 1 addition & 1 deletion tests/phpunit/tests/webfonts-api/wpWebfontsProviderRegistry.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
*/
class Tests_Webfonts_API_wpWebfontsProviderRegistry extends WP_UnitTestCase {

public static function wpSetUpBeforeClass() {
public static function set_up_before_class() {
require_once ABSPATH . WPINC . '/webfonts-api/class-wp-webfonts-provider-registry.php';
require_once __DIR__ . '/mocks/class-my-custom-webfonts-provider-mock.php';
}
Expand Down
49 changes: 32 additions & 17 deletions tests/phpunit/tests/webfonts-api/wpWebfontsRegistry.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,9 @@ private function get_registry() {
*
* @param array Webfonts input.
*/
public function test_register_with_invalid_schema( array $webfont ) {
$this->setExpectedIncorrectUsage( 'register_webfonts' );
public function test_register_with_invalid_schema( array $webfont, $expected_message ) {
$this->expectNotice();
$this->expectNoticeMessage( $expected_message );

$registry = $this->get_registry();

Expand All @@ -43,102 +44,116 @@ public function test_register_with_invalid_schema( array $webfont ) {
public function data_register_with_invalid_schema() {
return array(
'empty array - no schema' => array(
array(),
'webfont' => array(),
'expected_message' => 'Webfont provider must be a non-empty string.',
),
'provider: not set' => array(
array(
'webfont' => array(
'fontFamily' => 'Open Sans',
'fontStyle' => 'normal',
'fontWeight' => '400',
),
'expected_message' => 'Webfont provider must be a non-empty string.',
),
'provider: empty string' => array(
array(
'webfont' => array(
'provider' => '',
'fontFamily' => 'Open Sans',
'fontStyle' => 'normal',
'fontWeight' => '400',
),
'expected_message' => 'Webfont provider must be a non-empty string.',
),
'provider: invalid type' => array(
array(
'webfont' => array(
'provider' => true,
'fontFamily' => 'Open Sans',
'fontStyle' => 'normal',
'fontWeight' => '400',
),
'expected_message' => 'Webfont provider must be a non-empty string.',
),
'font family: not set' => array(
array(
'webfont' => array(
'provider' => 'local',
'fontStyle' => 'normal',
'fontWeight' => '400',
),
'expected_message' => 'Webfont font family must be a non-empty string.',
),
'font family: empty string' => array(
array(
'webfont' => array(
'provider' => 'local',
'fontFamily' => '',
'fontStyle' => 'normal',
'fontWeight' => '400',
),
'expected_message' => 'Webfont font family must be a non-empty string.',
),
'font family: invalid type' => array(
array(
'webfont' => array(
'provider' => 'local',
'fontFamily' => true,
'fontStyle' => 'normal',
'fontWeight' => '400',
),
'expected_message' => 'Webfont font family must be a non-empty string.',
),
'font style: empty string' => array(
array(
'webfont' => array(
'provider' => 'local',
'fontFamily' => 'Open Sans',
'fontStyle' => '',
'fontWeight' => '400',
),
'expected_message' => 'Webfont font style must be a non-empty string.',
),
'font style: invalid type' => array(
array(
'webfont' => array(
'provider' => 'local',
'fontFamily' => 'Open Sans',
'fontStyle' => true,
'fontWeight' => '400',
),
'expected_message' => 'Webfont font style must be a non-empty string.',
),
'font style: invalid value' => array(
array(
'webfont' => array(
'provider' => 'local',
'fontFamily' => 'Open Sans',
'fontStyle' => 'invalid',
'fontWeight' => '400',
),
'expected_message' => 'Webfont font style must be normal, italic, oblique, or oblique <angle>. Given: invalid.',
),
'font wegith: empty string' => array(
array(
'font weight: empty string' => array(
'webfont' => array(
'provider' => 'local',
'fontFamily' => 'Open Sans',
'fontStyle' => 'normal',
'fontWeight' => '',
),
'expected_message' => 'Webfont font weight must be a non-empty string.',
),
'font weight: invalid type' => array(
array(
'webfont' => array(
'provider' => 'local',
'fontFamily' => 'Open Sans',
'fontStyle' => 'normal',
'fontWeight' => true,
),
'expected_message' => 'Webfont font weight must be a non-empty string.',
),
/* @todo uncomment once value validation is added.
// @todo uncomment once value validation is added.
/*
'font weight: invalid value' => array(
array(
'webfont' => array(
'provider' => 'local',
'fontFamily' => 'Open Sans',
'fontStyle' => 'normal',
'fontWeight' => 'invalid',
),
'expected_message' => 'Webfont font weight must be a non-empty string.',
),
*/
);
Expand Down

0 comments on commit 1d02ed4

Please sign in to comment.