Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PHP 7.2 support #10

Merged
merged 9 commits into from
Jan 15, 2018
Merged

PHP 7.2 support #10

merged 9 commits into from
Jan 15, 2018

Conversation

jaydiablo
Copy link
Member

This removes any cases of each() from ZF for PHP 7.2 compatibility.

Most of the changes come from Bukashk0zzz:php-7.2 branch, but then modified to make tests pass (a couple of the changes modified behavior of the code, causing a few tests to fail).

This PR should replace #9.

All tests pass on my local machine, curious how travis will do, especially the 7.2 run.

@@ -289,8 +289,10 @@ protected static function _decodeYaml($currentIndent, &$lines)
{
$config = array();
$inIndent = false;
while (list($n, $line) = each($lines)) {
$lineno = $n + 1;
while (key($lines) !== null) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A simple foreach was not possible here because the position of the array pointer is important (both internal to this function (due to the prev used below) and external to this function (since $lines is passed by reference)).

@@ -363,10 +365,10 @@ protected static function _parseValue($value)

// remove quotes from string.
if ('"' == $value['0']) {
if ('"' == $value[count($value) -1]) {
if ('"' == $value[strlen($value) -1]) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is probably a long-standing bug, since count in this case would always return 1 on a string, so this would always be $value[0] which is the same as the condition above.

// Coerce the values to an array.
// Don't simply typecast to array, because the values
// might be Zend_Db_Expr objects.
if (!is_array($keyValues)) {
$keyValues = array($keyValues);
}
$keyValuesCount = count($keyValues);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seemed safe to move this down a bit, so count is always executed on an array.

@@ -201,7 +201,7 @@ public function getAuthors()
);
}

if (count($authors) == 0) {
if ($authors !== null && count($authors) == 0) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the tests $authors would sometimes be null here, which count returns 0 on.

@@ -47,7 +47,7 @@
* @copyright Copyright (c) 2005-2015 Zend Technologies USA Inc. (http://www.zend.com)
* @license http://framework.zend.com/license/new-bsd New BSD License
*/
class Zend_Mail_Part implements RecursiveIterator, Zend_Mail_Part_Interface
class Zend_Mail_Part implements RecursiveIterator, Zend_Mail_Part_Interface, Countable
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The tests run count on this object, so added the Countable interface. It doesn't seem to happen in the code base at all otherwise.

The specific test doing this was counting the object to make sure there were no parts.

@@ -51,8 +51,8 @@ public function testDebugDump()
$data = 'string';
$result = Zend_Debug::Dump($data, null, false);
$result = str_replace(array(PHP_EOL, "\n"), '_', $result);
$expected = "__string(6) \"string\"__";
$this->assertEquals($expected, $result);
$expected = "__.*string\(6\) \"string\"__";
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With newer versions of Xdebug, the full file path and line number is in this output. Using a regex condition to ignore that bit if present, but still check for the other strings.

$callback = create_function('$value, $options',
'return (isset($options["bar"]["quo"]["foo"]) &&
"foo Value" === $options["bar"]["quo"]["foo"]);');
$callback = function ($value, $options) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

create_function is deprecated.

@@ -54,7 +54,7 @@ class Zend_Queue_Stomp_Connection_Mock
* @param array $config ('scheme', 'host', 'port')
* @return true;
*/
public function open($scheme, $host, $port)
public function open($scheme, $host, $port, array $options = array())
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Signature here didn't match the object it's mocking.

@@ -85,7 +85,8 @@ function setUp()
*/
public function tearDown()
{
ini_set('session.save_path', $this->_savePath);
session_abort();
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The lines added here stop the session so that we can change the session save_path (php 7.2 emits a warning that this can't be done while the session is active).

@@ -1091,6 +1095,8 @@ public function testInvalidPreexistingSessionIdDoesNotPreventRegenerationOfSid()
@mkdir($sessionStore . DIRECTORY_SEPARATOR . $subdir);
}

session_start();
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This particular test requires that there be an existing session running, but due to the changes in the tearDown method above, this wasn't the case any longer. Forcing a start here (after the save_path is changed and the directory it's going to write to is created).

@aripringle aripringle merged commit ba8cf7a into master Jan 15, 2018
@aripringle aripringle deleted the php-7-2-fixes branch January 15, 2018 20:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants