Skip to content

Commit be902fc

Browse files
krakjoekrakjoe
committed
remove nullable, improve error messages, validate at compilation time
1 parent a0702ef commit be902fc

File tree

13 files changed

+126
-175
lines changed

13 files changed

+126
-175
lines changed

‎Zend/tests/return_hint/001.phpt

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,6 @@ Basic return hints at compilation
55
function test1() : array {
66

77
}
8-
/* this is bad ... */
98
?>
10-
--EXPECT--
9+
--EXPECTF--
10+
Fatal error: the function test1 was expected to return an array and returned nothing in %s on line %d

‎Zend/tests/return_hint/002.phpt

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,5 +9,6 @@ function test1() : array {
99
test1();
1010
?>
1111
--EXPECTF--
12-
Fatal error: the function test1 was expected to return array and returned null in %s on line %d
12+
Fatal error: the function test1 was expected to return an array and returned null in %s on line %d
13+
1314

‎Zend/tests/return_hint/003.phpt

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,10 @@
11
--TEST--
2-
Basic return hints at execution allowing null
2+
Basic return hints at compilation (constant scalar)
33
--FILE--
44
<?php
5-
function test1() : ?array {
6-
return null;
5+
function test1() : array {
6+
return 1;
77
}
8-
9-
var_dump(test1());
108
?>
11-
--EXPECT--
12-
NULL
13-
9+
--EXPECTF--
10+
Fatal error: the function test1 was expected to return an array and returned an integer in %s on line %d

‎Zend/tests/return_hint/004.phpt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,4 +9,4 @@ function test1() : array {
99
test1()
1010
?>
1111
--EXPECTF--
12-
Fatal error: the function test1 was expected to return array and returned string in %s on line %d
12+
Fatal error: the function test1 was expected to return an array and returned a string in %s on line %d

‎Zend/tests/return_hint/005.phpt

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,5 +16,6 @@ $qux = new qux();
1616
$qux->foo();
1717
?>
1818
--EXPECTF--
19-
Fatal error: the function foo was expected to return foo and returned baz in %s on line %d
19+
Catchable fatal error: the function qux::foo was expected to return an object of class foo and returned an object of class baz in %s on line %d
20+
2021

‎Zend/tests/return_hint/010.phpt

Lines changed: 0 additions & 20 deletions
This file was deleted.

‎Zend/tests/return_hint/012.phpt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ Basic return hints callable and closures
55
class foo {
66
public function bar() : callable {
77
$test = "one";
8-
return function() use($test) : ?array {
8+
return function() use($test) : array {
99
return array($test);
1010
};
1111
}

‎Zend/tests/return_hint/013.phpt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,5 +15,5 @@ $baz = new foo();
1515
var_dump($func=$baz->bar(), $func());
1616
?>
1717
--EXPECTF--
18-
Fatal error: the function %s was expected to return array and returned null in %s on line %d
18+
Fatal error: the function %s was expected to return an array and returned null in %s on line %d
1919

‎Zend/zend_compile.c

Lines changed: 67 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,63 @@ ZEND_API zend_compiler_globals compiler_globals;
102102
ZEND_API zend_executor_globals executor_globals;
103103
#endif
104104

105+
ZEND_API void zend_return_hint_error(int type, zend_function *function, zval *returned, const char *message TSRMLS_DC) {
106+
char *scope = NULL;
107+
char *expected = NULL;
108+
char *got = NULL;
109+
110+
if (function->common.scope) {
111+
zend_spprintf(&scope, 0, "%s::%s",
112+
ZEND_FN_SCOPE_NAME(function),
113+
function->common.function_name);
114+
} else scope = estrdup(function->common.function_name);
115+
116+
if (function->common.return_hint.type == IS_OBJECT) {
117+
zend_spprintf(&expected, 0, "an object of class %s", function->common.return_hint.class_name);
118+
} else switch (function->common.return_hint.type) {
119+
case IS_ARRAY: {
120+
zend_spprintf(&expected, 0, "an array");
121+
} break;
122+
123+
default:
124+
zend_spprintf(&expected, 0, "a %s", zend_get_type_by_const(function->common.return_hint.type));
125+
}
126+
127+
if (message) {
128+
zend_error(type, "the function %s was expected to return %s, %s", scope, expected, message);
129+
efree(scope);
130+
efree(expected);
131+
return;
132+
}
133+
134+
if (!returned) {
135+
got = "nothing";
136+
} else {
137+
if (Z_TYPE_P(returned) == IS_OBJECT) {
138+
zend_spprintf(&got, 0, "an object of class %s", Z_OBJCE_P(returned)->name);
139+
} else switch (Z_TYPE_P(returned)) {
140+
case IS_NULL:
141+
got = estrdup("null");
142+
break;
143+
144+
case IS_LONG:
145+
case IS_ARRAY: {
146+
zend_spprintf(&got, 0, "an %s", zend_zval_type_name(returned));
147+
} break;
148+
149+
default:
150+
zend_spprintf(&got, 0, "a %s", zend_zval_type_name(returned));
151+
}
152+
}
153+
154+
zend_error(type, "the function %s was expected to return %s and returned %s", scope, expected, got);
155+
156+
efree(scope);
157+
efree(expected);
158+
if (returned)
159+
efree(got);
160+
}
161+
105162
static void zend_push_function_call_entry(zend_function *fbc TSRMLS_DC) /* {{{ */
106163
{
107164
zend_function_call_entry fcall = { fbc };
@@ -1863,10 +1920,9 @@ void zend_do_end_function_declaration(const znode *function_token TSRMLS_DC) /*
18631920
/* }}} */
18641921

18651922
/* {{{ */
1866-
void zend_do_function_return_hint(const znode *return_hint, zend_bool allow_null TSRMLS_DC) {
1923+
void zend_do_function_return_hint(const znode *return_hint TSRMLS_DC) {
18671924
if (return_hint->op_type != IS_UNUSED) {
18681925
CG(active_op_array)->return_hint.used = 1;
1869-
CG(active_op_array)->return_hint.allow_null = allow_null;
18701926

18711927
if (return_hint->op_type == IS_CONST) {
18721928
if (Z_TYPE(return_hint->u.constant) == IS_STRING) {
@@ -2830,18 +2886,17 @@ void zend_do_return(znode *expr, int do_end_vparse TSRMLS_DC) /* {{{ */
28302886

28312887
start_op_number = get_next_op_number(CG(active_op_array));
28322888

2833-
if (CG(active_op_array)->return_hint.used && expr) {
2834-
if (expr->op_type == IS_UNUSED && !CG(active_op_array)->return_hint.allow_null) {
2835-
zend_error(E_COMPILE_ERROR, "return hint error in %s", CG(active_op_array)->function_name);
2836-
} else {
2837-
if (expr->op_type == IS_CONST) {
2838-
if (Z_TYPE(expr->u.constant) & ~IS_CONSTANT_TYPE_MASK != CG(active_op_array)->return_hint.type) {
2839-
zend_error(E_COMPILE_ERROR, "return type mismatch");
2840-
}
2889+
if (CG(active_op_array)->return_hint.used &&
2890+
!(CG(active_op_array)->fn_flags & ZEND_ACC_ABSTRACT) &&
2891+
!(CG(active_op_array)->fn_flags & ZEND_ACC_HAS_RETURN_HINT)) {
2892+
if (!expr || expr->op_type == IS_UNUSED) {
2893+
zend_return_hint_error(E_COMPILE_ERROR, (zend_function*)CG(active_op_array), NULL, NULL TSRMLS_CC);
2894+
} else if (expr && expr->op_type == IS_CONST) {
2895+
if ((Z_TYPE(expr->u.constant) & ~IS_CONSTANT_TYPE_MASK) != CG(active_op_array)->return_hint.type) {
2896+
zend_return_hint_error(E_COMPILE_ERROR, (zend_function*)CG(active_op_array), &expr->u.constant, NULL TSRMLS_CC);
28412897
}
2842-
2843-
/* work out if we can do anything else */
28442898
}
2899+
CG(active_op_array)->fn_flags |= ZEND_ACC_HAS_RETURN_HINT;
28452900
}
28462901

28472902
#ifdef ZTS
@@ -3563,10 +3618,6 @@ static char * zend_get_function_declaration(zend_function *fptr TSRMLS_DC) /* {{
35633618
*(offset++) = ':';
35643619
*(offset++) = ' ';
35653620

3566-
if (fptr->common.return_hint.allow_null) {
3567-
*(offset++) = '?';
3568-
}
3569-
35703621
switch (fptr->common.return_hint.type) {
35713622
case IS_OBJECT:
35723623
REALLOC_BUF_IF_EXCEED(buf, offset, length, fptr->common.return_hint.class_name_len);

‎Zend/zend_compile.h

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -216,6 +216,8 @@ typedef struct _zend_try_catch_element {
216216

217217
/* function has arguments with type hinting */
218218
#define ZEND_ACC_HAS_TYPE_HINTS 0x10000000
219+
/* function has return hint */
220+
#define ZEND_ACC_HAS_RETURN_HINT 0x20000000
219221

220222
char *zend_visibility_string(zend_uint fn_flags);
221223

@@ -268,7 +270,6 @@ typedef struct _zend_return_hint {
268270
zend_uchar type;
269271
const char *class_name;
270272
zend_uint class_name_len;
271-
zend_bool allow_null;
272273
zend_bool used;
273274
} zend_return_hint;
274275

@@ -460,6 +461,7 @@ ZEND_API void zend_restore_compiled_filename(char *original_compiled_filename TS
460461
ZEND_API char *zend_get_compiled_filename(TSRMLS_D);
461462
ZEND_API int zend_get_compiled_lineno(TSRMLS_D);
462463
ZEND_API size_t zend_get_scanned_file_offset(TSRMLS_D);
464+
ZEND_API void zend_return_hint_error(int type, zend_function *function, zval *returned, const char *message TSRMLS_DC);
463465

464466
void zend_resolve_non_class_name(znode *element_name, zend_bool *check_namespace, zend_bool case_sensitive, HashTable *current_import_sub TSRMLS_DC);
465467
void zend_resolve_function_name(znode *element_name, zend_bool *check_namespace TSRMLS_DC);
@@ -526,7 +528,7 @@ void zend_do_add_variable(znode *result, const znode *op1, const znode *op2 TSRM
526528
int zend_do_verify_access_types(const znode *current_access_type, const znode *new_modifier);
527529
void zend_do_begin_function_declaration(znode *function_token, znode *function_name, int is_method, int return_reference, znode *fn_flags_znode TSRMLS_DC);
528530
void zend_do_end_function_declaration(const znode *function_token TSRMLS_DC);
529-
void zend_do_function_return_hint(const znode *return_hint, zend_bool allow_null TSRMLS_DC);
531+
void zend_do_function_return_hint(const znode *return_hint TSRMLS_DC);
530532
void zend_do_receive_param(zend_uchar op, znode *varname, const znode *initialization, znode *class_type, zend_bool pass_by_reference, zend_bool is_variadic TSRMLS_DC);
531533
int zend_do_begin_function_call(znode *function_name, zend_bool check_namespace TSRMLS_DC);
532534
void zend_do_begin_method_call(znode *left_bracket TSRMLS_DC);

‎Zend/zend_language_parser.y

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -418,8 +418,7 @@ function_return_type:
418418

419419
function_return_hint:
420420
/* empty */ { $$.op_type = IS_UNUSED; }
421-
| ':' function_return_type { zend_do_function_return_hint(&$2, 0 TSRMLS_CC); }
422-
| ':' '?' function_return_type { zend_do_function_return_hint(&$3, 1 TSRMLS_CC); }
421+
| ':' function_return_type { zend_do_function_return_hint(&$2 TSRMLS_CC); }
423422
;
424423

425424
unticked_function_declaration_statement:

‎Zend/zend_vm_def.h

Lines changed: 8 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -2851,48 +2851,32 @@ ZEND_VM_HANDLER(62, ZEND_RETURN, CONST|TMP|VAR|CV, ANY)
28512851
if (EX(function_state).function->common.return_hint.used) {
28522852
zend_return_hint *return_hint = &EX(function_state).function->common.return_hint;
28532853

2854-
if (!retval_ptr || (!return_hint->allow_null && Z_TYPE_P(retval_ptr) == IS_NULL)) {
2855-
zend_error(E_ERROR,
2856-
"the function %s was expected to return %s and returned null",
2857-
EX(function_state).function->common.function_name,
2858-
(return_hint->type == IS_OBJECT) ?
2859-
return_hint->class_name : zend_get_type_by_const(return_hint->type));
2860-
} else if (retval_ptr && (Z_TYPE_P(retval_ptr) != IS_NULL || !return_hint->allow_null)){
2854+
if (!retval_ptr || Z_TYPE_P(retval_ptr) == IS_NULL) {
2855+
zend_return_hint_error(E_RECOVERABLE_ERROR, EX(function_state).function, retval_ptr, NULL TSRMLS_CC);
2856+
} else if (retval_ptr){
28612857
switch (return_hint->type) {
28622858
case IS_ARRAY: if (Z_TYPE_P(retval_ptr) != IS_ARRAY) {
2863-
zend_error(E_ERROR,
2864-
"the function %s was expected to return array and returned %s",
2865-
EX(function_state).function->common.function_name,
2866-
zend_get_type_by_const(Z_TYPE_P(retval_ptr)));
2859+
zend_return_hint_error(E_RECOVERABLE_ERROR, EX(function_state).function, retval_ptr, NULL TSRMLS_CC);
28672860
} break;
28682861

28692862
case IS_CALLABLE: if (Z_TYPE_P(retval_ptr) != IS_OBJECT ||
28702863
!zend_is_callable_ex(retval_ptr, NULL, IS_CALLABLE_CHECK_SILENT, NULL, NULL, NULL, NULL TSRMLS_CC)) {
2871-
zend_error(E_ERROR,
2872-
"the function %s was expected to return callable and returned %s",
2873-
EX(function_state).function->common.function_name,
2874-
zend_get_type_by_const(Z_TYPE_P(retval_ptr)));
2864+
zend_return_hint_error(E_RECOVERABLE_ERROR, EX(function_state).function, retval_ptr, NULL TSRMLS_CC);
28752865
} break;
28762866

28772867
case IS_OBJECT: {
28782868
zend_class_entry **ce = NULL;
28792869

28802870
if (Z_TYPE_P(retval_ptr) != IS_OBJECT) {
2881-
zend_error(E_ERROR,
2882-
"the function %s was expected to return %s and returned %s",
2883-
EX(function_state).function->common.function_name, return_hint->class_name, zend_zval_type_name(retval_ptr));
2871+
zend_return_hint_error(E_RECOVERABLE_ERROR, EX(function_state).function, retval_ptr, NULL TSRMLS_CC);
28842872
}
28852873

28862874
if (zend_lookup_class(return_hint->class_name, return_hint->class_name_len, &ce TSRMLS_CC) != SUCCESS) {
2887-
zend_error(E_ERROR,
2888-
"the function %s was expected to return %s, the class could not be found",
2889-
EX(function_state).function->common.function_name, return_hint->class_name);
2875+
zend_return_hint_error(E_RECOVERABLE_ERROR, EX(function_state).function, NULL, "the class could not be found" TSRMLS_CC);
28902876
}
28912877

28922878
if (!instanceof_function(Z_OBJCE_P(retval_ptr), *ce TSRMLS_CC)) {
2893-
zend_error(E_ERROR,
2894-
"the function %s was expected to return %s and returned %s",
2895-
EX(function_state).function->common.function_name, return_hint->class_name, Z_OBJCE_P(retval_ptr)->name);
2879+
zend_return_hint_error(E_RECOVERABLE_ERROR, EX(function_state).function, retval_ptr, NULL TSRMLS_CC);
28962880
}
28972881
}
28982882
}

0 commit comments

Comments
 (0)