-
-
Notifications
You must be signed in to change notification settings - Fork 188
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
TASK: Add basic support for PHP8 #2287
Changes from 31 commits
0326ac6
7093af5
5169338
9077b6f
7561613
3cfe0c9
e083f68
d84c96b
0da755f
c6eae50
d39a5c3
fb76c96
dc19a84
b050c36
754379f
1aaa37b
d70ba63
310b5c0
094d325
9bc2700
e85f76b
be4725e
9c84381
3ac08f0
d31db02
b985f37
b2c60da
a417883
b94e684
b61942b
6d3c5ca
23f4bc3
82d5c85
c5268c8
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -405,7 +405,7 @@ public function requireOnceDoesNotSwallowPhpWarningsOfTheIncludedFile() | |
$entryIdentifier = 'SomePhpEntryWithPhpWarning'; | ||
|
||
$simpleFileBackend = $this->getSimpleFileBackend(); | ||
$simpleFileBackend->set($entryIdentifier, '<?php trigger_error("Warning!", E_WARNING); ?>'); | ||
$simpleFileBackend->set($entryIdentifier, '<?php trigger_error("Warning!", E_USER_WARNING); ?>'); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Only |
||
$simpleFileBackend->requireOnce($entryIdentifier); | ||
} | ||
|
||
|
@@ -418,7 +418,7 @@ public function requireOnceDoesNotSwallowPhpNoticesOfTheIncludedFile() | |
$entryIdentifier = 'SomePhpEntryWithPhpNotice'; | ||
|
||
$simpleFileBackend = $this->getSimpleFileBackend(); | ||
$simpleFileBackend->set($entryIdentifier, '<?php $undefined ++; ?>'); | ||
$simpleFileBackend->set($entryIdentifier, '<?php trigger_error("Notice!", E_USER_NOTICE); ?>'); | ||
$simpleFileBackend->requireOnce($entryIdentifier); | ||
} | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -407,7 +407,7 @@ public function round($subject, $precision = 0) | |
if (!is_numeric($subject)) { | ||
return NAN; | ||
} | ||
$subject = floatval($subject); | ||
$subject = (float)$subject; | ||
if ($precision != null && !is_int($precision)) { | ||
return NAN; | ||
} | ||
|
@@ -422,14 +422,14 @@ public function round($subject, $precision = 0) | |
*/ | ||
public function sign($x) | ||
{ | ||
if ($x < 0) { | ||
if (!is_numeric($x)) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In PHP8 'foo' compares differently to a number: see https://3v4l.org/Zhdsh |
||
return NAN; | ||
} elseif ($x < 0) { | ||
return -1; | ||
} elseif ($x > 0) { | ||
return 1; | ||
} elseif ($x === 0 || $x === 0.0) { | ||
return 0; | ||
} else { | ||
return NAN; | ||
return 0; | ||
} | ||
} | ||
|
||
|
@@ -492,9 +492,9 @@ public function trunc($x) | |
$sign = $this->sign($x); | ||
switch ($sign) { | ||
case -1: | ||
return (int)ceil($x); | ||
return (int)ceil((float)$x); | ||
case 1: | ||
return (int)floor($x); | ||
return (int)floor((float)$x); | ||
default: | ||
return $sign; | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -95,15 +95,15 @@ public function getDescription() | |
} | ||
|
||
/** | ||
* @return string The name of a type (e.g. string, \stdClass) if it was declared as a return type, null otherwise | ||
* @return string|null The name of a type (e.g. string, \stdClass) if it was declared as a return type, null otherwise | ||
*/ | ||
public function getDeclaredReturnType() | ||
{ | ||
if (!is_callable([$this, 'getReturnType'])) { | ||
return null; | ||
} | ||
$type = $this->getReturnType(); | ||
return $type !== null ? (string)$type : null; | ||
return $type !== null ? ltrim((string)$type, '?') : null; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. PHP8 returns the nullable type operator |
||
} | ||
|
||
/** | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -52,14 +52,14 @@ public function getClass() | |
} | ||
|
||
/** | ||
* @return string The name of a builtin type (e.g. string, int) if it was declared for the parameter (scalar type declaration), null otherwise | ||
* @return string|null The name of a builtin type (e.g. string, int) if it was declared for the parameter (scalar type declaration), null otherwise | ||
*/ | ||
public function getBuiltinType() | ||
{ | ||
$type = $this->getType(); | ||
if ($type === null || !$type->isBuiltin()) { | ||
if (!$type instanceof \ReflectionNamedType) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could be a |
||
return null; | ||
} | ||
return $type instanceof \ReflectionNamedType ? $type->getName() : (string)$type; | ||
return $type->isBuiltin() ? $type->getName() : null; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1788,9 +1788,6 @@ protected function convertParameterReflectionToArray(ParameterReflection $parame | |
if ($parameter->isPassedByReference()) { | ||
$parameterInformation[self::DATA_PARAMETER_BY_REFERENCE] = true; | ||
} | ||
if ($parameter->isArray()) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
$parameterInformation[self::DATA_PARAMETER_ARRAY] = true; | ||
} | ||
if ($parameter->isOptional()) { | ||
$parameterInformation[self::DATA_PARAMETER_OPTIONAL] = true; | ||
} | ||
|
@@ -1800,11 +1797,20 @@ protected function convertParameterReflectionToArray(ParameterReflection $parame | |
|
||
$parameterType = $parameter->getType(); | ||
if ($parameterType !== null) { | ||
// TODO: This needs to handle ReflectionUnionType | ||
$parameterType = ($parameterType instanceof \ReflectionNamedType) ? $parameterType->getName() : $parameterType->__toString(); | ||
} | ||
if ($parameter->getClass() !== null) { | ||
if ($parameterType !== null && !TypeHandling::isSimpleType($parameterType)) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
// We use parameter type here to make class_alias usage work and return the hinted class name instead of the alias | ||
$parameterInformation[self::DATA_PARAMETER_CLASS] = $parameterType; | ||
} elseif ($parameterType === 'array') { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Else here makes sure that |
||
$parameterInformation[self::DATA_PARAMETER_ARRAY] = true; | ||
albe marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} else { | ||
$builtinType = $parameter->getBuiltinType(); | ||
if ($builtinType !== null) { | ||
$parameterInformation[self::DATA_PARAMETER_TYPE] = $builtinType; | ||
$parameterInformation[self::DATA_PARAMETER_SCALAR_DECLARATION] = true; | ||
} | ||
} | ||
if ($parameter->isOptional() && $parameter->isDefaultValueAvailable()) { | ||
$parameterInformation[self::DATA_PARAMETER_DEFAULT_VALUE] = $parameter->getDefaultValue(); | ||
|
@@ -1816,17 +1822,10 @@ protected function convertParameterReflectionToArray(ParameterReflection $parame | |
$parameterType = $this->expandType($method->getDeclaringClass(), $explodedParameters[0]); | ||
} | ||
} | ||
if (!$parameter->isArray()) { | ||
$builtinType = $parameter->getBuiltinType(); | ||
if ($builtinType !== null) { | ||
$parameterInformation[self::DATA_PARAMETER_TYPE] = $builtinType; | ||
$parameterInformation[self::DATA_PARAMETER_SCALAR_DECLARATION] = true; | ||
} | ||
} | ||
if (!isset($parameterInformation[self::DATA_PARAMETER_TYPE]) && $parameterType !== null) { | ||
$parameterInformation[self::DATA_PARAMETER_TYPE] = $this->cleanClassName($parameterType); | ||
} elseif (!isset($parameterInformation[self::DATA_PARAMETER_TYPE])) { | ||
$parameterInformation[self::DATA_PARAMETER_TYPE] = $parameter->isArray() ? 'array' : 'mixed'; | ||
$parameterInformation[self::DATA_PARAMETER_TYPE] = 'mixed'; | ||
} | ||
|
||
return $parameterInformation; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -149,7 +149,7 @@ public function dispatch(string $signalClassName, string $signalName, array $sig | |
} | ||
|
||
foreach ($this->slots[$signalClassName][$signalName] as $slotInformation) { | ||
$finalSignalArguments = $signalArguments; | ||
$finalSignalArguments = array_values($signalArguments); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. PHP8 named parameters... |
||
if (isset($slotInformation['object'])) { | ||
$object = $slotInformation['object']; | ||
} elseif (strpos($slotInformation['method'], '::') === 0) { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -82,8 +82,14 @@ public function extractNamespace(): ?string | |
break; | ||
} | ||
list($type, $value) = $token; | ||
if (defined('T_NAME_QUALIFIED') && $type === T_NAME_QUALIFIED) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Tokenization of namespaces changed in PHP8 and now provides |
||
return $value; | ||
} | ||
if (defined('T_NAME_FULLY_QUALIFIED') && $type === T_NAME_FULLY_QUALIFIED) { | ||
return ltrim($value, '\\'); | ||
} | ||
if ($type === T_STRING) { | ||
$namespaceParts[] = $value; | ||
$namespaceParts[] = ltrim($value, '\\'); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not 100% sure any more this is required, because in PHP8 namespace is parsed with above tokens. But we always expect namespaces to not start with |
||
continue; | ||
} | ||
if ($type !== T_NS_SEPARATOR && $type !== T_WHITESPACE) { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -429,7 +429,6 @@ public function argumentTestsDataProvider() | |
'required date string' => ['requiredDate', '1980-12-13T14:22:12+02:00', '1980-12-13'], | ||
'required date - missing value' => ['requiredDate', null, $requiredArgumentExceptionText], | ||
'required date - mapping error' => ['requiredDate', 'no date', 'Validation failed while trying to call Neos\Flow\Tests\Functional\Mvc\Fixtures\Controller\ActionControllerTestBController->requiredDateAction().'], | ||
'required date - empty value' => ['requiredDate', '', 'Uncaught Exception in Flow Argument 1 passed to Neos\Flow\Tests\Functional\Mvc\Fixtures\Controller\ActionControllerTestBController_Original::requiredDateAction() must be an instance of DateTime, null given'], | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The exception text changed in PHP8, so needs to be tested depending on PHP version => see below |
||
'optional date string' => ['optionalDate', '1980-12-13T14:22:12+02:00', '1980-12-13'], | ||
'optional date - default value' => ['optionalDate', null, 'null'], | ||
'optional date - mapping error' => ['optionalDate', 'no date', 'Validation failed while trying to call Neos\Flow\Tests\Functional\Mvc\Fixtures\Controller\ActionControllerTestBController->optionalDateAction().'], | ||
|
@@ -460,6 +459,25 @@ public function argumentTests($action, $argument, $expectedResult) | |
self::assertTrue(strpos(trim($response->getBody()->getContents()), (string)$expectedResult) === 0, sprintf('The resulting string did not start with the expected string. Expected: "%s", Actual: "%s"', $expectedResult, $response->getBody()->getContents())); | ||
} | ||
|
||
/** | ||
* @test | ||
*/ | ||
public function requiredDateNullArgumentTest() | ||
{ | ||
$arguments = [ | ||
'argument' => '', | ||
]; | ||
|
||
$uri = str_replace('{@action}', 'requireddate', 'http://localhost/test/mvc/actioncontrollertestb/{@action}'); | ||
$response = $this->browser->request($uri, 'POST', $arguments); | ||
if (PHP_MAJOR_VERSION < 8) { | ||
$expectedResult = 'Uncaught Exception in Flow Argument 1 passed to Neos\Flow\Tests\Functional\Mvc\Fixtures\Controller\ActionControllerTestBController_Original::requiredDateAction() must be an instance of DateTime, null given'; | ||
} else { | ||
$expectedResult = 'Uncaught Exception in Flow Neos\Flow\Tests\Functional\Mvc\Fixtures\Controller\ActionControllerTestBController_Original::requiredDateAction(): Argument #1 ($argument) must be of type DateTime, null given'; | ||
} | ||
self::assertTrue(strpos(trim($response->getBody()->getContents()), (string)$expectedResult) === 0, sprintf('The resulting string did not start with the expected string. Expected: "%s", Actual: "%s"', $expectedResult, $response->getBody()->getContents())); | ||
} | ||
|
||
/** | ||
* @test | ||
*/ | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is no longer a E_NOTICE in PHP8