Improvements in modifier parsing (php#9926)

iluuu1994 · web-flow · commit adfdfb2e1ed0 · 2022-11-17T16:20:27.000+01:00
Use a shared non-terminal for all class modifiers. This avoids conflicts when adding modifiers that are only valid for certain targets. This change is necessary for asymmetric visibility but might be useful for other future additions. Closes phpGH-9926
diff --git a/Zend/tests/access_modifiers_007.phpt b/Zend/tests/access_modifiers_007.phpt
@@ -10,4 +10,4 @@ class test {
 echo "Done\n";
 ?>
 --EXPECTF--
-Fatal error: Cannot use the final modifier on an abstract class member in %s on line %d
+Fatal error: Cannot use the final modifier on an abstract method in %s on line %d
diff --git a/Zend/tests/ctor_promotion_additional_modifiers.phpt b/Zend/tests/ctor_promotion_additional_modifiers.phpt
@@ -9,4 +9,4 @@ class Test {
 
 ?>
 --EXPECTF--
-Parse error: syntax error, unexpected token "static", expecting variable in %s on line %d
+Fatal error: Cannot use the static modifier on a promoted property in %s on line %d
diff --git a/Zend/tests/errmsg_037.phpt b/Zend/tests/errmsg_037.phpt
@@ -10,4 +10,4 @@ class test {
 echo "Done\n";
 ?>
 --EXPECTF--
-Fatal error: Properties cannot be declared abstract in %s on line %d
+Fatal error: Cannot use the abstract modifier on a property in %s on line %d
diff --git a/Zend/tests/errmsg_038.phpt b/Zend/tests/errmsg_038.phpt
@@ -10,4 +10,4 @@ class test {
 echo "Done\n";
 ?>
 --EXPECTF--
-Fatal error: Cannot declare property test::$var final, the final modifier is allowed only for methods, classes, and class constants in %s on line %d
+Fatal error: Cannot use the final modifier on a property in %s on line %d
diff --git a/Zend/tests/readonly_props/readonly_const.phpt b/Zend/tests/readonly_props/readonly_const.phpt
@@ -9,4 +9,4 @@ class Test {
 
 ?>
 --EXPECTF--
-Fatal error: Cannot use 'readonly' as constant modifier in %s on line %d
+Fatal error: Cannot use the readonly modifier on a class constant in %s on line %d
diff --git a/Zend/tests/readonly_props/readonly_method.phpt b/Zend/tests/readonly_props/readonly_method.phpt
@@ -9,4 +9,4 @@ class Test {
 
 ?>
 --EXPECTF--
-Fatal error: Cannot use 'readonly' as method modifier in %s on line %d
+Fatal error: Cannot use the readonly modifier on a method in %s on line %d
diff --git a/Zend/tests/readonly_props/readonly_method_trait.phpt b/Zend/tests/readonly_props/readonly_method_trait.phpt
@@ -9,4 +9,4 @@ class Test {
 
 ?>
 --EXPECTF--
-Fatal error: Cannot use 'readonly' as method modifier in %s on line %d
+Fatal error: Cannot use the readonly modifier on a method in %s on line %d
diff --git a/Zend/tests/traits/error_013.phpt b/Zend/tests/traits/error_013.phpt
@@ -16,4 +16,4 @@ var_dump($x->test());
 
 ?>
 --EXPECTF--
-Fatal error: Cannot use 'static' as method modifier in %s on line %d
+Fatal error: Cannot use "static" as method modifier in trait alias in %s on line %d
diff --git a/Zend/tests/traits/language018.phpt b/Zend/tests/traits/language018.phpt
@@ -12,4 +12,4 @@ class C1 {
 }
 ?>
 --EXPECTF--
-Fatal error: Cannot use 'abstract' as method modifier in %s on line %d
+Fatal error: Cannot use "abstract" as method modifier in trait alias in %s on line %d
diff --git a/Zend/tests/traits/language019.phpt b/Zend/tests/traits/language019.phpt
@@ -12,4 +12,4 @@ class C1 {
 }
 ?>
 --EXPECTF--
-Fatal error: Cannot use 'final' as method modifier in %s on line %d
+Fatal error: Cannot use "final" as method modifier in trait alias in %s on line %d
diff --git a/Zend/tests/type_declarations/static_type_param.phpt b/Zend/tests/type_declarations/static_type_param.phpt
@@ -12,4 +12,4 @@ class Test {
 
 ?>
 --EXPECTF--
-Parse error: syntax error, unexpected token "static", expecting variable in %s on line %d
+Fatal error: Cannot use the static modifier on a promoted property in %s on line %d
diff --git a/Zend/zend_ast.h b/Zend/zend_ast.h
@@ -66,6 +66,7 @@ enum _zend_ast_kind {
 	ZEND_AST_ATTRIBUTE_LIST,
 	ZEND_AST_ATTRIBUTE_GROUP,
 	ZEND_AST_MATCH_ARM_LIST,
+	ZEND_AST_MODIFIER_LIST,
 
 	/* 0 child nodes */
 	ZEND_AST_MAGIC_CONST = 0 << ZEND_AST_NUM_CHILDREN_SHIFT,
diff --git a/Zend/zend_compile.c b/Zend/zend_compile.c
@@ -787,6 +787,96 @@ static void zend_do_free(znode *op1) /* {{{ */
 }
 /* }}} */
 
+
+static char *zend_modifier_token_to_string(uint32_t token)
+{
+	switch (token) {
+		case T_PUBLIC:
+			return "public";
+		case T_PROTECTED:
+			return "protected";
+		case T_PRIVATE:
+			return "private";
+		case T_STATIC:
+			return "static";
+		case T_FINAL:
+			return "final";
+		case T_READONLY:
+			return "readonly";
+		case T_ABSTRACT:
+			return "abstract";
+		EMPTY_SWITCH_DEFAULT_CASE()
+	}
+}
+
+uint32_t zend_modifier_token_to_flag(zend_modifier_target target, uint32_t token)
+{
+	switch (token) {
+		case T_PUBLIC:
+			return ZEND_ACC_PUBLIC;
+		case T_PROTECTED:
+			return ZEND_ACC_PROTECTED;
+		case T_PRIVATE:
+			return ZEND_ACC_PRIVATE;
+		case T_READONLY:
+			if (target == ZEND_MODIFIER_TARGET_PROPERTY || target == ZEND_MODIFIER_TARGET_CPP) {
+				return ZEND_ACC_READONLY;
+			}
+			break;
+		case T_ABSTRACT:
+			if (target == ZEND_MODIFIER_TARGET_METHOD) {
+				return ZEND_ACC_ABSTRACT;
+			}
+			break;
+		case T_FINAL:
+			if (target == ZEND_MODIFIER_TARGET_METHOD || target == ZEND_MODIFIER_TARGET_CONSTANT) {
+				return ZEND_ACC_FINAL;
+			}
+			break;
+		case T_STATIC:
+			if (target == ZEND_MODIFIER_TARGET_PROPERTY || target == ZEND_MODIFIER_TARGET_METHOD) {
+				return ZEND_ACC_STATIC;
+			}
+			break;
+	}
+
+	char *member;
+	if (target == ZEND_MODIFIER_TARGET_PROPERTY) {
+		member = "property";
+	} else if (target == ZEND_MODIFIER_TARGET_METHOD) {
+		member = "method";
+	} else if (target == ZEND_MODIFIER_TARGET_CONSTANT) {
+		member = "class constant";
+	} else if (target == ZEND_MODIFIER_TARGET_CPP) {
+		member = "promoted property";
+	} else {
+		ZEND_UNREACHABLE();
+	}
+
+	zend_throw_exception_ex(zend_ce_compile_error, 0,
+		"Cannot use the %s modifier on a %s", zend_modifier_token_to_string(token), member);
+	return 0;
+}
+
+uint32_t zend_modifier_list_to_flags(zend_modifier_target target, zend_ast *modifiers)
+{
+	uint32_t flags = 0;
+	zend_ast_list *modifier_list = zend_ast_get_list(modifiers);
+
+	for (uint32_t i = 0; i < modifier_list->children; i++) {
+		uint32_t new_flag = zend_modifier_token_to_flag(target, (uint32_t) Z_LVAL_P(zend_ast_get_zval(modifier_list->child[i])));
+		if (!new_flag) {
+			return 0;
+		}
+		flags = zend_add_member_modifier(flags, new_flag, target);
+		if (!flags) {
+			return 0;
+		}
+	}
+
+	return flags;
+}
+
 uint32_t zend_add_class_modifier(uint32_t flags, uint32_t new_flag) /* {{{ */
 {
 	uint32_t new_flags = flags | new_flag;
@@ -812,7 +902,7 @@ uint32_t zend_add_class_modifier(uint32_t flags, uint32_t new_flag) /* {{{ */
 }
 /* }}} */
 
-uint32_t zend_add_member_modifier(uint32_t flags, uint32_t new_flag) /* {{{ */
+uint32_t zend_add_member_modifier(uint32_t flags, uint32_t new_flag, zend_modifier_target target) /* {{{ */
 {
 	uint32_t new_flags = flags | new_flag;
 	if ((flags & ZEND_ACC_PPP_MASK) && (new_flag & ZEND_ACC_PPP_MASK)) {
@@ -837,9 +927,9 @@ uint32_t zend_add_member_modifier(uint32_t flags, uint32_t new_flag) /* {{{ */
 			"Multiple readonly modifiers are not allowed", 0);
 		return 0;
 	}
-	if ((new_flags & ZEND_ACC_ABSTRACT) && (new_flags & ZEND_ACC_FINAL)) {
+	if (target == ZEND_MODIFIER_TARGET_METHOD && (new_flags & ZEND_ACC_ABSTRACT) && (new_flags & ZEND_ACC_FINAL)) {
 		zend_throw_exception(zend_ce_compile_error,
-			"Cannot use the final modifier on an abstract class member", 0);
+			"Cannot use the final modifier on an abstract method", 0);
 		return 0;
 	}
 	return new_flags;
@@ -7474,10 +7564,6 @@ static void zend_compile_prop_decl(zend_ast *ast, zend_ast *type_ast, uint32_t f
 		zend_error_noreturn(E_COMPILE_ERROR, "Enum %s cannot include properties", ZSTR_VAL(ce->name));
 	}
 
-	if (flags & ZEND_ACC_ABSTRACT) {
-		zend_error_noreturn(E_COMPILE_ERROR, "Properties cannot be declared abstract");
-	}
-
 	for (i = 0; i < children; ++i) {
 		zend_property_info *info;
 		zend_ast *prop_ast = list->child[i];
@@ -7505,12 +7591,6 @@ static void zend_compile_prop_decl(zend_ast *ast, zend_ast *type_ast, uint32_t f
 			doc_comment = zend_string_copy(zend_ast_get_str(doc_comment_ast));
 		}
 
-		if (flags & ZEND_ACC_FINAL) {
-			zend_error_noreturn(E_COMPILE_ERROR, "Cannot declare property %s::$%s final, "
-				"the final modifier is allowed only for methods, classes, and class constants",
-				ZSTR_VAL(ce->name), ZSTR_VAL(name));
-		}
-
 		if (zend_hash_exists(&ce->properties_info, name)) {
 			zend_error_noreturn(E_COMPILE_ERROR, "Cannot redeclare %s::$%s",
 				ZSTR_VAL(ce->name), ZSTR_VAL(name));
@@ -7583,16 +7663,14 @@ static void zend_compile_prop_group(zend_ast *ast) /* {{{ */
 }
 /* }}} */
 
-static void zend_check_const_and_trait_alias_attr(uint32_t attr, const char* entity) /* {{{ */
+static void zend_check_trait_alias_modifiers(uint32_t attr) /* {{{ */
 {
 	if (attr & ZEND_ACC_STATIC) {
-		zend_error_noreturn(E_COMPILE_ERROR, "Cannot use 'static' as %s modifier", entity);
+		zend_error_noreturn(E_COMPILE_ERROR, "Cannot use \"static\" as method modifier in trait alias");
 	} else if (attr & ZEND_ACC_ABSTRACT) {
-		zend_error_noreturn(E_COMPILE_ERROR, "Cannot use 'abstract' as %s modifier", entity);
+		zend_error_noreturn(E_COMPILE_ERROR, "Cannot use \"abstract\" as method modifier in trait alias");
 	} else if (attr & ZEND_ACC_FINAL) {
-		zend_error_noreturn(E_COMPILE_ERROR, "Cannot use 'final' as %s modifier", entity);
-	} else if (attr & ZEND_ACC_READONLY) {
-		zend_error_noreturn(E_COMPILE_ERROR, "Cannot use 'readonly' as %s modifier", entity);
+		zend_error_noreturn(E_COMPILE_ERROR, "Cannot use \"final\" as method modifier in trait alias");
 	}
 }
 /* }}} */
@@ -7613,10 +7691,6 @@ static void zend_compile_class_const_decl(zend_ast *ast, uint32_t flags, zend_as
 		zend_string *doc_comment = doc_comment_ast ? zend_string_copy(zend_ast_get_str(doc_comment_ast)) : NULL;
 		zval value_zv;
 
-		if (UNEXPECTED(flags & (ZEND_ACC_STATIC|ZEND_ACC_ABSTRACT|ZEND_ACC_READONLY))) {
-			zend_check_const_and_trait_alias_attr(flags, "constant");
-		}
-
 		if (UNEXPECTED((flags & ZEND_ACC_PRIVATE) && (flags & ZEND_ACC_FINAL))) {
 			zend_error_noreturn(
 				E_COMPILE_ERROR, "Private constant %s::%s cannot be final as it is not visible to other classes",
@@ -7687,7 +7761,7 @@ static void zend_compile_trait_alias(zend_ast *ast) /* {{{ */
 
 	zend_trait_alias *alias;
 
-	zend_check_const_and_trait_alias_attr(modifiers, "method");
+	zend_check_trait_alias_modifiers(modifiers);
 
 	alias = emalloc(sizeof(zend_trait_alias));
 	zend_compile_method_ref(method_ref_ast, &alias->trait_method);
diff --git a/Zend/zend_compile.h b/Zend/zend_compile.h
@@ -806,11 +806,22 @@ ZEND_API binary_op_type get_binary_op(int opcode);
 void zend_stop_lexing(void);
 void zend_emit_final_return(bool return_one);
 
+typedef enum {
+	ZEND_MODIFIER_TARGET_PROPERTY = 0,
+	ZEND_MODIFIER_TARGET_METHOD,
+	ZEND_MODIFIER_TARGET_CONSTANT,
+	ZEND_MODIFIER_TARGET_CPP,
+} zend_modifier_target;
+
 /* Used during AST construction */
 zend_ast *zend_ast_append_str(zend_ast *left, zend_ast *right);
 zend_ast *zend_negate_num_string(zend_ast *ast);
 uint32_t zend_add_class_modifier(uint32_t flags, uint32_t new_flag);
-uint32_t zend_add_member_modifier(uint32_t flags, uint32_t new_flag);
+uint32_t zend_add_member_modifier(uint32_t flags, uint32_t new_flag, zend_modifier_target target);
+
+uint32_t zend_modifier_token_to_flag(zend_modifier_target target, uint32_t flags);
+uint32_t zend_modifier_list_to_flags(zend_modifier_target target, zend_ast *modifiers);
+
 bool zend_handle_encoding_declaration(zend_ast *ast);
 
 ZEND_API zend_class_entry *zend_bind_class_in_slot(
diff --git a/Zend/zend_language_parser.y b/Zend/zend_language_parser.y
diff --git a/ext/reflection/tests/bug74454.phpt b/ext/reflection/tests/bug74454.phpt
diff --git a/tests/classes/abstract_final.phpt b/tests/classes/abstract_final.phpt
diff --git a/tests/classes/constants_visibility_005.phpt b/tests/classes/constants_visibility_005.phpt
diff --git a/tests/classes/constants_visibility_006.phpt b/tests/classes/constants_visibility_006.phpt
diff --git a/tests/classes/final_abstract.phpt b/tests/classes/final_abstract.phpt
diff --git a/tests/classes/interface_method_final.phpt b/tests/classes/interface_method_final.phpt

Original file line number	Diff line number	Diff line change
`@@ -12,4 +12,4 @@ class C1 {`
`12`	`12`	`}`
`13`	`13`	`?>`
`14`	`14`	`--EXPECTF--`
`15`		`-Fatal error: Cannot use 'abstract' as method modifier in %s on line %d`
	`15`	`+Fatal error: Cannot use "abstract" as method modifier in trait alias in %s on line %d`