Imrpove or remove reporting of missing default timezone

[MarkMaldaba]

Currently, our compatibility sniff emits an error if date.timezone is not set in the php.ini file of the PHP instance on which you are running the tests.

To me, this seems wholly un-useful. It bears no relation to your code (which may be explicitly setting the default timezone, or perhaps doesn't use dates at all) nor to the system on which you will be running the code (unless you are running your tests on your production environment, which is unlikely and inadvisable).

As it stands, this test seems pointless, and I would argue for its removal.

If it is to be retained, then it needs to be a lot more intelligent:

• It should be output at most once, across the set of files being tested.
• It should not be output if no date functions are used.
• It should not be output if date_default_timezone_set() is called at any point (except, perhaps, if there is use of a date function earlier in the same scope)
• It should not be output for date_default_timezone_get() if this call is error-suppressed.
• It should not be checking php.ini settings at all, as they are nothing to do with the code under test.

I've been meaning to log this for a while, but was prompted to do so by a comment in issue #455.

[wimg]

I agree with this and it's something that's been bothering me for a while too. It was added for PHP 5.3's requirement to set one by default. However, since that requirement has disappeared in 7.0, I think we can safely remove this entire sniff, for all of the reasons you mention and many more ;-)

Unless someone knows of a good reason to keep it ?

[MarkMaldaba]

I don't think the PHP 7.0 change should make a difference to the decision. We still want to support earlier PHP versions and I can see the usefulness of reporting this issue if we are able to detect it meaningfully.

For example, if you are building a framework or product that you anticipate running on a wide range of PHP versions, you should take this setting into account and make sure you avoid this error no matter how php.ini is configured. (Most frameworks will use a variant of date_default_timezone_set(@date_default_timezone_get()); or an explicit framework configuration setting for this).

However, the reason I think it should probably be removed is that I am unconvinced that we could meaningfully detect these scenarios.

Or rather, I am certain that we couldn't detect all situations where this error has been avoided - what I am less sure about is whether we could lower the rate of false-positives to a sufficiently low level to make this sniff worth retaining.

And in all cases, it should be based on the code, not on the test server's php.ini.

[wimg]

I'll leave the issue open for a few days to see if anyone has a reason to really keep it.
Personally I agree completely that the test should be based on the code and not on the php.ini settings of the test server. And since we can't enforce that, it would be better to remove it altogether.

[jrfnl]

+1 from me for removal as it is neigh impossible to detect this in a meaningful way as that would require cross-file sniffing which PHPCS is not build to handle and - while it may still be possible to do so in v2.x in a very round-about way (with properties keeping track of files sniffed and code detected) - in v 3.x with multi-threaded sniffing, it would become unmanageable.