Uploaded image for project: 'Moodle'
  1. Moodle
  2. MDL-31167 PHP strict META
  3. MDL-12730

include E_STRICT in DEBUG_DEVELOPER starting in 2.3

    Details

    • Type: Sub-task
    • Status: Closed
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 2.0, 2.2
    • Fix Version/s: 2.3
    • Component/s: Administration
    • Labels:

      Description

      starting with 2.0 only PHP5 supported - yay!
      we sure want to fix all strict errors too, right?

        Gliffy Diagrams

          Issue Links

            Activity

            Hide
            skodak Petr Skoda added a comment -

            hmmmm, not yet...

            Show
            skodak Petr Skoda added a comment - hmmmm, not yet...
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            For reference, some bits happening right now under 2.0.2+ and E_STRICT:

            Strict standards: Declaration of ReferenceExpectation::test() should be compatible with that of SimpleExpectation::test() in /Users/stronk7/git_moodle/moodle/lib/simpletestlib/test_case.php on line 17
             
            Strict standards: Declaration of SimpleImageSubmitTag::write() should be compatible with that of SimpleWidget::write() in /Users/stronk7/git_moodle/moodle/lib/simpletestlib/page.php on line 14
             
            Strict standards: Non-static method PEAR::setErrorHandling() should not be called statically in /Users/stronk7/git_moodle/moodle/lib/formslib.php on line 63
             
            Strict standards: Non-static method HTML_QuickForm::registerElementType() should not be called statically in /Users/stronk7/git_moodle/moodle/lib/formslib.php on line 2337
             
            Strict standards: Non-static method HTML_QuickForm::registerElementType() should not be called statically in /Users/stronk7/git_moodle/moodle/lib/formslib.php on line 2338
             
             
            Strict standards: Non-static method HTML_QuickForm::registerElementType() should not be called statically in /Users/stronk7/git_moodle/moodle/lib/formslib.php on line 2339
             
            Strict standards: Non-static method HTML_QuickForm::registerElementType() should not be called statically in /Users/stronk7/git_moodle/moodle/lib/formslib.php on line 2340
             
            Strict standards: Non-static method HTML_QuickForm::registerElementType() should not be called statically in /Users/stronk7/git_moodle/moodle/lib/formslib.php on line 2341
             
            Strict standards: Non-static method HTML_QuickForm::registerElementType() should not be called statically in /Users/stronk7/git_moodle/moodle/lib/formslib.php on line 2342
            ...
            ...
             
            Strict standards: Declaration of repository_instance_form::validation() should be compatible with that of moodleform::validation() in /Users/stronk7/git_moodle/moodle/repository/lib.php on line 1578
             
            Strict standards: Declaration of repository_local::get_listing() should be compatible with that of repository::get_listing() in /Users/stronk7/git_moodle/moodle/repository/lib.php on line 825
             
            Strict standards: Declaration of repository_recent::type_config_form() should be compatible with that of repository::type_config_form() in /Users/stronk7/git_moodle/moodle/repository/lib.php on line 825
             
            Strict standards: Declaration of auth_plugin_manual::user_confirm() should be compatible with that of auth_plugin_base::user_confirm() in /Users/stronk7/git_moodle/moodle/auth/manual/auth.php on line 3
             
            Strict standards: Creating default object from empty value in /Users/stronk7/git_moodle/moodle/blocks/navigation/block_navigation.php on line 197
             
            Strict standards: Creating default object from empty value in /Users/stronk7/git_moodle/moodle/blocks/settings/block_settings.php on line 132

            So:

            1. One more reason to switch from simpletest to phpunit
            2. formslib, lalala
            3. other minor bits here and there

            Ciao

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - For reference, some bits happening right now under 2.0.2+ and E_STRICT: Strict standards: Declaration of ReferenceExpectation::test() should be compatible with that of SimpleExpectation::test() in /Users/stronk7/git_moodle/moodle/lib/simpletestlib/test_case.php on line 17   Strict standards: Declaration of SimpleImageSubmitTag::write() should be compatible with that of SimpleWidget::write() in /Users/stronk7/git_moodle/moodle/lib/simpletestlib/page.php on line 14   Strict standards: Non-static method PEAR::setErrorHandling() should not be called statically in /Users/stronk7/git_moodle/moodle/lib/formslib.php on line 63   Strict standards: Non-static method HTML_QuickForm::registerElementType() should not be called statically in /Users/stronk7/git_moodle/moodle/lib/formslib.php on line 2337   Strict standards: Non-static method HTML_QuickForm::registerElementType() should not be called statically in /Users/stronk7/git_moodle/moodle/lib/formslib.php on line 2338     Strict standards: Non-static method HTML_QuickForm::registerElementType() should not be called statically in /Users/stronk7/git_moodle/moodle/lib/formslib.php on line 2339   Strict standards: Non-static method HTML_QuickForm::registerElementType() should not be called statically in /Users/stronk7/git_moodle/moodle/lib/formslib.php on line 2340   Strict standards: Non-static method HTML_QuickForm::registerElementType() should not be called statically in /Users/stronk7/git_moodle/moodle/lib/formslib.php on line 2341   Strict standards: Non-static method HTML_QuickForm::registerElementType() should not be called statically in /Users/stronk7/git_moodle/moodle/lib/formslib.php on line 2342 ... ...   Strict standards: Declaration of repository_instance_form::validation() should be compatible with that of moodleform::validation() in /Users/stronk7/git_moodle/moodle/repository/lib.php on line 1578   Strict standards: Declaration of repository_local::get_listing() should be compatible with that of repository::get_listing() in /Users/stronk7/git_moodle/moodle/repository/lib.php on line 825   Strict standards: Declaration of repository_recent::type_config_form() should be compatible with that of repository::type_config_form() in /Users/stronk7/git_moodle/moodle/repository/lib.php on line 825   Strict standards: Declaration of auth_plugin_manual::user_confirm() should be compatible with that of auth_plugin_base::user_confirm() in /Users/stronk7/git_moodle/moodle/auth/manual/auth.php on line 3   Strict standards: Creating default object from empty value in /Users/stronk7/git_moodle/moodle/blocks/navigation/block_navigation.php on line 197   Strict standards: Creating default object from empty value in /Users/stronk7/git_moodle/moodle/blocks/settings/block_settings.php on line 132 So: One more reason to switch from simpletest to phpunit formslib, lalala other minor bits here and there Ciao
            Hide
            gerry Gerard Caulfield added a comment -

            How about creating a higher debug level or an option which displays E_STRICT? If not shouldn't we at least be passing these along to error_log() so that people know of the issue and so that coders are not creating new E_STRICT issues?

            Show
            gerry Gerard Caulfield added a comment - How about creating a higher debug level or an option which displays E_STRICT? If not shouldn't we at least be passing these along to error_log() so that people know of the issue and so that coders are not creating new E_STRICT issues?
            Hide
            skodak Petr Skoda added a comment -

            My current plan is to fix all strict issues in 2.3dev at least because most of them are standard warnings/notices in PHP 5.4. Moodle 2.3dev should imo display all strict problems. thanks for reminding me about this issue, I already forgot it exists...

            Show
            skodak Petr Skoda added a comment - My current plan is to fix all strict issues in 2.3dev at least because most of them are standard warnings/notices in PHP 5.4. Moodle 2.3dev should imo display all strict problems. thanks for reminding me about this issue, I already forgot it exists...
            Hide
            gerry Gerard Caulfield added a comment -

            awesome

            Show
            gerry Gerard Caulfield added a comment - awesome
            Hide
            skodak Petr Skoda added a comment -

            Originally we used numbers because PHP4 did not understand new constants. We use the constants now because future versions of PHP may change their values. The 32768 was originally intended as a "show debugging" flag, but it was later changed to > comparison in debugging(). This comparison is not technically correct, luckily our debug levels are defined in ascending order so that usually works.

            Show
            skodak Petr Skoda added a comment - Originally we used numbers because PHP4 did not understand new constants. We use the constants now because future versions of PHP may change their values. The 32768 was originally intended as a "show debugging" flag, but it was later changed to > comparison in debugging(). This comparison is not technically correct, luckily our debug levels are defined in ascending order so that usually works.
            Hide
            samhemelryk Sam Hemelryk added a comment -

            Hi Petr,

            I've been looking at this just now.
            I've come around to the idea of this, and certainly the sooner we get it in the better.
            However there was one immediate problem that jumped out at me when I started to play with this.
            Because the debug value has changed STRICT notices are not going to be shown for anyone who had turned debugging on prior to this change.
            In order to see those changes they are going to need to edit and re-save their debug setup, or like myself change the value in config.php.

            As said I certainly think this should go in still and will likely integrate shortly anyway so that it is there during today's testing.
            However I think we should consider introducing some upgrade code or at least a notice if the old debug value is being used.
            For reference:

            Debug level Old value New value How it should be set in config.php
            None 0 0 $CFG->debug = 0;
            Minimal 5 5 $CFG->debug = E_ERROR | E_PARSE;
            Normal 15 15 $CFG->debug = E_ERROR | E_PARSE | E_WARNING | E_NOTICE;
            All 6143 30719 $CFG->debug = E_ALL & ~E_STRICT
            Developer 38911 32767 $CFG->debug = E_ALL | E_STRICT

            Cheers
            Sam

            Show
            samhemelryk Sam Hemelryk added a comment - Hi Petr, I've been looking at this just now. I've come around to the idea of this, and certainly the sooner we get it in the better. However there was one immediate problem that jumped out at me when I started to play with this. Because the debug value has changed STRICT notices are not going to be shown for anyone who had turned debugging on prior to this change. In order to see those changes they are going to need to edit and re-save their debug setup, or like myself change the value in config.php. As said I certainly think this should go in still and will likely integrate shortly anyway so that it is there during today's testing. However I think we should consider introducing some upgrade code or at least a notice if the old debug value is being used. For reference: Debug level Old value New value How it should be set in config.php None 0 0 $CFG->debug = 0; Minimal 5 5 $CFG->debug = E_ERROR | E_PARSE; Normal 15 15 $CFG->debug = E_ERROR | E_PARSE | E_WARNING | E_NOTICE; All 6143 30719 $CFG->debug = E_ALL & ~E_STRICT Developer 38911 32767 $CFG->debug = E_ALL | E_STRICT Cheers Sam
            Hide
            skodak Petr Skoda added a comment -

            adding upgrade step, thanks

            Show
            skodak Petr Skoda added a comment - adding upgrade step, thanks
            Hide
            skodak Petr Skoda added a comment -

            reopening, I got some more E_STRICT problem reports, I will create one more E_STRICT patch and then fix this enabler. Ciao

            Show
            skodak Petr Skoda added a comment - reopening, I got some more E_STRICT problem reports, I will create one more E_STRICT patch and then fix this enabler. Ciao
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            Taking on this....

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - Taking on this....
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            Integrated, thanks! (note I've picked, amended and sent together the 2 original commits, observing your authorship, of course).

            I also tested the admin UI and upgrade and seemed to work ok. Also executed all the unit tests and got two failures there @ lib/simpletest/testoutputlib.php that didn't exist before the patch, so I guess we need to look there. Will do after integrating MDL-32095, just in case it includes the fix.

            Ciao

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - Integrated, thanks! (note I've picked, amended and sent together the 2 original commits, observing your authorship, of course). I also tested the admin UI and upgrade and seemed to work ok. Also executed all the unit tests and got two failures there @ lib/simpletest/testoutputlib.php that didn't exist before the patch, so I guess we need to look there. Will do after integrating MDL-32095 , just in case it includes the fix. Ciao
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            As commented above, test passed. Ciao

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - As commented above, test passed. Ciao
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            And this has landed upstream, finally! Yay!

            תודה רבה && شكرا جزيلا



            Closing, ciao

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - And this has landed upstream, finally! Yay! תודה רבה && شكرا جزيلا Closing, ciao
            Hide
            daniss Daniele Cordella added a comment -

            Coooooool! Finally. Thanks.

            Show
            daniss Daniele Cordella added a comment - Coooooool! Finally. Thanks.

              People

              • Votes:
                1 Vote for this issue
                Watchers:
                6 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved:
                  Fix Release Date:
                  25/Jun/12