Treat attribute argument lists like normal argument lists

nikic · nikic · commit 6a195cacf3aa · 2020-07-02T15:27:45.000+02:00
Allow trailing comma. Syntactically allow unpacking, but forbid it during compilation. The trailing comma test-case is adopted from GH-5796.
diff --git a/Zend/tests/attributes/026_unpack_in_args.phpt b/Zend/tests/attributes/026_unpack_in_args.phpt
@@ -0,0 +1,11 @@
+--TEST--
+Cannot use unpacking in attribute argument list
+--FILE--
+<?php
+
+<<MyAttribute(...[1, 2, 3])>>
+class Foo { }
+
+?>
+--EXPECTF--
+Fatal error: Cannot use unpacking in attribute argument list in %s on line %d
diff --git a/Zend/tests/attributes/027_trailing_comma_args.phpt b/Zend/tests/attributes/027_trailing_comma_args.phpt
@@ -0,0 +1,30 @@
+--TEST--
+Trailing comma in attribute argument list
+--FILE--
+<?php
+
+<<MyAttribute(
+	"there",
+	"are",
+	"many",
+	"arguments",
+)>>
+class Foo { }
+
+$ref = new \ReflectionClass(Foo::class);
+$attr = $ref->getAttributes()[0];
+var_dump($attr->getName(), $attr->getArguments());
+
+?>
+--EXPECT--
+string(11) "MyAttribute"
+array(4) {
+  [0]=>
+  string(5) "there"
+  [1]=>
+  string(3) "are"
+  [2]=>
+  string(4) "many"
+  [3]=>
+  string(9) "arguments"
+}
diff --git a/Zend/tests/varSyntax/globalNonSimpleVariableError.phpt b/Zend/tests/varSyntax/globalNonSimpleVariableError.phpt
@@ -7,4 +7,4 @@ global $$foo->bar;
 
 ?>
 --EXPECTF--
-Parse error: syntax error, unexpected '->' (T_OBJECT_OPERATOR), expecting ',' or ';' in %s on line %d
+Parse error: syntax error, unexpected '->' (T_OBJECT_OPERATOR), expecting ';' or ',' in %s on line %d
diff --git a/Zend/zend_compile.c b/Zend/zend_compile.c
@@ -5743,6 +5743,11 @@ static void zend_compile_attributes(HashTable **attributes, zend_ast *ast, uint3
 			ZEND_ASSERT(args->kind == ZEND_AST_ARG_LIST);
 
 			for (j = 0; j < args->children; j++) {
+				if (args->child[j]->kind == ZEND_AST_UNPACK) {
+					zend_error_noreturn(E_COMPILE_ERROR,
+						"Cannot use unpacking in attribute argument list");
+				}
+
 				zend_const_expr_to_zval(&attr->argv[j], args->child[j]);
 			}
 		}
diff --git a/Zend/zend_language_parser.y b/Zend/zend_language_parser.y
@@ -260,7 +260,7 @@ static YYSIZE_T zend_yytnamerr(char*, const char*);
 %type <ast> identifier type_expr_without_static union_type_without_static
 %type <ast> inline_function union_type
 %type <ast> attributed_statement attributed_class_statement attributed_parameter
-%type <ast> attribute_arguments attribute_decl attribute attributes
+%type <ast> attribute_decl attribute attributes
 
 %type <num> returns_ref function fn is_reference is_variadic variable_modifiers
 %type <num> method_modifiers non_empty_member_modifiers member_modifier optional_visibility_modifier
@@ -317,20 +317,11 @@ name:
 	|	T_NS_SEPARATOR namespace_name				{ $$ = $2; $$->attr = ZEND_NAME_FQ; }
 ;
 
-attribute_arguments:
-		expr
-			{ $$ = zend_ast_create_list(1, ZEND_AST_ARG_LIST, $1); }
-	|	attribute_arguments ',' expr
-			{ $$ = zend_ast_list_add($1, $3); }
-;
-
 attribute_decl:
 		class_name
 			{ $$ = zend_ast_create(ZEND_AST_ATTRIBUTE, $1, NULL); }
-	|	class_name '(' ')'
-			{ $$ = zend_ast_create(ZEND_AST_ATTRIBUTE, $1, NULL); }
-	|	class_name '(' attribute_arguments ')'
-			{ $$ = zend_ast_create(ZEND_AST_ATTRIBUTE, $1, $3); }
+	|	class_name argument_list
+			{ $$ = zend_ast_create(ZEND_AST_ATTRIBUTE, $1, $2); }
 ;
 
 attribute:

Original file line number	Diff line number	Diff line change
`@@ -5743,6 +5743,11 @@ static void zend_compile_attributes(HashTable *attributes, zend_ast ast, uint3`
`5743`	`5743`	`ZEND_ASSERT(args->kind == ZEND_AST_ARG_LIST);`
`5744`	`5744`
`5745`	`5745`	`for (j = 0; j < args->children; j++) {`
	`5746`	`+ if (args->child[j]->kind == ZEND_AST_UNPACK) {`
	`5747`	`+ zend_error_noreturn(E_COMPILE_ERROR,`
	`5748`	`+ "Cannot use unpacking in attribute argument list");`
	`5749`	`+ }`
	`5750`	`+`
`5746`	`5751`	`zend_const_expr_to_zval(&attr->argv[j], args->child[j]);`
`5747`	`5752`	`}`
`5748`	`5753`	`}`