Prohibit pipe & arrow function combination that leads to confusing parse trees

See https://externals.io/message/128473
Closes GH-19533

NEWS

@@ -27,6 +27,8 @@ PHP                                                                        NEWS
   . Various closure binding issues are now deprecated. (alexandre-daubois)
   . Fixed bug GH-18373 (Don't substitute self/parent with anonymous class).
     (ilutov)
+  . Prohibit pipe & arrow function combination that leads to confusing parse
+    trees. (ilutov)
 
 - Filter:
   . Added support for configuring the URI parser for FILTER_VALIDATE_URL

Zend/tests/arrow_functions/gh7900.phpt

@@ -23,4 +23,4 @@ try {
 ?>
 --EXPECT--
 Here
-assert(fn(): never => 42 && false)
+assert((fn(): never => 42) && false)

Zend/tests/pipe_operator/mixed_callable_call.phpt

@@ -71,7 +71,7 @@ $res1 = 1
     |> [StaticTest::class, 'times17']
     |> new Times23()
     |> $times29
-    |> fn($x) => times2($x)
+    |> (fn($x) => times2($x))
 ;
 
 var_dump($res1);

Zend/tests/pipe_operator/prec_001.phpt

@@ -0,0 +1,12 @@
+--TEST--
+Pipe precedence 001
+--FILE--
+<?php
+
+42
+    |> fn($x) => $x < 42
+    |> fn($x) => var_dump($x);
+
+?>
+--EXPECTF--
+Fatal error: Arrow functions on the right hand side of |> must be parenthesized in %s on line %d

Zend/tests/pipe_operator/prec_002.phpt

@@ -0,0 +1,12 @@
+--TEST--
+Pipe precedence 002
+--FILE--
+<?php
+
+42
+    |> (fn($x) => $x < 42)
+    |> (fn($x) => var_dump($x));
+
+?>
+--EXPECT--
+bool(false)

Zend/tests/pipe_operator/prec_003.phpt

@@ -0,0 +1,12 @@
+--TEST--
+Pipe precedence 003
+--FILE--
+<?php
+
+null
+    |> fn() => print (new Exception)->getTraceAsString() . "\n\n"
+    |> fn() => print (new Exception)->getTraceAsString() . "\n\n";
+
+?>
+--EXPECTF--
+Fatal error: Arrow functions on the right hand side of |> must be parenthesized in %s on line %d

Zend/tests/pipe_operator/prec_004.phpt

@@ -0,0 +1,16 @@
+--TEST--
+Pipe precedence 004
+--FILE--
+<?php
+
+null
+    |> (fn() => print (new Exception)->getTraceAsString() . "\n\n")
+    |> (fn() => print (new Exception)->getTraceAsString() . "\n\n");
+
+?>
+--EXPECTF--
+#0 %s(%d): {closure:%s:%d}(NULL)
+#1 {main}
+
+#0 %s(%d): {closure:%s:%d}(1)
+#1 {main}

Zend/tests/pipe_operator/prec_005.phpt

@@ -0,0 +1,14 @@
+--TEST--
+Pipe precedence 005
+--FILE--
+<?php
+
+try {
+    assert(false && 1 |> (fn() => 2));
+} catch (AssertionError $e) {
+    echo $e->getMessage(), "\n";
+}
+
+?>
+--EXPECT--
+assert(false && 1 |> (fn() => 2))

Zend/tests/pipe_operator/prec_006.phpt

@@ -0,0 +1,13 @@
+--TEST--
+Pipe precedence 006
+--FILE--
+<?php
+
+$value = null;
+$value
+    |> fn ($x) => $x ?? throw new Exception('Value may not be null')
+    |> fn ($x) => var_dump($x);
+
+?>
+--EXPECTF--
+Fatal error: Arrow functions on the right hand side of |> must be parenthesized in %s on line %d

Zend/tests/pipe_operator/prec_007.phpt

@@ -0,0 +1,24 @@
+--TEST--
+Pipe precedence 007
+--FILE--
+<?php
+
+$value = 42;
+$value
+    |> (fn ($x) => $x ?? throw new Exception('Value may not be null'))
+    |> (fn ($x) => var_dump($x));
+
+$value = null;
+$value
+    |> (fn ($x) => $x ?? throw new Exception('Value may not be null'))
+    |> (fn ($x) => var_dump($x));
+
+?>
+--EXPECTF--
+int(42)
+
+Fatal error: Uncaught Exception: Value may not be null in %s:%d
+Stack trace:
+#0 %s(%d): {closure:%s:%d}(NULL)
+#1 {main}
+  thrown in %s on line %d

Zend/zend_ast.c

@@ -2070,6 +2070,9 @@ tail_call:
 		case ZEND_AST_ARROW_FUNC:
 		case ZEND_AST_METHOD:
 			decl = (const zend_ast_decl *) ast;
+			if (decl->kind == ZEND_AST_ARROW_FUNC && (decl->attr & ZEND_PARENTHESIZED_ARROW_FUNC)) {
+				smart_str_appendc(str, '(');
+			}
 			if (decl->child[4]) {
 				bool newlines = !(ast->kind == ZEND_AST_CLOSURE || ast->kind == ZEND_AST_ARROW_FUNC);
 				zend_ast_export_attributes(str, decl->child[4], indent, newlines);
@@ -2113,6 +2116,9 @@ tail_call:
 					}
 					smart_str_appends(str, " => ");
 					zend_ast_export_ex(str, body, 0, indent);
+					if (decl->attr & ZEND_PARENTHESIZED_ARROW_FUNC) {
+						smart_str_appendc(str, ')');
+					}
 					break;
 				}
 

Zend/zend_ast.h

@@ -220,7 +220,7 @@ typedef struct _zend_ast_op_array {
 /* Separate structure for function and class declaration, as they need extra information. */
 typedef struct _zend_ast_decl {
 	zend_ast_kind kind;
-	zend_ast_attr attr; /* Unused - for structure compatibility */
+	zend_ast_attr attr;
 	uint32_t start_lineno;
 	uint32_t end_lineno;
 	uint32_t flags;

Zend/zend_compile.c

@@ -6467,6 +6467,10 @@ static void zend_compile_pipe(znode *result, zend_ast *ast)
 	zend_ast *operand_ast = ast->child[0];
 	zend_ast *callable_ast = ast->child[1];
 
+	if (callable_ast->kind == ZEND_AST_ARROW_FUNC && !(callable_ast->attr & ZEND_PARENTHESIZED_ARROW_FUNC)) {
+		zend_error_noreturn(E_COMPILE_ERROR, "Arrow functions on the right hand side of |> must be parenthesized");
+	}
+
 	/* Compile the left hand side down to a value first. */
 	znode operand_result;
 	zend_compile_expr(&operand_result, operand_ast);

Zend/zend_compile.h

@@ -1207,6 +1207,9 @@ static zend_always_inline bool zend_check_arg_send_type(const zend_function *zf,
 /* Used to distinguish (parent::$prop)::get() from parent hook call. */
 #define ZEND_PARENTHESIZED_STATIC_PROP 1
 
+/* Used to disallow pipes with arrow functions that lead to confusing parse trees. */
+#define ZEND_PARENTHESIZED_ARROW_FUNC 1
+
 /* For "use" AST nodes and the seen symbol table */
 #define ZEND_SYMBOL_CLASS    (1<<0)
 #define ZEND_SYMBOL_FUNCTION (1<<1)

Zend/zend_language_parser.y

@@ -1348,6 +1348,7 @@ expr:
 	|	'(' expr ')' {
 			$$ = $2;
 			if ($$->kind == ZEND_AST_CONDITIONAL) $$->attr = ZEND_PARENTHESIZED_CONDITIONAL;
+			if ($$->kind == ZEND_AST_ARROW_FUNC) $$->attr = ZEND_PARENTHESIZED_ARROW_FUNC;
 		}
 	|	new_dereferenceable { $$ = $1; }
 	|	new_non_dereferenceable { $$ = $1; }