Skip to content

Commit

Permalink
Move date function group from RestrictedPHPFunctions to `Timezone…
Browse files Browse the repository at this point in the history
…Change`

Follow up on PR 1714 which fixed 1713.

As per the discussion in 1731#discussion_r301840793. moving the check for use of the `date()` to the `TimezoneChange` sniff.

As the `date` function group has not been in a released version (yet), we can still safely make this change without concerns about introducing breaking changes (error code change).

As that sniff is not part of the `Core` ruleset, I'm explicitly adding this particular error code to the `Core` ruleset.

Note: we may want to consider renaming this sniff as the name does not seem to be 100% correct anymore.

Suggestions for a better name welcome. If we go down that road, I'd deprecate the `TimezoneChange` sniff to be removed in the next major release.
  • Loading branch information
jrfnl committed Sep 12, 2019
1 parent ecb633d commit 3337752
Show file tree
Hide file tree
Showing 8 changed files with 34 additions and 16 deletions.
4 changes: 4 additions & 0 deletions WordPress-Core/ruleset.xml
Original file line number Diff line number Diff line change
Expand Up @@ -519,4 +519,8 @@
<!-- Check for correct spelling of WordPress. -->
<rule ref="WordPress.WP.CapitalPDangit"/>

<!-- Use gmdate() not date().
See: https://github.com/WordPress/WordPress-Coding-Standards/issues/1713 -->
<rule ref="WordPress.WP.TimezoneChange.date_date"/>

</ruleset>
4 changes: 3 additions & 1 deletion WordPress-Extra/ruleset.xml
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,9 @@
<rule ref="WordPress.Security.PluginMenuSlug"/>
<rule ref="WordPress.WP.CronInterval"/>
<rule ref="WordPress.WP.PostsPerPage"/>
<rule ref="WordPress.WP.TimezoneChange"/>
<rule ref="WordPress.WP.TimezoneChange">
<severity>5</severity>
</rule>

<!-- Verify some regex best practices.
https://github.com/WordPress/WordPress-Coding-Standards/issues/1371 -->
Expand Down
8 changes: 0 additions & 8 deletions WordPress/Sniffs/PHP/RestrictedPHPFunctionsSniff.php
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
* @package WPCS\WordPressCodingStandards
*
* @since 0.14.0
* @since 2.2.0 New group `date` added.
*/
class RestrictedPHPFunctionsSniff extends AbstractFunctionRestrictionsSniff {

Expand All @@ -43,13 +42,6 @@ public function getGroups() {
'create_function',
),
),
'date' => array(
'type' => 'error',
'message' => '%s() is affected by runtime timezone changes which can cause date/time to be incorrectly displayed. Use gmdate() instead.',
'functions' => array(
'date',
),
),
);
}

Expand Down
26 changes: 23 additions & 3 deletions WordPress/Sniffs/WP/TimezoneChangeSniff.php
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,8 @@
use WordPressCS\WordPress\AbstractFunctionRestrictionsSniff;

/**
* Disallow the changing of timezone.
*
* @link https://vip.wordpress.com/documentation/vip-go/code-review-blockers-warnings-notices/#manipulating-the-timezone-server-side
* Disallow changing the timezone and use of selective date/time functions which lead to
* timezone related bugs.
*
* @package WPCS\WordPressCodingStandards
*
Expand All @@ -23,6 +22,7 @@
* class instead of the upstream `Generic.PHP.ForbiddenFunctions` sniff.
* @since 0.13.0 Class name changed: this class is now namespaced.
* @since 1.0.0 This sniff has been moved from the `VIP` category to the `WP` category.
* @since 2.2.0 New group `date` added.
*/
class TimezoneChangeSniff extends AbstractFunctionRestrictionsSniff {

Expand All @@ -41,13 +41,33 @@ class TimezoneChangeSniff extends AbstractFunctionRestrictionsSniff {
*/
public function getGroups() {
return array(

/*
* Don't change the PHP time zone.
*
* @link https://vip.wordpress.com/documentation/vip-go/code-review-blockers-warnings-notices/#manipulating-the-timezone-server-side
*/
'timezone_change' => array(
'type' => 'error',
'message' => 'Using %s() and similar isn\'t allowed, instead use WP internal timezone support.',
'functions' => array(
'date_default_timezone_set',
),
),

/*
* Use gmdate(), not date().
*
* @link https://core.trac.wordpress.org/ticket/46438
* @link https://github.com/WordPress/WordPress-Coding-Standards/issues/1713
*/
'date' => array(
'type' => 'error',
'message' => '%s() is affected by runtime timezone changes which can cause date/time to be incorrectly displayed. Use gmdate() instead.',
'functions' => array(
'date',
),
),
);
}

Expand Down
3 changes: 0 additions & 3 deletions WordPress/Tests/PHP/RestrictedPHPFunctionsUnitTest.inc
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,3 @@
add_action( 'widgets_init', create_function( '', // Error.
'return register_widget( "time_more_on_time_widget" );'
) );

$post_data['post_title'] = sprintf( __( 'Draft created on %1$s at %2$s' ), date( __( 'F j, Y' ), $now ), date( __( 'g:i a' ), $now ) ); // Error.
$post_data['post_title'] = sprintf( __( 'Draft created on %1$s at %2$s' ), gmdate( __( 'F j, Y' ), $now ), gmdate( __( 'g:i a' ), $now ) ); // OK.
1 change: 0 additions & 1 deletion WordPress/Tests/PHP/RestrictedPHPFunctionsUnitTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ class RestrictedPHPFunctionsUnitTest extends AbstractSniffUnitTest {
public function getErrorList() {
return array(
3 => 1,
7 => 2,
);
}

Expand Down
3 changes: 3 additions & 0 deletions WordPress/Tests/WP/TimezoneChangeUnitTest.inc
Original file line number Diff line number Diff line change
Expand Up @@ -4,3 +4,6 @@ date_default_timezone_set( 'Foo/Bar' ); // Bad.

$date = new DateTime();
$date->setTimezone( new DateTimeZone( 'America/Toronto' ) ); // Yay!

$post_data['post_title'] = sprintf( __( 'Draft created on %1$s at %2$s' ), date( __( 'F j, Y' ), $now ), date( __( 'g:i a' ), $now ) ); // Error.
$post_data['post_title'] = sprintf( __( 'Draft created on %1$s at %2$s' ), gmdate( __( 'F j, Y' ), $now ), gmdate( __( 'g:i a' ), $now ) ); // OK.
1 change: 1 addition & 0 deletions WordPress/Tests/WP/TimezoneChangeUnitTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ class TimezoneChangeUnitTest extends AbstractSniffUnitTest {
public function getErrorList() {
return array(
3 => 1,
8 => 2,
);
}

Expand Down

0 comments on commit 3337752

Please sign in to comment.