Skip to content

Allow null and false as standalone types #7546

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.

Sign up for GitHub

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

Merged
merged 4 commits into from
Apr 8, 2022
Merged

Allow null and false as standalone types #7546

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
bf1c3e1
Allow 'null' as a standalone type
Girgias Sep 30, 2021
3ef48bc
Allow false as standalone type
Girgias Feb 20, 2022
f9d5360
Address review comments
Girgias Mar 24, 2022
cec8ad0
Make false|null be represented as ?false
Girgias Apr 7, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions Zend/tests/type_declarations/standalone_null.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
--TEST--
Null can be used as a standalone type
--FILE--
<?php

function test(null $v): null {
return $v;
}

var_dump(test(null));

?>
--EXPECT--
NULL
20 changes: 20 additions & 0 deletions Zend/tests/type_declarations/typed_properties_110.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
--TEST--
Test typed properties allow null
--FILE--
<?php
class Foo {
public null $value;
}

$foo = new Foo();
$foo->value = null;

try {
$foo->value = 1;
} catch (\TypeError $e) {
echo $e->getMessage();
}

?>
--EXPECT--
Cannot assign int to property Foo::$value of type null
20 changes: 20 additions & 0 deletions Zend/tests/type_declarations/typed_properties_111.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
--TEST--
Test typed properties allow false
--FILE--
<?php
class Foo {
public false $value;
}

$foo = new Foo();
$foo->value = false;

try {
$foo->value = true;
} catch (\TypeError $e) {
echo $e->getMessage();
}

?>
--EXPECT--
Cannot assign bool to property Foo::$value of type false
14 changes: 14 additions & 0 deletions Zend/tests/type_declarations/typed_return_null_false_without_value.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
--TEST--
Typed null|false return without value generates compile-time error
--FILE--
<?php

function test() : null|false {
return;
}

test();

?>
--EXPECTF--
Fatal error: A function with return type must return a value (did you mean "return null;" instead of "return;"?) in %s on line %d
14 changes: 14 additions & 0 deletions Zend/tests/type_declarations/typed_return_null_without_value.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
--TEST--
Typed null return without value generates compile-time error
--FILE--
<?php

function test() : null {
return;
}

test();

?>
--EXPECTF--
Fatal error: A function with return type must return a value (did you mean "return null;" instead of "return;"?) in %s on line %d
12 changes: 12 additions & 0 deletions Zend/tests/type_declarations/union_types/null_false_union.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
--TEST--
Null and false can be used in a union type
--FILE--
<?php

function test1(): null|false {}
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: Would be useful and help readability to call these and test the expected behavior, and have example implementations where return null; is properly included in the implementation.

Readers of the RFC might be looking at the tests to see how it's used

Warning: Uncaught TypeError: Return value of test1() must be of the type false or null, none returned....

Copy link
Member Author

Choose a reason for hiding this comment

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

I'll add such a test case

function test2(): false|null {}

?>
===DONE===
--EXPECT--
===DONE===
2 changes: 1 addition & 1 deletion Zend/tests/type_declarations/union_types/redundant_types/nullable_null.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,4 +8,4 @@ function test(): ?null {

?>
--EXPECTF--
Fatal error: Null cannot be used as a standalone type in %s on line %d
Fatal error: null cannot be marked as nullable in %s on line %d
7 changes: 4 additions & 3 deletions Zend/tests/type_declarations/union_types/standalone_false.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
--TEST--
False cannot be used as a standalone type
False can be used as a standalone type
--FILE--
<?php

function test(): false {}

?>
--EXPECTF--
Fatal error: False cannot be used as a standalone type in %s on line %d
===DONE===
--EXPECT--
===DONE===
11 changes: 11 additions & 0 deletions Zend/tests/type_declarations/union_types/standalone_false_implicit_nullability.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
--TEST--
False can be used as a standalone type even with implicit nullability
--FILE--
<?php

function test(false $v = null) {}

?>
===DONE===
--EXPECT--
===DONE===
7 changes: 4 additions & 3 deletions Zend/tests/type_declarations/union_types/standalone_null.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
--TEST--
Null cannot be used as a standalone type
Null can be used as a standalone type
--FILE--
<?php

function test(): null {}

?>
--EXPECTF--
Fatal error: Null cannot be used as a standalone type in %s on line %d
===DONE===
--EXPECT--
===DONE===
7 changes: 4 additions & 3 deletions Zend/tests/type_declarations/union_types/standalone_nullable_false.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
--TEST--
Nullable false cannot be used as a standalone type
Nullable false can be used as a standalone type
--FILE--
<?php

function test(): ?false {}

?>
--EXPECTF--
Fatal error: False cannot be used as a standalone type in %s on line %d
===DONE===
--EXPECT--
===DONE===
30 changes: 13 additions & 17 deletions Zend/zend_compile.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6186,11 +6186,11 @@ static bool zend_type_contains_traversable(zend_type type) {
static zend_type zend_compile_typename(
zend_ast *ast, bool force_allow_null) /* {{{ */
{
bool allow_null = force_allow_null;
bool is_marked_nullable = ast->attr & ZEND_TYPE_NULLABLE;
zend_ast_attr orig_ast_attr = ast->attr;
zend_type type = ZEND_TYPE_INIT_NONE(0);
if (ast->attr & ZEND_TYPE_NULLABLE) {
allow_null = 1;

if (is_marked_nullable) {
ast->attr &= ~ZEND_TYPE_NULLABLE;
}

Expand Down Expand Up @@ -6314,10 +6314,6 @@ static zend_type zend_compile_typename(
type = zend_compile_single_typename(ast);
}

if (allow_null) {
ZEND_TYPE_FULL_MASK(type) |= MAY_BE_NULL;
}

uint32_t type_mask = ZEND_TYPE_PURE_MASK(type);
if ((type_mask & (MAY_BE_ARRAY|MAY_BE_ITERABLE)) == (MAY_BE_ARRAY|MAY_BE_ITERABLE)) {
zend_string *type_str = zend_type_to_string(type);
Expand All @@ -6332,7 +6328,7 @@ static zend_type zend_compile_typename(
ZSTR_VAL(type_str));
}

if (type_mask == MAY_BE_ANY && (orig_ast_attr & ZEND_TYPE_NULLABLE)) {
if (type_mask == MAY_BE_ANY && is_marked_nullable) {
zend_error_noreturn(E_COMPILE_ERROR, "Type mixed cannot be marked as nullable since mixed already includes null");
}

Expand All @@ -6343,6 +6339,15 @@ static zend_type zend_compile_typename(
ZSTR_VAL(type_str));
}

if ((type_mask & MAY_BE_NULL) && is_marked_nullable) {
zend_error_noreturn(E_COMPILE_ERROR, "null cannot be marked as nullable");
}

if (is_marked_nullable || force_allow_null) {
ZEND_TYPE_FULL_MASK(type) |= MAY_BE_NULL;
type_mask = ZEND_TYPE_PURE_MASK(type);
}

if ((type_mask & MAY_BE_VOID) && (ZEND_TYPE_IS_COMPLEX(type) || type_mask != MAY_BE_VOID)) {
zend_error_noreturn(E_COMPILE_ERROR, "Void can only be used as a standalone type");
}
Expand All @@ -6351,15 +6356,6 @@ static zend_type zend_compile_typename(
zend_error_noreturn(E_COMPILE_ERROR, "never can only be used as a standalone type");
}

if ((type_mask & (MAY_BE_NULL|MAY_BE_FALSE))
&& !ZEND_TYPE_IS_COMPLEX(type) && !(type_mask & ~(MAY_BE_NULL|MAY_BE_FALSE))) {
if (type_mask == MAY_BE_NULL) {
zend_error_noreturn(E_COMPILE_ERROR, "Null cannot be used as a standalone type");
} else {
zend_error_noreturn(E_COMPILE_ERROR, "False cannot be used as a standalone type");
}
}

ast->attr = orig_ast_attr;
return type;
}
Expand Down
3 changes: 2 additions & 1 deletion ext/reflection/php_reflection.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1356,6 +1356,7 @@ static void reflection_type_factory(zend_type type, zval *object, bool legacy_be
type_reference *reference;
reflection_type_kind type_kind = get_type_kind(type);
bool is_mixed = ZEND_TYPE_PURE_MASK(type) == MAY_BE_ANY;
bool is_only_null = (ZEND_TYPE_PURE_MASK(type) == MAY_BE_NULL && !ZEND_TYPE_IS_COMPLEX(type));

switch (type_kind) {
case INTERSECTION_TYPE:
Expand All @@ -1373,7 +1374,7 @@ static void reflection_type_factory(zend_type type, zval *object, bool legacy_be
intern = Z_REFLECTION_P(object);
reference = (type_reference*) emalloc(sizeof(type_reference));
reference->type = type;
reference->legacy_behavior = legacy_behavior && type_kind == NAMED_TYPE && !is_mixed;
reference->legacy_behavior = legacy_behavior && type_kind == NAMED_TYPE && !is_mixed && !is_only_null;
intern->ptr = reference;
intern->ref_type = REF_TYPE_TYPE;

Expand Down
4 changes: 4 additions & 0 deletions ext/reflection/tests/ReflectionType_possible_types.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ $functions = [
function(): array {},
function(): callable {},
function(): iterable {},
function(): null {},
function(): false {},
function(): StdClass {}
];

Expand All @@ -30,4 +32,6 @@ string(4) "bool"
string(5) "array"
string(8) "callable"
string(8) "iterable"
string(4) "null"
string(5) "false"
string(8) "StdClass"
19 changes: 19 additions & 0 deletions ext/reflection/tests/union_types.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,15 +13,26 @@ function dumpType(ReflectionUnionType $rt) {
}
}

function dumpBCType(ReflectionNamedType $rt) {
echo "Type $rt:\n";
echo " Name: " . $rt->getName() . "\n";
echo " String: " . (string) $rt . "\n";
echo " Allows Null: " . ($rt->allowsNull() ? "true" : "false") . "\n";
}

function test1(): X|Y|int|float|false|null { }
function test2(): X|iterable|bool { }
function test3(): null|false { }
function test4(): ?false { }

class Test {
public X|Y|int $prop;
}

dumpType((new ReflectionFunction('test1'))->getReturnType());
dumpType((new ReflectionFunction('test2'))->getReturnType());
dumpBCType((new ReflectionFunction('test3'))->getReturnType());
dumpBCType((new ReflectionFunction('test4'))->getReturnType());

$rc = new ReflectionClass(Test::class);
$rp = $rc->getProperty('prop');
Expand Down Expand Up @@ -75,6 +86,14 @@ Allows null: false
Name: bool
String: bool
Allows Null: false
Type ?false:
Name: false
String: ?false
Allows Null: true
Type ?false:
Name: false
String: ?false
Allows Null: true
Type X|Y|int:
Allows null: false
Name: X
Expand Down