PHP 7.1 compatibility #90
Conversation
PHP no longer supports "<?" as an opening tag; make it read "<?php" instead. Signed-off-by: Dan Wells <dbw2@calvin.edu>
$OutputQueries is not static, so we need $this here. Older versions of PHP (<7.0) were more lax about this, particular the way we were mixing contexts. Signed-off-by: Dan Wells <dbw2@calvin.edu>
Our workaround for PHP 7.0 broke on 7.1. This is another workaround, though even more tenuous in nature than the first. Similar to our original goal, we're trying to keep changes to a minimum, not write an ideal and long-term solution. Our old solution stopped working due to changes in PHP 7.1 as expressed by NikiC here: https://stackoverflow.com/questions/47165930/php-binding-method-to-another-class For further reference, here is the commit message from our original PHP 7.0 fix: Archon's object composition technique makes heavy use of a PHP feature allowing method calls from an incompatible context. However, this ability was deprecated in PHP 5.6 and removed from PHP 7.0 [1]. Ultimately, it seems like moving over to Traits may be a logical path forward. This patch instead attempts to allow current Archon to run on PHP 7+ with the fewest changes possible. Rather than simply attempt the method call in the wrong context, we instead create a closure in the desired class, then insert our current context into that closure using bindTo. There may, of course, be a simpler way to do this. In particular, it seems messy to need to create an object only to get the closure. I do not claim to be an expert in PHP internals, but this functions, and is decently boiled down. This new technique should be supported for PHP >= 5.4.0, but this is untested. One questionable area would be the use of the getClosure method call on a parenthesized "new" object statement. I believe it is supported syntax on at least 5.6, and would be very easy to work around in any case. That said, it may be better to retain the old technique and branch within the code to suit the running version. [1] http://php.net/manual/en/migration56.deprecated.php#migration56.deprecated.incompatible-context Signed-off-by: Dan Wells <dbw2@calvin.edu>
The previous commit left the "old" workaround in place in Core_Archon, as the code appeared to work, and we were interested in making fewer changes. However, it appears the above code branch is used exactly once in Archon (the Collections core mixin), and the initialize function there was simply not running. This change gets it to run again as expected. Signed-off-by: Dan Wells <dbw2@calvin.edu>
|
Added new commit to this PR which simply runs the proposed technique in one place we had skipped before. |
|
I tried this out on a PHP 7.3.6 server and got the following errors:
I don't think the notices are related to your code -- those showed up in my own testing independently of your patch. But the last bit is definitely part of this patch. I tried fiddling with the values passed to |
2 participants
This is a replacement for the pull request here:
#82
It has the same goal of getting to a working state with minimal changes. Please see the commit message for more in-depth information.