From 2edc3cf8d2dd75501bf1049cf1b3ca57f11f1234 Mon Sep 17 00:00:00 2001 From: Niklas Keller Date: Sun, 1 Jan 2017 15:53:24 +0100 Subject: [PATCH] Implement Parameter Type Widening RFC --- UPGRADING | 6 +++ Zend/tests/objects_009.phpt | 3 +- .../parameter_type_variance.phpt | 34 ++++++++++++++++ Zend/zend_inheritance.c | 39 ++++++++++++------- tests/classes/type_hinting_005b.phpt | 14 ------- 5 files changed, 67 insertions(+), 29 deletions(-) create mode 100644 Zend/tests/type_declarations/parameter_type_variance.phpt delete mode 100644 tests/classes/type_hinting_005b.phpt diff --git a/UPGRADING b/UPGRADING index 387696268b..e072d407ea 100644 --- a/UPGRADING +++ b/UPGRADING @@ -86,6 +86,12 @@ PHP 7.2 UPGRADE NOTES 2. New Features ======================================== +- Core: + . It is now possible to remove argument type annotations when overriding an + inherited method. This complies with contravariance of method argument types + under the Liskov Substitution Principle. + (https://wiki.php.net/rfc/parameter-no-type-variance) + - PCRE: . Added `J` modifier for setting PCRE_DUPNAMES. diff --git a/Zend/tests/objects_009.phpt b/Zend/tests/objects_009.phpt index 353266b093..e2333235ab 100644 --- a/Zend/tests/objects_009.phpt +++ b/Zend/tests/objects_009.phpt @@ -19,6 +19,5 @@ class test3 extends test { echo "Done\n"; ?> ---EXPECTF-- -Warning: Declaration of test3::foo($arg) should be compatible with test::foo(Test $arg) in %s on line %d +--EXPECT-- Done diff --git a/Zend/tests/type_declarations/parameter_type_variance.phpt b/Zend/tests/type_declarations/parameter_type_variance.phpt new file mode 100644 index 0000000000..61915a2e6c --- /dev/null +++ b/Zend/tests/type_declarations/parameter_type_variance.phpt @@ -0,0 +1,34 @@ +--TEST-- +Parameter variance with no type +--FILE-- + +--EXPECTF-- +Warning: Declaration of Bar::testChildClass(Foo $foo) should be compatible with Foo::testChildClass($foo) in %s on line %d + +Warning: Declaration of Bar::testChildBuiltin(int $foo) should be compatible with Foo::testChildBuiltin($foo) in %s on line %d diff --git a/Zend/zend_inheritance.c b/Zend/zend_inheritance.c index f3cb81cf41..5b5c9756e3 100644 --- a/Zend/zend_inheritance.c +++ b/Zend/zend_inheritance.c @@ -172,23 +172,20 @@ static zend_always_inline zend_bool zend_iterable_compatibility_check(zend_arg_i if (ZEND_TYPE_CODE(arg_info->type) == IS_ARRAY) { return 1; } - + if (ZEND_TYPE_IS_CLASS(arg_info->type) && zend_string_equals_literal_ci(ZEND_TYPE_NAME(arg_info->type), "Traversable")) { return 1; } - + return 0; } /* }}} */ static int zend_do_perform_type_hint_check(const zend_function *fe, zend_arg_info *fe_arg_info, const zend_function *proto, zend_arg_info *proto_arg_info) /* {{{ */ { - if (ZEND_LOG_XOR(ZEND_TYPE_IS_CLASS(fe_arg_info->type), ZEND_TYPE_IS_CLASS(proto_arg_info->type))) { - /* Only one has a type declaration and the other one doesn't */ - return 0; - } + ZEND_ASSERT(ZEND_TYPE_IS_SET(fe_arg_info->type) && ZEND_TYPE_IS_SET(proto_arg_info->type)); - if (ZEND_TYPE_IS_CLASS(fe_arg_info->type)) { + if (ZEND_TYPE_IS_CLASS(fe_arg_info->type) && ZEND_TYPE_IS_CLASS(proto_arg_info->type)) { zend_string *fe_class_name, *proto_class_name; const char *class_name; @@ -237,7 +234,7 @@ static int zend_do_perform_type_hint_check(const zend_function *fe, zend_arg_inf zend_string_release(proto_class_name); zend_string_release(fe_class_name); } else if (ZEND_TYPE_CODE(fe_arg_info->type) != ZEND_TYPE_CODE(proto_arg_info->type)) { - /* Incompatible type */ + /* Incompatible built-in types */ return 0; } @@ -245,6 +242,22 @@ static int zend_do_perform_type_hint_check(const zend_function *fe, zend_arg_inf } /* }}} */ +static int zend_do_perform_arg_type_hint_check(const zend_function *fe, zend_arg_info *fe_arg_info, const zend_function *proto, zend_arg_info *proto_arg_info) /* {{{ */ +{ + if (!ZEND_TYPE_IS_SET(fe_arg_info->type)) { + /* Child with no type is always compatible */ + return 1; + } + + if (!ZEND_TYPE_IS_SET(proto_arg_info->type)) { + /* Child defines a type, but parent doesn't, violates LSP */ + return 0; + } + + return zend_do_perform_type_hint_check(fe, fe_arg_info, proto, proto_arg_info); +} +/* }}} */ + static zend_bool zend_do_perform_implementation_check(const zend_function *fe, const zend_function *proto) /* {{{ */ { uint32_t i, num_args; @@ -312,15 +325,15 @@ static zend_bool zend_do_perform_implementation_check(const zend_function *fe, c } else { proto_arg_info = &proto->common.arg_info[proto->common.num_args]; } - - if (!zend_do_perform_type_hint_check(fe, fe_arg_info, proto, proto_arg_info)) { + + if (!zend_do_perform_arg_type_hint_check(fe, fe_arg_info, proto, proto_arg_info)) { switch (ZEND_TYPE_CODE(fe_arg_info->type)) { case IS_ITERABLE: if (!zend_iterable_compatibility_check(proto_arg_info)) { return 0; } break; - + default: return 0; } @@ -345,7 +358,7 @@ static zend_bool zend_do_perform_implementation_check(const zend_function *fe, c if (!(fe->common.fn_flags & ZEND_ACC_HAS_RETURN_TYPE)) { return 0; } - + if (!zend_do_perform_type_hint_check(fe, fe->common.arg_info - 1, proto, proto->common.arg_info - 1)) { switch (ZEND_TYPE_CODE(proto->common.arg_info[-1].type)) { case IS_ITERABLE: @@ -353,7 +366,7 @@ static zend_bool zend_do_perform_implementation_check(const zend_function *fe, c return 0; } break; - + default: return 0; } diff --git a/tests/classes/type_hinting_005b.phpt b/tests/classes/type_hinting_005b.phpt deleted file mode 100644 index b9fb198fc5..0000000000 --- a/tests/classes/type_hinting_005b.phpt +++ /dev/null @@ -1,14 +0,0 @@ ---TEST-- -Check type hint compatibility in overrides with array hints. ---FILE-- - -==DONE== ---EXPECTF-- -Warning: Declaration of D::f($a) should be compatible with C::f(array $a) in %s on line 5 -No hint, should be array. -==DONE== -- 2.40.0