Implement Parameter Type Widening RFC

kelunik · nikic · commit 2edc3cf8d2dd · 2017-02-04T23:33:24.000+01:00
diff --git 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
@@ -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
@@ -0,0 +1,34 @@
+--TEST--
+Parameter variance with no type
+--FILE--
+<?php
+
+class Foo {
+    function testParentClass(Foo $foo) {}
+    function testBothClass(Foo $foo) {}
+    function testChildClass($foo) {}
+    function testNoneClass($foo) {}
+
+    function testParentBuiltin(int $foo) {}
+    function testBothBuiltin(int $foo) {}
+    function testChildBuiltin($foo) {}
+    function testNoneBuiltin($foo) {}
+}
+
+class Bar extends Foo {
+    function testParentClass($foo) {}
+    function testBothClass(Foo $foo) {}
+    function testChildClass(Foo $foo) {}
+    function testNoneClass($foo) {}
+
+    function testParentBuiltin($foo) {}
+    function testBothBuiltin(int $foo) {}
+    function testChildBuiltin(int $foo) {}
+    function testNoneBuiltin($foo) {}
+}
+
+?>
+--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
@@ -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,14 +234,30 @@ 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;
 	}
 
 	return 1;
 }
 /* }}} */
 
+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,15 +358,15 @@ 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:
 					if (!zend_iterable_compatibility_check(fe->common.arg_info - 1)) {
 						return 0;
 					}
 					break;
-					
+
 				default:
 					return 0;
 			}
diff --git a/tests/classes/type_hinting_005b.phpt b/tests/classes/type_hinting_005b.phpt
