Skip to content
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,9 @@ class A {
public function method2(X $a): X&Y {
return new TestParent();
}
public function method3(?X&Y $a): ?X&Y {
return $a;
}
}

$tp = new TestParent();
Expand All @@ -42,7 +45,10 @@ $r = $o->method1($tc);
var_dump($r);
$r = $o->method2($tc);
var_dump($r);

$r = $o->method3($tc);
var_dump($r);
$r = $o->method3(null);
var_dump($r);

?>
--EXPECTF--
Expand All @@ -55,3 +61,6 @@ object(TestChild)#%d (0) {
}
object(TestParent)#%d (0) {
}
object(TestChild)#%d (0) {
}
NULL
4 changes: 2 additions & 2 deletions Zend/tests/type_declarations/intersection_types/bug81268.phpt
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
--TEST--
Bug #81268 Wrong message when using null as a default value for intersection types
Bug #81268 Message when using null as a default value for intersection types
--FILE--
<?php

Expand All @@ -9,4 +9,4 @@ class Test {

?>
--EXPECTF--
Fatal error: Cannot use null as default value for property Test::$y of type X&Y in %s on line %d
Fatal error: Default value for property of type X&Y may not be null. Use the nullable type ?X&Y to allow null default value in %s on line %d
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
--TEST--
Intersection types cannot be implicitly nullable
Intersection types can be implicitly nullable as the others
--FILE--
<?php

function foo(X&Y $foo = null) {}
function foo(X&Y $foo = null) { return $foo; }

var_dump(foo());

?>
--EXPECTF--
Fatal error: Cannot use null as default value for parameter $foo of type X&Y in %s on line %d
--EXPECT--
NULL

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,11 @@ interface B {}
class Foo implements A, B {}
class Bar implements A {}

function foo(A&B $bar) {
function foo(?A&B $bar) {
var_dump($bar);
}

foo(null);
foo(new Foo());

try {
Expand All @@ -23,6 +24,7 @@ try {

?>
--EXPECTF--
NULL
object(Foo)#1 (0) {
}
foo(): Argument #1 ($bar) must be of type A&B, Bar given, called in %s on line %d
foo(): Argument #1 ($bar) must be of type ?A&B, Bar given, called in %s on line %d
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
--TEST--
Intersection types and typed reference
--FILE--
<?php

interface X {}
interface Y {}
interface Z {}

class A implements X, Y, Z {}
class B implements X, Y {}

class Test {
public ?X&Y $y;
public X&Z $z;
}
$test = new Test;
$r = new A;
$test->y =& $r;
$test->z =& $r;


try {
$r = null;
} catch (\TypeError $e) {
echo $e->getMessage(), \PHP_EOL;
}

?>
--EXPECT--
Cannot assign null to reference held by property Test::$z of type X&Z
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
--TEST--
Intersection types and typed reference
--FILE--
<?php

interface X {}
interface Y {}
interface Z {}

class A implements X, Y, Z {}
class B implements X, Y {}

class Test {
public ?X&Y $y;
public ?X&Z $z;
}
$test = new Test;
$r = new A;
$test->y =& $r;
$test->z =& $r;

$r = null;

?>
==DONE==
--EXPECT--
==DONE==
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
--TEST--
Invalid nullable widening
--FILE--
<?php

interface A {}
interface B {}
interface C {}

class Test implements A, B, C {}

class Foo {
public function foo(): A {
return new Test();
}
}

class FooChild extends Foo {
public function foo(): A&B {
return new Test();
}
}

class FooSecondChild extends FooChild {
public function foo(): ?A&B {
return new Test();
}
}

?>
--EXPECTF--
Fatal error: Declaration of FooSecondChild::foo(): ?A&B must be compatible with FooChild::foo(): A&B in %s on line %d
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
--TEST--
Intersection type removing nullable
--FILE--
<?php

class A {}
class B extends A {}

class Test {
public ?A&B $prop;
}
class Test2 extends Test {
public A&B $prop;
}

?>
--EXPECTF--
Fatal error: Type of Test2::$prop must be ?A&B (as in class Test) in %s on line %d
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
--TEST--
Invalid nullable narrowing
--FILE--
<?php

interface A {}
interface B {}

class Foo {
public function foo(?A&B $foo) {
}
}

class FooChild extends Foo {
public function foo(A&B $foo) {
}
}

?>
--EXPECTF--
Fatal error: Declaration of FooChild::foo(A&B $foo) must be compatible with Foo::foo(?A&B $foo) in %s on line %d
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
--TEST--
Valid intersection type variance
--FILE--
<?php

interface X {}
interface Y {}
interface Z {}

class TestParent implements X, Y, Z {}
class TestChild implements Z {}

class A {
public ?X&Y $prop;

public function method1(X&Y&Z $a): ?X&Y {}
public function method2(X&Y $a): ?X {}
}
class B extends A {
public ?X&Y $prop;

public function method1(?X&Y $a): X&Y&Z {}
public function method2(?X $a): X&Y {}
}

?>
===DONE===
--EXPECT--
===DONE===
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
--TEST--
Valid inheritence - co-variance
--FILE--
<?php

interface A {}
interface B {}
interface C {}

class Test implements A, B, C {}

class Foo {
public function foo(): ?A&B {
return null;
}
}

class FooChild extends Foo {
public function foo(): A&B {
return new Test();
}
}

$o = new Foo();
var_dump($o->foo());
$o = new FooChild();
var_dump($o->foo());

?>
--EXPECTF--
NULL
object(Test)#%d (0) {
}
6 changes: 3 additions & 3 deletions Zend/zend_ast.c
Original file line number Diff line number Diff line change
Expand Up @@ -1535,6 +1535,9 @@ static ZEND_COLD void zend_ast_export_type(smart_str *str, zend_ast *ast, int in
}
return;
}
if (ast->attr & ZEND_TYPE_NULLABLE) {
smart_str_appendc(str, '?');
}
if (ast->kind == ZEND_AST_TYPE_INTERSECTION) {
zend_ast_list *list = zend_ast_get_list(ast);
for (uint32_t i = 0; i < list->children; i++) {
Expand All @@ -1545,9 +1548,6 @@ static ZEND_COLD void zend_ast_export_type(smart_str *str, zend_ast *ast, int in
}
return;
}
if (ast->attr & ZEND_TYPE_NULLABLE) {
smart_str_appendc(str, '?');
}
zend_ast_export_ns_name(str, ast, 0, indent);
}

Expand Down
11 changes: 1 addition & 10 deletions Zend/zend_compile.c
Original file line number Diff line number Diff line change
Expand Up @@ -6694,15 +6694,6 @@ void zend_compile_params(zend_ast *ast, zend_ast *return_type_ast, uint32_t fall
zend_error_noreturn(E_COMPILE_ERROR, "never cannot be used as a parameter type");
}

if (force_nullable && ZEND_TYPE_IS_INTERSECTION(arg_info->type)) {
zend_string *type_str = zend_type_to_string(arg_info->type);
zend_error_noreturn(E_COMPILE_ERROR,
"Cannot use null as default value for parameter $%s of type %s",
/* We move type_str pointer one char forward to skip the '?' generated by
* the call to zend_compile_typename() */
ZSTR_VAL(name), ZSTR_VAL(type_str)+1);
}

if (default_type != IS_UNDEF && default_type != IS_CONSTANT_AST && !force_nullable
&& !zend_is_valid_default_value(arg_info->type, &default_node.u.constant)) {
zend_string *type_str = zend_type_to_string(arg_info->type);
Expand Down Expand Up @@ -7333,7 +7324,7 @@ void zend_compile_prop_decl(zend_ast *ast, zend_ast *type_ast, uint32_t flags, z
if (ZEND_TYPE_IS_SET(type) && !Z_CONSTANT(value_zv)
&& !zend_is_valid_default_value(type, &value_zv)) {
zend_string *str = zend_type_to_string(type);
if (Z_TYPE(value_zv) == IS_NULL && !ZEND_TYPE_IS_INTERSECTION(type)) {
if (Z_TYPE(value_zv) == IS_NULL) {
ZEND_TYPE_FULL_MASK(type) |= MAY_BE_NULL;
zend_string *nullable_str = zend_type_to_string(type);

Expand Down
2 changes: 2 additions & 0 deletions Zend/zend_language_parser.y
Original file line number Diff line number Diff line change
Expand Up @@ -798,6 +798,7 @@ type_expr:
| '?' type { $$ = $2; $$->attr |= ZEND_TYPE_NULLABLE; }
| union_type { $$ = $1; }
| intersection_type { $$ = $1; }
| '?' intersection_type { $$ = $2; $$->attr |= ZEND_TYPE_NULLABLE; }
Copy link
Member

Choose a reason for hiding this comment

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

Regarding the nullability syntax. I find ?X&Y, while technically unambiguous due to null&Y not making sense, just isn't great for readability.

Further, we decided against ?X|Y in the union types RFC, and I think that decision should hold (even though the reasons are somewhat different.

I've got an implementation for (X&Y)|null, which is my preferred syntax at: https://gist.github.com/sgolemon/660b105e6894a6341792a479cda8f76b

However, we are much too late for 8.1 at this point and syntax is too important to rush, so this is going to have to wait for 8.2 no matter what we do.

Copy link
Contributor

Choose a reason for hiding this comment

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

Regarding the nullability syntax. I find ?X&Y, while technically unambiguous due to null&Y not making sense, just isn't great for readability.

Further, we decided against ?X|Y in the union types RFC, and I think that decision should hold (even though the reasons are somewhat different.

Agreed.

I've got an implementation for (X&Y)|null

I'm not opposed to allowing parenthesis, but requiring them seems unnecessary - the precedence of &/| and &&/|| is known in other contexts in php (operator precedence).

If (A&B)|null was allowed, there'd be the question of whether (A)|null should be allowed, (A)|(null), etc. It seems like it should (IF allowed), though the syntax seems less readable

However, we are much too late for 8.1 at this point and syntax is too important to rush, so this is going to have to wait for 8.2 no matter what we do.

Agreed.

Copy link

@iquito iquito Jul 19, 2021

Choose a reason for hiding this comment

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

I don't think you can use ?X|Y being disallowed to conclude that everyone would also be against the special case of ?X&Y. At the time in the Github discussion I argued against ?(X|Y), because it added parenthesis to not be ambiguous which would otherwise not be needed, and X|Y|null was available as an alternative. In that case having a more complicated syntax only to somehow include ? would have been counterproductive, in my opinion.

Yet I very much see the value of ?X&Y - it enables optional parameters/properties, and it does not contain parenthesis. If combined union type and intersection type syntax ever makes it into PHP, it would be easy to not allow ? there, just as ? is not allowed for union types. So the syntax seems forward-compatible and geared towards a common use case in PHP.

Copy link
Member

@sgolemon sgolemon Jul 22, 2021

Choose a reason for hiding this comment

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

If (A&B)|null was allowed, there'd be the question of whether (A)|null should be allowed, (A)|(null), etc. It seems like it should (IF allowed), though the syntax seems less readable

That's a fair complaint. For me it comes down to finding a balance between providing the functionality that's being asked for quite strongly, and not over-delivering syntax that hasn't been entirely thought through. That's why my version of this patch only allows null on the right hand side, and requires parens around the intersection. It's the MOST restrictive syntax, and thereby the least likely to cause issues after we've spent more time thinking about it.

;

type:
Expand All @@ -823,6 +824,7 @@ type_expr_without_static:
| '?' type_without_static { $$ = $2; $$->attr |= ZEND_TYPE_NULLABLE; }
| union_type_without_static { $$ = $1; }
| intersection_type_without_static { $$ = $1; }
| '?' intersection_type_without_static { $$ = $2; $$->attr |= ZEND_TYPE_NULLABLE; }
;

type_without_static:
Expand Down