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

          Attachments

            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