Docs: Add TimezoneChange XML doc

[GaryJones]

See #1722.

[GaryJones]

The description doesn't match up with the example. The description says WP, but the example is pure PHP!

(Pulled from the unit test file.)

[jrfnl]

I've done some tracing back of this and it looks like the sniff has not had any significant changes (other than sniff technical improvements) since it was first added to the library:

• Issue Flag manipulation of server (default) timezone #76 requested it.
• PR Flag manipulation of server (default) timezone #103 added it.

The issue and the inline docs refer to the following links as relevant:

• https://vip.wordpress.com/documentation/vip-go/code-review-blockers-warnings-notices/#manipulating-the-timezone-server-side
• https://wpvip.com/documentation/vip-go/use-current_time-not-date_default_timezone_set/
• https://weston.ruter.net/2013/04/02/do-not-change-the-default-timezone-from-utc-in-wordpress/

Based on the information there, the more recent issue around the use of gmdate() instead of date() - #1713 - is very much related to this as well.

Now there are two things which are confusing here:

1. The unit test "good" case.
   That case shouldn't really be there as it doesn't test the sniff at all.
   The sniff is not looking for anything in the code of the "good" case, so would never trigger on it.
   It could be replaced with something like My_Class::date_default_timezone_set( 'Foo/Bar' ); which would be testing the proper functioning of the sniff against false positives.
2. The error message refers to "WP internal timezone support" which is an unclear phrase to begin with.
   This is IMO where the docs you are creating come in and can help hugely to clarify what is meant by this, though if someone can come up with a simple phrase to improve the error message, that could be helpful too.
   I think that summarizing the information found in the last two docs links I mention above, to come up with a better description and better code samples would be the way forward.
   I can also imagine some additional issues may need to be opened, sooner rather than later, to get WPCS to detect additional cases of doing it wrong regarding retrieving/displaying date/time information. /cc @Rarst

On a related note - PR #1714 added a check for the use of date() to the WordPress.PHP.RestrictedPHPFunctions sniff.
This change has not yet been released and based on the above, I'm now wondering if we should move that check to this sniff instead.

What do you think ?

[Rarst]

Yeah, gmdate() change is purely to make timezone issue more foolproof, so if it can be here.

Using date_default_timezone_set() and similar isn't allowed, instead use WP internal timezone support.

Now this line is a mess. There is no "similar", there aren't other things that would mess up default time zone. And this doesn't really come up in WordPress context, this comes up when people do PHP in WordPress in incompatible way.

It should be something like this (probably in sniff itself as well?):

Using date_default_timezone_set() isn't allowed, because WP core needs default time zone set to UTC. Use DateTime object or core API functions instead.

PS https://wpvip.com/documentation/vip-go/use-current_time-not-date_default_timezone_set/ this is highly outdated rec, local timestamps are a huge problem (I am close to getting rid of current_time('timestamp') in core) and timezone_string is highly preferable to gmt_offset if available (that mess is about to get abstracted as wp_timezone() in core).

[jrfnl]

@GaryJones Now there is clarity about the future of the sniff - this sniff has been deprecated and the check moved to the WordPress.DateTime.RestrictedFunctions sniff, would you like to update this PR to cover the new WordPress.DateTime.RestrictedFunctions sniff instead ?

[GaryJones]

I'm not going to get to this any time soon - happy for someone else to take it over.

[jrfnl]

Keeping my fingers crossed that there might be someone at contributor day WCUS who'd like to.

[GaryJones]

Noting that I've refreshed the PR on my local branch, but my account is having difficulty pushing to this repo at the moment.

[GaryJones]

Manually copying it here for visibility:

WordPress/Docs/DateTime/RestrictedFunctionsStandard.xml

<documentation title="Restricted Date and Time Functions">
    <standard>
    <![CDATA[
      The restricted functions date_default_timezone_set() and date() should not be used.

      Using date_default_timezone_set() isn't allowed, because WordPress core needs default time zone set to UTC. Use DateTime object or WP core API functions instead.

      Using date() isn't allowed, as it is affected by runtime timezone changes which can cause date/time to be incorrectly displayed. Use gmdate() instead.
    ]]>
    </standard>
    <code_comparison>
        <code title="Valid: Using DateTime object.">
        <![CDATA[
$date = new DateTime();
$date->setTimezone(
    new DateTimeZone( 'Europe/Amsterdam' )
);
        ]]>
        </code>
        <code title="Invalid: Using date_default_timezone_set().">
        <![CDATA[
date_default_timezone_set( 'Europe/Amsterdam' );
        ]]>
        </code>
    </code_comparison>
    <code_comparison>
        <code title="Valid: Using gmdate().">
            <![CDATA[
$last_updated = gmdate(
    'Y-m-d\TH:i:s',
    strtotime( $plugin['last_updated'] )
);
        ]]>
        </code>
        <code title="Invalid: Using date().">
            <![CDATA[
$last_updated = date(
    'Y-m-d\TH:i:s',
    strtotime( $plugin['last_updated'] )
);
        ]]>
        </code>
    </code_comparison>
</documentation>

Commit message:

Docs: Refresh restricted datetime functions docs

Kept the descriptions for individual code comparisons simpler, and populated the <standard> element with more description instead.

With:

./vendor/bin/phpcs --standard=WordPress --generator=text --sniffs=WordPress.DateTime.RestrictedFunctions

You get:

[GaryJones]

Fixed my pushing issue, so now rebased onto develop, and it is now ready for review (again).

[Rarst]

I have no memory of this or mental bandwidth for it, but from quick look at last screenshot I only don't really like gmdate() bit.

It would be a little hard to succinctly document, but gmdate() is kinda "technical" 1:1 replacement, but in many cases code should be written with DateTime anyway. That "wrong" example in practice it should be changed to create from format or something (depending on what's in that input).

[jrfnl]

Verified:

• File is in the right place and uses the correct file name.
• Displays correctly within character limits when displayed on CLI.
• Uses space indentation.
• Code samples comply with WPCS (except for the bit which is being described).
• Missing: Uses <em> tags to emphasize the "good" vs "bad" code.
• The start of the "open" and "close" part of the <![CDATA[ markers do not align.

When looking at the docs, I'm wondering if this is one of those where it would be good to have multiple <standard> and <code comparison> blocks.

At this moment, there is one <standard> block at the top with two associated code comparisons, but parts of the text in the <standard> block apply specifically to each code sample, so I'm wondering if a structure like the below would work better ?

<standard>Generic info about the sniff</standard>
<standard>Specific info about the date_default_timezone_set() restriction</standard>
<code_comparison>date_default_timezone_set() code sample</code_comparison>
<standard>Specific info about the date() restriction</standard>
<code_comparison>date() code sample</code_comparison>

[jrfnl]

Suggested change

	Using date_default_timezone_set() isn't allowed, because WordPress core needs default time zone set to UTC. Use DateTime object or WP core API functions instead.
	Using the PHP native date_default_timezone_set() function isn't allowed, because WordPress Core needs the default time zone to be set to UTC for timezone calculations using the WP Core API to work correctly.
	Use a DateTime object or use the WP Core API functions instead.

[jrfnl]

Suggested change

	Using date() isn't allowed, as it is affected by runtime timezone changes which can cause date/time to be incorrectly displayed. Use gmdate() instead.
	Using the PHP native date() function isn't allowed, as it is affected by runtime timezone changes which can cause the date/time to be incorrectly displayed. Use gmdate() instead.

[jrfnl]

Would it be an idea to "WordPressify" this code sample a little more and use something like get_option() with gmt_offset or timezone_string to retrieve the timezone to be passed to new DateTimeZone() ?

[Rarst]

I think the basic PHP example is fine there, people tend to change default time zone to something that is not normal WP time zone.

Again, there is a problem of choice between a technical 1:1 code equivalent and idiomatic rewrite, that is hard to decide on for brief out-of-context example.

[jrfnl]

Thanks for the update @GaryJones ! Looks all good to me! 💯