gron                                     Thu, 17 Nov 2011 21:04:15 +0000

Revision: http://svn.php.net/viewvc?view=revision&revision=319420

Log:
Fixed Bug #60165 (Aliasing unexisting trait should throw/trigger the 
exception/error)

- aliases that are not actually matching anything are treated as errors now. 
This
  will make sure that all methods that are expected to be in a class are 
actually
  there, or in case a trait changed for instance, that the code breaks already
  on composition
- Precedence declarations are also checked to ensure that the method
  which is supposed to take precedence actually exists, however,
  the other traits mentioned in the declaration are not regarded.
  We are more lenient here, since this avoids unnecessary fragility.
- fixed another seamingly unrelated test which broke in the progress
  but wasn't clear before either.

Bug: https://bugs.php.net/60165 (Assigned) Overriding unexisting trait should 
throw/trigger the exception/error
      
Changed paths:
    A   php/php-src/branches/PHP_5_4/Zend/tests/traits/bug60165a.phpt
    A   php/php-src/branches/PHP_5_4/Zend/tests/traits/bug60165b.phpt
    A   php/php-src/branches/PHP_5_4/Zend/tests/traits/bug60165c.phpt
    A   php/php-src/branches/PHP_5_4/Zend/tests/traits/bug60165d.phpt
    U   php/php-src/branches/PHP_5_4/Zend/tests/traits/language011.phpt
    U   php/php-src/branches/PHP_5_4/Zend/zend_compile.c
    A   php/php-src/trunk/Zend/tests/traits/bug60165a.phpt
    A   php/php-src/trunk/Zend/tests/traits/bug60165b.phpt
    A   php/php-src/trunk/Zend/tests/traits/bug60165c.phpt
    A   php/php-src/trunk/Zend/tests/traits/bug60165d.phpt
    U   php/php-src/trunk/Zend/tests/traits/language011.phpt
    U   php/php-src/trunk/Zend/zend_compile.c

Added: php/php-src/branches/PHP_5_4/Zend/tests/traits/bug60165a.phpt
===================================================================
--- php/php-src/branches/PHP_5_4/Zend/tests/traits/bug60165a.phpt	                        (rev 0)
+++ php/php-src/branches/PHP_5_4/Zend/tests/traits/bug60165a.phpt	2011-11-17 21:04:15 UTC (rev 319420)
@@ -0,0 +1,17 @@
+--TEST--
+Bug #60165 (Aliasing unexisting trait should throw/trigger the exception/error)
+--FILE--
+<?php
+
+trait A {
+    public function bar() {}
+}
+
+class MyClass {
+    use A {
+        nonExistent as barA;
+    }
+}
+
+--EXPECTF--
+Fatal error: An alias (barA) was defined for method nonExistent(), but this method does not exist in %s on line %d

Added: php/php-src/branches/PHP_5_4/Zend/tests/traits/bug60165b.phpt
===================================================================
--- php/php-src/branches/PHP_5_4/Zend/tests/traits/bug60165b.phpt	                        (rev 0)
+++ php/php-src/branches/PHP_5_4/Zend/tests/traits/bug60165b.phpt	2011-11-17 21:04:15 UTC (rev 319420)
@@ -0,0 +1,17 @@
+--TEST--
+Bug #60165 (Aliasing unexisting trait should throw/trigger the exception/error)
+--FILE--
+<?php
+
+trait A {
+    public function bar() {}
+}
+
+class MyClass {
+    use A {
+        A::nonExistent as barA;
+    }
+}
+
+--EXPECTF--
+Fatal error: An alias was defined for A::nonExistent but this method does not exist in %s on line %d

Added: php/php-src/branches/PHP_5_4/Zend/tests/traits/bug60165c.phpt
===================================================================
--- php/php-src/branches/PHP_5_4/Zend/tests/traits/bug60165c.phpt	                        (rev 0)
+++ php/php-src/branches/PHP_5_4/Zend/tests/traits/bug60165c.phpt	2011-11-17 21:04:15 UTC (rev 319420)
@@ -0,0 +1,22 @@
+--TEST--
+Bug #60165 (Aliasing unexisting trait should throw/trigger the exception/error)
+--FILE--
+<?php
+
+trait A {
+    public function bar() {}
+}
+
+trait B {
+    public function foo() {}
+}
+
+class MyClass {
+    use A, B {
+        foo as fooB;
+        baz as foobar;
+    }
+}
+
+--EXPECTF--
+Fatal error: An alias (foobar) was defined for method baz(), but this method does not exist in %s on line %d

Added: php/php-src/branches/PHP_5_4/Zend/tests/traits/bug60165d.phpt
===================================================================
--- php/php-src/branches/PHP_5_4/Zend/tests/traits/bug60165d.phpt	                        (rev 0)
+++ php/php-src/branches/PHP_5_4/Zend/tests/traits/bug60165d.phpt	2011-11-17 21:04:15 UTC (rev 319420)
@@ -0,0 +1,21 @@
+--TEST--
+Bug #60165 (Aliasing unexisting trait should throw/trigger the exception/error)
+--FILE--
+<?php
+
+// The same is true for the insteadof operator to resolve conflicts
+
+trait A {}
+
+trait B {
+    public function bar() {}
+}
+
+class MyClass {
+    use A, B {
+        A::bar insteadof B;
+    }
+}
+
+--EXPECTF--
+Fatal error: A precedence rule was defined for A::bar but this method does not exist in %s on line %d

Modified: php/php-src/branches/PHP_5_4/Zend/tests/traits/language011.phpt
===================================================================
--- php/php-src/branches/PHP_5_4/Zend/tests/traits/language011.phpt	2011-11-17 20:43:56 UTC (rev 319419)
+++ php/php-src/branches/PHP_5_4/Zend/tests/traits/language011.phpt	2011-11-17 21:04:15 UTC (rev 319420)
@@ -1,5 +1,5 @@
 --TEST--
-Aliasing leading to conflict should result in error message
+Aliasing on conflicting method should not cover up conflict.
 --FILE--
 <?php
 error_reporting(E_ALL);
@@ -27,4 +27,4 @@

 ?>
 --EXPECTF--
-Fatal error: Trait method sayWorld has not been applied, because there are collisions with other trait methods on MyClass in %s on line %d
+Fatal error: Trait method sayHello has not been applied, because there are collisions with other trait methods on MyClass in %s on line %d

Modified: php/php-src/branches/PHP_5_4/Zend/zend_compile.c
===================================================================
--- php/php-src/branches/PHP_5_4/Zend/zend_compile.c	2011-11-17 20:43:56 UTC (rev 319419)
+++ php/php-src/branches/PHP_5_4/Zend/zend_compile.c	2011-11-17 21:04:15 UTC (rev 319420)
@@ -3980,6 +3980,11 @@
 					zend_error(E_COMPILE_ERROR, "Failed to add aliased trait method (%s) to the trait table. There is probably already a trait method with the same name", fn_copy.common.function_name);
 				}
 				efree(lcname);
+
+				/** Record the trait from which this alias was resolved. */
+				if (!aliases[i]->trait_method->ce) {
+					aliases[i]->trait_method->ce = fn->common.scope;
+				}
 			}
 			i++;
 		}
@@ -4008,6 +4013,11 @@
 						fn_copy.common.fn_flags |= ZEND_ACC_PUBLIC;
 					}
 					fn_copy.common.fn_flags |= fn->common.fn_flags ^ (fn->common.fn_flags & ZEND_ACC_PPP_MASK);
+
+					/** Record the trait from which this alias was resolved. */
+					if (!aliases[i]->trait_method->ce) {
+						aliases[i]->trait_method->ce = fn->common.scope;
+					}
 				}
 				i++;
 			}
@@ -4036,14 +4046,36 @@
 	size_t i, j = 0;
 	zend_trait_precedence *cur_precedence;
 	zend_trait_method_reference *cur_method_ref;
+	char *lcname;
+	bool method_exists;

 	/* resolve class references */
 	if (ce->trait_precedences) {
 		i = 0;
 		while ((cur_precedence = ce->trait_precedences[i])) {
+			/** Resolve classes for all precedence operations. */
 			if (cur_precedence->exclude_from_classes) {
-				cur_precedence->trait_method->ce = zend_fetch_class(cur_precedence->trait_method->class_name, cur_precedence->trait_method->cname_len, ZEND_FETCH_CLASS_TRAIT TSRMLS_CC);
+				cur_method_ref = cur_precedence->trait_method;
+				cur_precedence->trait_method->ce = zend_fetch_class(cur_method_ref->class_name,
+																	cur_method_ref->cname_len, ZEND_FETCH_CLASS_TRAIT TSRMLS_CC);
+
+				/** Ensure that the prefered method is actually available. */
+				lcname = zend_str_tolower_dup(cur_method_ref->method_name,
+											  cur_method_ref->mname_len);
+				method_exists = zend_hash_exists(&cur_method_ref->ce->function_table,
+												 lcname,
+												 cur_method_ref->mname_len + 1);
+				efree(lcname);
+				if (!method_exists) {
+					zend_error(E_COMPILE_ERROR,
+							   "A precedence rule was defined for %s::%s but this method does not exist",
+							   cur_method_ref->ce->name,
+							   cur_method_ref->method_name);
+				}

+				/** With the other traits, we are more permissive.
+					We do not give errors for those. This allows to be more
+					defensive in such definitions. */
 				j = 0;
 				while (cur_precedence->exclude_from_classes[j]) {
 					char* class_name = (char*)cur_precedence->exclude_from_classes[j];
@@ -4061,9 +4093,21 @@
 	if (ce->trait_aliases) {
 		i = 0;
 		while (ce->trait_aliases[i]) {
+			/** For all aliases with an explicit class name, resolve the class now. */
 			if (ce->trait_aliases[i]->trait_method->class_name) {
 				cur_method_ref = ce->trait_aliases[i]->trait_method;
 				cur_method_ref->ce = zend_fetch_class(cur_method_ref->class_name, cur_method_ref->cname_len, ZEND_FETCH_CLASS_TRAIT TSRMLS_CC);
+
+				/** And, ensure that the referenced method is resolvable, too. */
+				lcname = zend_str_tolower_dup(cur_method_ref->method_name,
+                                      cur_method_ref->mname_len);
+				method_exists = zend_hash_exists(&cur_method_ref->ce->function_table,
+                                         lcname, cur_method_ref->mname_len + 1);
+				efree(lcname);
+
+				if (!method_exists) {
+					zend_error(E_COMPILE_ERROR, "An alias was defined for %s::%s but this method does not exist", cur_method_ref->ce->name, cur_method_ref->method_name);
+				}
 			}
 			i++;
 		}
@@ -4273,6 +4317,26 @@
 }
 /* }}} */

+static void zend_do_check_for_inconsistent_traits_aliasing(zend_class_entry *ce TSRMLS_DC) /* {{{ */
+{
+	int i = 0;
+
+	if (ce->trait_aliases) {
+		while (ce->trait_aliases[i]) {
+			/** The trait for this alias has not been resolved, this means, this
+				alias was not applied. Abort with an error. */
+			if (!ce->trait_aliases[i]->trait_method->ce) {
+				zend_error(E_COMPILE_ERROR,
+						   "An alias (%s) was defined for method %s(), but this method does not exist",
+						   ce->trait_aliases[i]->alias,
+						   ce->trait_aliases[i]->trait_method->method_name);
+			}
+			i++;
+		}
+	}
+}
+/* }}} */
+
 ZEND_API void zend_do_bind_traits(zend_class_entry *ce TSRMLS_DC) /* {{{ */
 {

@@ -4285,6 +4349,9 @@

 	/* first care about all methods to be flattened into the class */
 	zend_do_traits_method_binding(ce TSRMLS_CC);
+
+	/* Aliases which have not been applied indicate typos/bugs. */
+	zend_do_check_for_inconsistent_traits_aliasing(ce TSRMLS_CC);

 	/* then flatten the properties into it, to, mostly to notfiy developer about problems */
 	zend_do_traits_property_binding(ce TSRMLS_CC);

Added: php/php-src/trunk/Zend/tests/traits/bug60165a.phpt
===================================================================
--- php/php-src/trunk/Zend/tests/traits/bug60165a.phpt	                        (rev 0)
+++ php/php-src/trunk/Zend/tests/traits/bug60165a.phpt	2011-11-17 21:04:15 UTC (rev 319420)
@@ -0,0 +1,17 @@
+--TEST--
+Bug #60165 (Aliasing unexisting trait should throw/trigger the exception/error)
+--FILE--
+<?php
+
+trait A {
+    public function bar() {}
+}
+
+class MyClass {
+    use A {
+        nonExistent as barA;
+    }
+}
+
+--EXPECTF--
+Fatal error: An alias (barA) was defined for method nonExistent(), but this method does not exist in %s on line %d

Added: php/php-src/trunk/Zend/tests/traits/bug60165b.phpt
===================================================================
--- php/php-src/trunk/Zend/tests/traits/bug60165b.phpt	                        (rev 0)
+++ php/php-src/trunk/Zend/tests/traits/bug60165b.phpt	2011-11-17 21:04:15 UTC (rev 319420)
@@ -0,0 +1,17 @@
+--TEST--
+Bug #60165 (Aliasing unexisting trait should throw/trigger the exception/error)
+--FILE--
+<?php
+
+trait A {
+    public function bar() {}
+}
+
+class MyClass {
+    use A {
+        A::nonExistent as barA;
+    }
+}
+
+--EXPECTF--
+Fatal error: An alias was defined for A::nonExistent but this method does not exist in %s on line %d

Added: php/php-src/trunk/Zend/tests/traits/bug60165c.phpt
===================================================================
--- php/php-src/trunk/Zend/tests/traits/bug60165c.phpt	                        (rev 0)
+++ php/php-src/trunk/Zend/tests/traits/bug60165c.phpt	2011-11-17 21:04:15 UTC (rev 319420)
@@ -0,0 +1,22 @@
+--TEST--
+Bug #60165 (Aliasing unexisting trait should throw/trigger the exception/error)
+--FILE--
+<?php
+
+trait A {
+    public function bar() {}
+}
+
+trait B {
+    public function foo() {}
+}
+
+class MyClass {
+    use A, B {
+        foo as fooB;
+        baz as foobar;
+    }
+}
+
+--EXPECTF--
+Fatal error: An alias (foobar) was defined for method baz(), but this method does not exist in %s on line %d

Added: php/php-src/trunk/Zend/tests/traits/bug60165d.phpt
===================================================================
--- php/php-src/trunk/Zend/tests/traits/bug60165d.phpt	                        (rev 0)
+++ php/php-src/trunk/Zend/tests/traits/bug60165d.phpt	2011-11-17 21:04:15 UTC (rev 319420)
@@ -0,0 +1,21 @@
+--TEST--
+Bug #60165 (Aliasing unexisting trait should throw/trigger the exception/error)
+--FILE--
+<?php
+
+// The same is true for the insteadof operator to resolve conflicts
+
+trait A {}
+
+trait B {
+    public function bar() {}
+}
+
+class MyClass {
+    use A, B {
+        A::bar insteadof B;
+    }
+}
+
+--EXPECTF--
+Fatal error: A precedence rule was defined for A::bar but this method does not exist in %s on line %d

Modified: php/php-src/trunk/Zend/tests/traits/language011.phpt
===================================================================
--- php/php-src/trunk/Zend/tests/traits/language011.phpt	2011-11-17 20:43:56 UTC (rev 319419)
+++ php/php-src/trunk/Zend/tests/traits/language011.phpt	2011-11-17 21:04:15 UTC (rev 319420)
@@ -1,5 +1,5 @@
 --TEST--
-Aliasing leading to conflict should result in error message
+Aliasing on conflicting method should not cover up conflict.
 --FILE--
 <?php
 error_reporting(E_ALL);
@@ -27,4 +27,4 @@

 ?>
 --EXPECTF--
-Fatal error: Trait method sayWorld has not been applied, because there are collisions with other trait methods on MyClass in %s on line %d
+Fatal error: Trait method sayHello has not been applied, because there are collisions with other trait methods on MyClass in %s on line %d

Modified: php/php-src/trunk/Zend/zend_compile.c
===================================================================
--- php/php-src/trunk/Zend/zend_compile.c	2011-11-17 20:43:56 UTC (rev 319419)
+++ php/php-src/trunk/Zend/zend_compile.c	2011-11-17 21:04:15 UTC (rev 319420)
@@ -3980,6 +3980,11 @@
 					zend_error(E_COMPILE_ERROR, "Failed to add aliased trait method (%s) to the trait table. There is probably already a trait method with the same name", fn_copy.common.function_name);
 				}
 				efree(lcname);
+
+				/** Record the trait from which this alias was resolved. */
+				if (!aliases[i]->trait_method->ce) {
+					aliases[i]->trait_method->ce = fn->common.scope;
+				}
 			}
 			i++;
 		}
@@ -4008,6 +4013,11 @@
 						fn_copy.common.fn_flags |= ZEND_ACC_PUBLIC;
 					}
 					fn_copy.common.fn_flags |= fn->common.fn_flags ^ (fn->common.fn_flags & ZEND_ACC_PPP_MASK);
+
+					/** Record the trait from which this alias was resolved. */
+					if (!aliases[i]->trait_method->ce) {
+						aliases[i]->trait_method->ce = fn->common.scope;
+					}
 				}
 				i++;
 			}
@@ -4036,14 +4046,36 @@
 	size_t i, j = 0;
 	zend_trait_precedence *cur_precedence;
 	zend_trait_method_reference *cur_method_ref;
+	char *lcname;
+	bool method_exists;

 	/* resolve class references */
 	if (ce->trait_precedences) {
 		i = 0;
 		while ((cur_precedence = ce->trait_precedences[i])) {
+			/** Resolve classes for all precedence operations. */
 			if (cur_precedence->exclude_from_classes) {
-				cur_precedence->trait_method->ce = zend_fetch_class(cur_precedence->trait_method->class_name, cur_precedence->trait_method->cname_len, ZEND_FETCH_CLASS_TRAIT TSRMLS_CC);
+				cur_method_ref = cur_precedence->trait_method;
+				cur_precedence->trait_method->ce = zend_fetch_class(cur_method_ref->class_name,
+																	cur_method_ref->cname_len, ZEND_FETCH_CLASS_TRAIT TSRMLS_CC);
+
+				/** Ensure that the prefered method is actually available. */
+				lcname = zend_str_tolower_dup(cur_method_ref->method_name,
+											  cur_method_ref->mname_len);
+				method_exists = zend_hash_exists(&cur_method_ref->ce->function_table,
+												 lcname,
+												 cur_method_ref->mname_len + 1);
+				efree(lcname);
+				if (!method_exists) {
+					zend_error(E_COMPILE_ERROR,
+							   "A precedence rule was defined for %s::%s but this method does not exist",
+							   cur_method_ref->ce->name,
+							   cur_method_ref->method_name);
+				}

+				/** With the other traits, we are more permissive.
+					We do not give errors for those. This allows to be more
+					defensive in such definitions. */
 				j = 0;
 				while (cur_precedence->exclude_from_classes[j]) {
 					char* class_name = (char*)cur_precedence->exclude_from_classes[j];
@@ -4061,9 +4093,21 @@
 	if (ce->trait_aliases) {
 		i = 0;
 		while (ce->trait_aliases[i]) {
+			/** For all aliases with an explicit class name, resolve the class now. */
 			if (ce->trait_aliases[i]->trait_method->class_name) {
 				cur_method_ref = ce->trait_aliases[i]->trait_method;
 				cur_method_ref->ce = zend_fetch_class(cur_method_ref->class_name, cur_method_ref->cname_len, ZEND_FETCH_CLASS_TRAIT TSRMLS_CC);
+
+				/** And, ensure that the referenced method is resolvable, too. */
+				lcname = zend_str_tolower_dup(cur_method_ref->method_name,
+                                      cur_method_ref->mname_len);
+				method_exists = zend_hash_exists(&cur_method_ref->ce->function_table,
+                                         lcname, cur_method_ref->mname_len + 1);
+				efree(lcname);
+
+				if (!method_exists) {
+					zend_error(E_COMPILE_ERROR, "An alias was defined for %s::%s but this method does not exist", cur_method_ref->ce->name, cur_method_ref->method_name);
+				}
 			}
 			i++;
 		}
@@ -4273,6 +4317,26 @@
 }
 /* }}} */

+static void zend_do_check_for_inconsistent_traits_aliasing(zend_class_entry *ce TSRMLS_DC) /* {{{ */
+{
+	int i = 0;
+
+	if (ce->trait_aliases) {
+		while (ce->trait_aliases[i]) {
+			/** The trait for this alias has not been resolved, this means, this
+				alias was not applied. Abort with an error. */
+			if (!ce->trait_aliases[i]->trait_method->ce) {
+				zend_error(E_COMPILE_ERROR,
+						   "An alias (%s) was defined for method %s(), but this method does not exist",
+						   ce->trait_aliases[i]->alias,
+						   ce->trait_aliases[i]->trait_method->method_name);
+			}
+			i++;
+		}
+	}
+}
+/* }}} */
+
 ZEND_API void zend_do_bind_traits(zend_class_entry *ce TSRMLS_DC) /* {{{ */
 {

@@ -4285,6 +4349,9 @@

 	/* first care about all methods to be flattened into the class */
 	zend_do_traits_method_binding(ce TSRMLS_CC);
+
+	/* Aliases which have not been applied indicate typos/bugs. */
+	zend_do_check_for_inconsistent_traits_aliasing(ce TSRMLS_CC);

 	/* then flatten the properties into it, to, mostly to notfiy developer about problems */
 	zend_do_traits_property_binding(ce TSRMLS_CC);
-- 
PHP CVS Mailing List (http://www.php.net/)
To unsubscribe, visit: http://www.php.net/unsub.php

Reply via email to