Drupal is now (almost?) E_STRICT complaint. At least the complete test suite now runs without strict warnings (see #348448: Always report E_STRICT errors).

There is an E_STRICT issue being handled in #325827: Avoid PHP strict timezone warnings by calling date_default_timezone_set in bootstrap, but apart from that Drupal core appears to work pretty good with E_STRICT warnings enabled, though there may still be a few problems here and there.

Dries suggests that we raise the error reporting level during the development cycle. This patch sets error_reporting to E_ALL | E_STRICT in default.settings.php. We may want to remove this prior to releasing D7, though I suggest that we leave it to encourage users and contrib module developers to write code as compatible and future proof as possible.

I know this may cause some additional work when porting existing modules to D7. However, these problems are usually easy to fix and are probably only a minor part of upgrading to D7.

I am postponing this until at least #325827: Avoid PHP strict timezone warnings by calling date_default_timezone_set in bootstrap has been fixed.

CommentFileSizeAuthor
#2 strict-error-reporting-2.patch3.01 KBc960657
strict-error-reporting-1.patch1.48 KBc960657

Comments

damien tournoud’s picture

damien tournoud commented
Status: Postponed » Needs work

I support the change to E_STRICT, but http://api.drupal.org/api/function/drupal_initialize_variables/7 is the good place to put it.

c960657’s picture

c960657 commented
StatusFileSize
new strict-error-reporting-2.patch3.01 KB

error_reporting is now set to E_ALL | E_STRICT in drupal_initialize_variables().

dries’s picture

dries commented

This patch no longer applies and will need a re-roll.

damien tournoud’s picture

damien tournoud commented

Anyway, this should be:

error_reporting(E_ALL | E_STRICT | error_reporting());
c960657’s picture

c960657 commented

E_ALL covers all warnings, notices etc. except E_STRICT (and in PHP6 it actually covers all warnings etc. including E_STRICT), so or'ing with error_reporting() doesn't change anything.

mfb’s picture

mfb
they or he
commented

You also need to set E_ALL | E_STRICT in php.ini to catch the compile-time strict errors in core. This mostly involves some incorrect method signatures in the db layer. I patched a few of these over in http://drupal.org/node/325827#comment-1124273 a while back.

Edit: updated patch at #351898: Database API is not E_STRICT compatible.

dave reid’s picture

dave reid
he/him
Primary language English
Location Nebraska USA
commented

We don't need the following segment because we now have an option to select which errors are displayed.

 /**
+ * By default Drupal reports all errors, i.e. E_ALL | E_STRICT. On production
+ * sites you may reduce this to E_ALL. This may also be necessary when using
+ * third-party code that is not E_STRICT compliant.
+ */
+# error_reporting(E_ALL);
mfb’s picture

mfb
they or he
commented
Status: Needs work » Closed (duplicate)

This patch is tiny and not useful on its own so marking as a duplicate of #348448: Always report E_STRICT errors