Moodle

deprecated new instance assignment

Details

  • Type: Sub-task Sub-task
  • Status: Closed Closed
  • Priority: Minor Minor
  • Resolution: Fixed
  • Affects Version/s: 2.0
  • Fix Version/s: 2.0
  • Component/s: Forms Library
  • Labels:
    None
  • Affected Branches:
    MOODLE_20_STABLE
  • Fixed Branches:
    MOODLE_20_STABLE

Description

$x = & new something(); is now deprecated and throws warnings

Activity

Hide
Petr Škoda (skodak) added a comment -

done

Show
Petr Škoda (skodak) added a comment - done
Hide
Eric Bollens added a comment -

Petr, which commits should fix these bad assignments? Checked CVS and it looks like there are definitely some illegal assignments still in the trunk. As far as I can tell, there are ~100 places in Moodle that make illegal old-style assignments.

Are these going to be patched based on changing syntax or using the PHP4/5 dual-compatibility workaround?

The former:

$obj =& new Class($attrs);
$obj = new Class($attrs);

The latter:

function & build(&$new){ return $new; }
$obj =& build(new Class($attrs));

This is an interesting issue that plagues Moodle heavily.

Show
Eric Bollens added a comment - Petr, which commits should fix these bad assignments? Checked CVS and it looks like there are definitely some illegal assignments still in the trunk. As far as I can tell, there are ~100 places in Moodle that make illegal old-style assignments. Are these going to be patched based on changing syntax or using the PHP4/5 dual-compatibility workaround? The former: $obj =& new Class($attrs); $obj = new Class($attrs); The latter: function & build(&$new){ return $new; } $obj =& build(new Class($attrs)); This is an interesting issue that plagues Moodle heavily.
Hide
Martin Dougiamas added a comment -

We don't support PHP 4 in Moodle 2, so we just need to change the syntax. Eric, could you post your list of places and perhaps even a patch?

Show
Martin Dougiamas added a comment - We don't support PHP 4 in Moodle 2, so we just need to change the syntax. Eric, could you post your list of places and perhaps even a patch?
Hide
Martin Dougiamas added a comment -

Added Eric as a watcher

Show
Martin Dougiamas added a comment - Added Eric as a watcher
Hide
Jonathan Harker added a comment -

This issue is not yet resolved - see attached grep from CVS head.

Show
Jonathan Harker added a comment - This issue is not yet resolved - see attached grep from CVS head.
Hide
Jonathan Harker added a comment -

I hope to go through and patch some of these shortly. What should we do where these occur in imported libraries - e.g. PEAR, simplepie, etc.? Perhaps we should fix in place in the meantime, as upgrading to more recent versions which might have fixes might also have new changes/bugs too close to a release.

Show
Jonathan Harker added a comment - I hope to go through and patch some of these shortly. What should we do where these occur in imported libraries - e.g. PEAR, simplepie, etc.? Perhaps we should fix in place in the meantime, as upgrading to more recent versions which might have fixes might also have new changes/bugs too close to a release.
Hide
Martin Dougiamas added a comment -

Am I right in thinking that we just need to drop the "&" from these and it should all work as before?

If so, then +1 to just patch in-place.

Show
Martin Dougiamas added a comment - Am I right in thinking that we just need to drop the "&" from these and it should all work as before? If so, then +1 to just patch in-place.
Hide
Jonathan Harker added a comment -

Yes - see attached. Patch for CVS HEAD

Show
Jonathan Harker added a comment - Yes - see attached. Patch for CVS HEAD
Hide
Martin Dougiamas added a comment -

Awesome!

My only suggestion would be that we should record the changes per library, in the Moodle readme file for each one, to help remember what we need to fix if we update.

I'll leave this for Petr to commit today.

Show
Martin Dougiamas added a comment - Awesome! My only suggestion would be that we should record the changes per library, in the Moodle readme file for each one, to help remember what we need to fix if we update. I'll leave this for Petr to commit today.
Hide
Petr Škoda (skodak) added a comment -

all should be fixed with the exception of pear/xml which should not be used at all now

Show
Petr Škoda (skodak) added a comment - all should be fixed with the exception of pear/xml which should not be used at all now

People

Dates

  • Created:
    Updated:
    Resolved: