new warnings and notices in PHP 5.4 #1

This issue is not supposed to fix all PHP5.4 warnings and notices, I will create a new issue to address the rest later.

"and disable E_STRICT" ?

yes, problems in this patch are standard notices in PHP 5.4RC, previously E_STRICT in PHP 5.3

Integrated (21, 22 and master), thanks!

Note1: Because of 1 typo the the commit message I've integrated this by cherry-picking (instead of merge).

Note2: I've backported it to 21_STABLE (supported), there were 2 conflicts, easy to solve.

Ciao

Got 1 assertion failed in all branches:

filter_delete_config_test.test_filter_delete_all_for_filter 

Expected false, got [Object: of stdClass] at [/lib/simpletest/testfilterconfig.php line 750]

Before the changes it was returning null and curiously, this passes:

$this->assertFalse(null);

Going to review uses (to check that there are no uses relying on false/null) and then fix it to check for empty stdClass() instead.

Using

grep -r '[^>:]get_config([^,]*)' *

revealed 94 uses, grrr. At least:

(array)null === (array)new stdClass() ==> empty array

Looking...

Crap, supercrap, but this is not true:

(bool)null === (bool)new stdClass() ==> false and true

So we cannot change previous behavior without modifying code here and there (the results of the calls are used within conditions.

So, I'm reverting the change in get_config() back to return null. Feel free to create another issue about moving again to stdClass() + review uses in conditions.

Ciao

Done, I've added this to master, 22, and 21:

http://git.moodle.org/gw?p=integration.git;a=commitdiff;h=7fb7e9835adac5bb756d9eb4bfcf307d802bc791

If we really want it to start returning stdClass(), we'll need to review all the uses where it's being evaluated as boolean, changing it to empty($xxxx) or whatever.

But not here and today. For your consideration.

Ciao

Oki, I've verified:

• all unit tests passing.
• install and upgrade working
• login working
• blocks content and configuration working
• filters doing their work
• server-side validation of required and empty texts

So I think this can be considered passed. All the rest of changes were only to get variables initialized before setting their members.

Ciao

Hi, I have looked at the config already, first trying to "fix" all incorrect uses and then decided to return the empty stdClass instead - I did not find any place that would be broken by this change - most of them did already the change of false to something else.

In any case I agree this needs more work and separate review, next week, I am filing new issue.

Well, I started looking for uses sequentially, using the grep above. And in the 2nd/3rd file I found something like:

$config = get_config('xxxx');
if (!$config) {
    ....
    ....

And that evaluates differently if we switch to stdClass(), and that what the reason for the (partial) revert. There are no problems in places casting to array, but any place using if as boolean or checking with empty() will need change for sure.

Just in case it helps looking for the patterns to modify.

Ciao

Yes, I looked at the places that explicitely look for false, but the end result was the same when I used the stdClass in get_config(). Maybe the best way will be to do it in master with full code cleanup and do the backporting only when necessary. In any case thanks a lot!

This is now available in the git and cvs repositories.

Consider the responsibility of your fingerprints engraved there for future generations!

Thanks for the work, closing, ciao

Cherry-picking fixes that use context_system into 2.1 was a pretty bad idea: http://moodle.org/mod/forum/discuss.php?d=194417. I just filed MDL-31330.