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.

diff --git a/Zend/tests/traits/bug60165a.phpt b/Zend/tests/traits/bug60165a.phpt
new file mode 100644 (file)
index 0000000..245bb94
--- /dev/null
+++ b/Zend/tests/traits/bug60165a.phpt
@@ -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

diff --git a/Zend/tests/traits/bug60165b.phpt b/Zend/tests/traits/bug60165b.phpt
new file mode 100644 (file)
index 0000000..7b4855a
--- /dev/null
+++ b/Zend/tests/traits/bug60165b.phpt
@@ -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

diff --git a/Zend/tests/traits/bug60165c.phpt b/Zend/tests/traits/bug60165c.phpt
new file mode 100644 (file)
index 0000000..d72491f
--- /dev/null
+++ b/Zend/tests/traits/bug60165c.phpt
@@ -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

diff --git a/Zend/tests/traits/bug60165d.phpt b/Zend/tests/traits/bug60165d.phpt
new file mode 100644 (file)
index 0000000..26ac927
--- /dev/null
+++ b/Zend/tests/traits/bug60165d.phpt
@@ -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

diff --git a/Zend/tests/traits/language011.phpt b/Zend/tests/traits/language011.phpt
index cf535b4c115ee741118ec0077f3e5677e0ccec62..585699da559f0c962564fd2b696150013aaabf59 100644 (file)
--- a/Zend/tests/traits/language011.phpt
+++ b/Zend/tests/traits/language011.phpt
@@ -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 @@ $o->sayWorld();
 
 ?>
 --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

diff --git a/Zend/zend_compile.c b/Zend/zend_compile.c
index dde71ed549e37164ace81114160614b57b250320..a3ff84750fa888eae4c4d976c80cbd9ca398d9b1 100644 (file)
--- a/Zend/zend_compile.c
+++ b/Zend/zend_compile.c
@@ -3980,6 +3980,11 @@ static int zend_traits_copy_functions(zend_function *fn TSRMLS_DC, int num_args,
                                        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 @@ static int zend_traits_copy_functions(zend_function *fn TSRMLS_DC, int num_args,
                                                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 @@ static void zend_traits_init_trait_structures(zend_class_entry *ce TSRMLS_DC) /*
        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 @@ static void zend_traits_init_trait_structures(zend_class_entry *ce TSRMLS_DC) /*
        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_traits_property_binding(zend_class_entry *ce TSRMLS_DC) /* {
 }
 /* }}} */
 
+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 @@ ZEND_API void zend_do_bind_traits(zend_class_entry *ce TSRMLS_DC) /* {{{ */
 
        /* 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);