-
Notifications
You must be signed in to change notification settings - Fork 7.9k
Adding object type-hint and return-type #2080
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
Changes from all commits
cd162bd
9e332e2
9be7177
20fc5db
3a959c1
18752dd
cac0619
70f6cce
889deba
be2a996
5276f51
03b8541
1a8d8e4
2ad84d1
21ecea6
e0825e9
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 |
---|---|---|
@@ -0,0 +1,15 @@ | ||
--TEST-- | ||
Missing class method a object return type during inheritance | ||
--FILE-- | ||
<?php | ||
|
||
class One { | ||
public function a() : object {} | ||
} | ||
|
||
class Two extends One { | ||
public function a() {} | ||
} | ||
|
||
--EXPECTF-- | ||
Fatal error: Declaration of Two::a() must be compatible with One::a(): object in %s on line 9 |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,15 @@ | ||
--TEST-- | ||
Missing interface method a object return type during inheritance | ||
--FILE-- | ||
<?php | ||
|
||
interface One { | ||
public function a() : object {} | ||
} | ||
|
||
interface Two extends One { | ||
public function a() {} | ||
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. It would be awesome to have covariance support for return type hints stricter than This way, you could have: interface Repo { public function find(Uuid $id) : object; }
class UserRepo { public function find(Uuid $id) : User; } 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. It should be fine to add this. We're okay with return type covariance where it does not cause technical issues. E.g. 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. But why there is no support for the same thing in arguments? 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 arguments, that breaks variance: a consumer relying on the interface will bot work when an implementation has stricter parameter types. To solve that, there was a recent discussion at https://wiki.php.net/rfc/never_for_parameter_types |
||
} | ||
|
||
--EXPECTF-- | ||
Fatal error: Interface function One::a() cannot contain body in %s on line 4 | ||
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. This is probably not what you want to test here ;) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,26 @@ | ||
--TEST-- | ||
Adding a class method object return type | ||
--FILE-- | ||
<?php | ||
|
||
interface One { | ||
public function a() : object; | ||
} | ||
|
||
class Two implements One { | ||
public function a() : object {} | ||
} | ||
|
||
$three = new class extends Two { | ||
public function a() : object { | ||
return 12345; | ||
} | ||
}; | ||
$three->a(); | ||
--EXPECTF-- | ||
|
||
Fatal error: Uncaught TypeError: Return value of class@anonymous::a() must be an object, integer returned in %s:13 | ||
Stack trace: | ||
#0 %s(16): class@anonymous->a() | ||
#1 {main} | ||
thrown in %s on line 13 |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,16 @@ | ||
--TEST-- | ||
Adding a function object return type | ||
--FILE-- | ||
<?php | ||
|
||
function a() : object { | ||
return 12345; | ||
} | ||
a(); | ||
--EXPECTF-- | ||
|
||
Fatal error: Uncaught TypeError: Return value of a() must be an object, integer returned in %s:4 | ||
Stack trace: | ||
#0 %s(6): a() | ||
#1 {main} | ||
thrown in %s on line 4 |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,22 @@ | ||
--TEST-- | ||
Adding a interface method object return type | ||
--FILE-- | ||
<?php | ||
|
||
interface One { | ||
public function a() : object; | ||
} | ||
|
||
$two = new class implements One { | ||
public function a() : object { | ||
return 12345; | ||
} | ||
}; | ||
$two->a(); | ||
--EXPECTF-- | ||
|
||
Fatal error: Uncaught TypeError: Return value of class@anonymous::a() must be an object, integer returned in %s:9 | ||
Stack trace: | ||
#0 %s(12): class@anonymous->a() | ||
#1 {main} | ||
thrown in %s on line 9 | ||
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. This test looks redundant with Zend/tests/object_types/return_type_in_class.phpt, which also has an object type on the interface. |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,26 @@ | ||
--TEST-- | ||
Adding class method a object return type during inheritance is allowed | ||
--FILE-- | ||
<?php | ||
|
||
class One { | ||
public function a() {} | ||
} | ||
|
||
class Two extends One { | ||
public function a() : object {} | ||
} | ||
|
||
$three = new class extends Two { | ||
public function a() : object { | ||
return 12345; | ||
} | ||
}; | ||
$three->a(); | ||
|
||
--EXPECTF-- | ||
Fatal error: Uncaught TypeError: Return value of class@anonymous::a() must be an object, integer returned in %s:13 | ||
Stack trace: | ||
#0 %s(16): class@anonymous->a() | ||
#1 {main} | ||
thrown in /%s on line 13 |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,26 @@ | ||
--TEST-- | ||
Adding interface method a object return type during inheritance is allowed | ||
--FILE-- | ||
<?php | ||
|
||
interface One { | ||
public function a(); | ||
} | ||
|
||
interface Two extends One { | ||
public function a() : object; | ||
} | ||
|
||
$three = new class implements Two { | ||
public function a() : object { | ||
return 12345; | ||
} | ||
}; | ||
$three->a(); | ||
|
||
--EXPECTF-- | ||
Fatal error: Uncaught TypeError: Return value of class@anonymous::a() must be an object, integer returned in %s:13 | ||
Stack trace: | ||
#0 %s(16): class@anonymous->a() | ||
#1 {main} | ||
thrown in /%s on line 13 |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,31 @@ | ||
--TEST-- | ||
Reflecting object return type | ||
--FILE-- | ||
<?php | ||
|
||
interface One { | ||
public function a() : object; | ||
} | ||
|
||
class Two implements One { | ||
public function a() : object {} | ||
} | ||
|
||
function a() : object {} | ||
|
||
$returnTypeOne = (new ReflectionClass(One::class))->getMethod('a')->getReturnType(); | ||
var_dump($returnTypeOne->isBuiltin(), (string)$returnTypeOne); | ||
|
||
$returnTypeTwo = (new ReflectionClass(Two::class))->getMethod('a')->getReturnType(); | ||
var_dump($returnTypeTwo->isBuiltin(), (string)$returnTypeTwo); | ||
|
||
$returnTypea = (new ReflectionFunction('a'))->getReturnType(); | ||
var_dump($returnTypea->isBuiltin(), (string)$returnTypea); | ||
|
||
--EXPECTF-- | ||
bool(true) | ||
string(6) "object" | ||
bool(true) | ||
string(6) "object" | ||
bool(true) | ||
string(6) "object" |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,19 @@ | ||
--TEST-- | ||
Adding a class method object type hint | ||
--FILE-- | ||
<?php | ||
|
||
class One { | ||
public function a(object $obj) {} | ||
} | ||
|
||
$one = new One(); | ||
$one->a(new One()); | ||
$one->a(123); | ||
--EXPECTF-- | ||
|
||
Fatal error: Uncaught TypeError: Argument 1 passed to One::a() must be an object, integer given, called in %s:4 | ||
Stack trace: | ||
#0 %s(9): One->a(123) | ||
#1 {main} | ||
thrown in %s on line 4 |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,17 @@ | ||
--TEST-- | ||
Adding a function object type hint | ||
--FILE-- | ||
<?php | ||
|
||
class A {} | ||
function a(object $obj) {} | ||
|
||
a(new A()); | ||
a(123); | ||
--EXPECTF-- | ||
|
||
Fatal error: Uncaught TypeError: Argument 1 passed to a() must be an object, integer given, called in %s.php on line 7 and defined in %s:4 | ||
Stack trace: | ||
#0 %s(7): a(123) | ||
#1 {main} | ||
thrown in %s on line 4 |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,31 @@ | ||
--TEST-- | ||
Reflecting object type hint | ||
--FILE-- | ||
<?php | ||
|
||
interface One { | ||
public function a(object $obj); | ||
} | ||
|
||
class Two implements One { | ||
public function a(object $obj) {} | ||
} | ||
|
||
function a(object $obj) {} | ||
|
||
$typeHintOne = (new ReflectionClass(One::class))->getMethod('a')->getParameters()[0]->getType(); | ||
var_dump($typeHintOne->isBuiltin(), (string)$typeHintOne); | ||
|
||
$typeHintTwo = (new ReflectionClass(Two::class))->getMethod('a')->getParameters()[0]->getType(); | ||
var_dump($typeHintTwo->isBuiltin(), (string)$typeHintTwo); | ||
|
||
$typeHinta = (new ReflectionFunction('a'))->getParameters()[0]->getType(); | ||
var_dump($typeHinta->isBuiltin(), (string)$typeHinta); | ||
|
||
--EXPECTF-- | ||
bool(true) | ||
string(6) "object" | ||
bool(true) | ||
string(6) "object" | ||
bool(true) | ||
string(6) "object" |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -161,6 +161,7 @@ static const struct reserved_class_name reserved_class_names[] = { | |
{ZEND_STRL("true")}, | ||
{ZEND_STRL("void")}, | ||
{ZEND_STRL("iterable")}, | ||
{ZEND_STRL("object")}, | ||
{NULL, 0} | ||
}; | ||
|
||
|
@@ -206,6 +207,7 @@ static const builtin_type_info builtin_types[] = { | |
{ZEND_STRL("bool"), _IS_BOOL}, | ||
{ZEND_STRL("void"), IS_VOID}, | ||
{ZEND_STRL("iterable"), IS_ITERABLE}, | ||
{ZEND_STRL("object"), IS_OBJECT}, | ||
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. Wait... This is all there is to the patch? 😵 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. Yes, that's all what is needed :) |
||
{NULL, 0, IS_UNDEF} | ||
}; | ||
|
||
|
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.
The test description doesn't match the actual body