Skip to content

Commit 39cc5c7

Browse files
committed
fix(zend): do not shortcircuit parenthesized nullsafe operations
Signed-off-by: azjezz <[email protected]>
1 parent c27368c commit 39cc5c7

File tree

4 files changed

+78
-1
lines changed

4 files changed

+78
-1
lines changed
Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
--TEST--
2+
Parentheses should contain nullsafe short-circuiting
3+
--FILE--
4+
<?php
5+
6+
class Foo {
7+
public function bar(): array {
8+
return [1, 2, 3];
9+
}
10+
11+
public string $prop = 'value';
12+
}
13+
14+
$nullable = null;
15+
$foo = new Foo();
16+
17+
echo "Without parentheses (short-circuits):\n";
18+
var_dump($nullable?->bar()[0]);
19+
var_dump($nullable?->prop[0]);
20+
var_dump($foo?->bar()[0]);
21+
22+
echo "\nWith parentheses (should warn on null):\n";
23+
var_dump(($nullable?->bar())[0]);
24+
var_dump(($nullable?->prop)[0]);
25+
26+
echo "\nDirect null (should warn):\n";
27+
var_dump((null)[0]);
28+
29+
echo "\nNon-null with parentheses:\n";
30+
var_dump(($foo?->bar())[0]);
31+
var_dump(($foo?->prop)[0]);
32+
33+
?>
34+
--EXPECTF--
35+
Without parentheses (short-circuits):
36+
NULL
37+
NULL
38+
int(1)
39+
40+
With parentheses (should warn on null):
41+
42+
Warning: Trying to access array offset on null in %s on line %d
43+
NULL
44+
45+
Warning: Trying to access array offset on null in %s on line %d
46+
NULL
47+
48+
Direct null (should warn):
49+
50+
Warning: Trying to access array offset on null in %s on line %d
51+
NULL
52+
53+
Non-null with parentheses:
54+
int(1)
55+
string(1) "v"

Zend/zend_compile.c

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2509,6 +2509,9 @@ static bool zend_ast_is_short_circuited(const zend_ast *ast)
25092509
return zend_ast_is_short_circuited(ast->child[0]);
25102510
case ZEND_AST_NULLSAFE_PROP:
25112511
case ZEND_AST_NULLSAFE_METHOD_CALL:
2512+
if (ast->attr & ZEND_PARENTHESIZED_NULLSAFE) {
2513+
return 0;
2514+
}
25122515
return 1;
25132516
default:
25142517
return 0;
@@ -2530,6 +2533,9 @@ static void zend_assert_not_short_circuited(const zend_ast *ast)
25302533

25312534
static void zend_short_circuiting_mark_inner(zend_ast *ast) {
25322535
if (zend_ast_kind_is_short_circuited(ast->kind)) {
2536+
if (ast->attr & ZEND_PARENTHESIZED_NULLSAFE) {
2537+
return;
2538+
}
25332539
ast->attr |= ZEND_SHORT_CIRCUITING_INNER;
25342540
}
25352541
}
@@ -12120,6 +12126,11 @@ static zend_op *zend_delayed_compile_var(znode *result, zend_ast *ast, uint32_t
1212012126
{
1212112127
zend_check_stack_limit();
1212212128

12129+
if ((ast->kind == ZEND_AST_NULLSAFE_PROP || ast->kind == ZEND_AST_NULLSAFE_METHOD_CALL)
12130+
&& (ast->attr & ZEND_PARENTHESIZED_NULLSAFE)) {
12131+
return zend_compile_var(result, ast, type, by_ref);
12132+
}
12133+
1212312134
switch (ast->kind) {
1212412135
case ZEND_AST_VAR:
1212512136
return zend_compile_simple_var(result, ast, type, true);

Zend/zend_compile.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1224,6 +1224,9 @@ static zend_always_inline bool zend_check_arg_send_type(const zend_function *zf,
12241224
/* Used to disallow pipes with arrow functions that lead to confusing parse trees. */
12251225
#define ZEND_PARENTHESIZED_ARROW_FUNC 1
12261226

1227+
/* Used to contain nullsafe short-circuiting within parentheses */
1228+
#define ZEND_PARENTHESIZED_NULLSAFE 1
1229+
12271230
/* For "use" AST nodes and the seen symbol table */
12281231
#define ZEND_SYMBOL_CLASS (1<<0)
12291232
#define ZEND_SYMBOL_FUNCTION (1<<1)

Zend/zend_language_parser.y

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1349,6 +1349,8 @@ expr:
13491349
$$ = $2;
13501350
if ($$->kind == ZEND_AST_CONDITIONAL) $$->attr = ZEND_PARENTHESIZED_CONDITIONAL;
13511351
if ($$->kind == ZEND_AST_ARROW_FUNC) $$->attr = ZEND_PARENTHESIZED_ARROW_FUNC;
1352+
if ($$->kind == ZEND_AST_NULLSAFE_PROP) $$->attr = ZEND_PARENTHESIZED_NULLSAFE;
1353+
if ($$->kind == ZEND_AST_NULLSAFE_METHOD_CALL) $$->attr = ZEND_PARENTHESIZED_NULLSAFE;
13521354
}
13531355
| new_dereferenceable { $$ = $1; }
13541356
| new_non_dereferenceable { $$ = $1; }
@@ -1544,6 +1546,8 @@ fully_dereferenceable:
15441546
| '(' expr ')' {
15451547
$$ = $2;
15461548
if ($$->kind == ZEND_AST_STATIC_PROP) $$->attr = ZEND_PARENTHESIZED_STATIC_PROP;
1549+
if ($$->kind == ZEND_AST_NULLSAFE_PROP) $$->attr = ZEND_PARENTHESIZED_NULLSAFE;
1550+
if ($$->kind == ZEND_AST_NULLSAFE_METHOD_CALL) $$->attr = ZEND_PARENTHESIZED_NULLSAFE;
15471551
}
15481552
| dereferenceable_scalar { $$ = $1; }
15491553
| class_constant { $$ = $1; }
@@ -1557,7 +1561,11 @@ array_object_dereferenceable:
15571561

15581562
callable_expr:
15591563
callable_variable { $$ = $1; }
1560-
| '(' expr ')' { $$ = $2; }
1564+
| '(' expr ')' {
1565+
$$ = $2;
1566+
if ($$->kind == ZEND_AST_NULLSAFE_PROP) $$->attr = ZEND_PARENTHESIZED_NULLSAFE;
1567+
if ($$->kind == ZEND_AST_NULLSAFE_METHOD_CALL) $$->attr = ZEND_PARENTHESIZED_NULLSAFE;
1568+
}
15611569
| dereferenceable_scalar { $$ = $1; }
15621570
| new_dereferenceable { $$ = $1; }
15631571
;

0 commit comments

Comments
 (0)