Added strict handling of properties in traits.

# This is the first attempt to implement the properties as discussed on the mailinglist.
# RFC is not updated to reflect this changes, yet.

diff --git a/Zend/tests/traits/property001.phpt b/Zend/tests/traits/property001.phpt
new file mode 100644 (file)
index 0000000..7efd02e
--- /dev/null
+++ b/Zend/tests/traits/property001.phpt
@@ -0,0 +1,40 @@
+--TEST--
+Potentially conflicting properties should result in a strict notice. Property use is discorage for traits that are supposed to enable maintainable code reuse. Accessor methods are the language supported idiom for this.
+--FILE--
+<?php
+error_reporting(E_ALL);
+
+trait THello1 {
+  private $foo;
+}
+
+trait THello2 {
+  private $foo;
+}
+
+echo "PRE-CLASS-GUARD-TraitsTest\n";
+
+class TraitsTest {
+       use THello1;
+       use THello2;
+}
+
+error_reporting(E_ALL | E_STRICT);
+
+echo "PRE-CLASS-GUARD-TraitsTest2\n";
+
+class TraitsTest2 {
+       use THello1;
+       use THello2;
+}
+
+var_dump(property_exists('TraitsTest', 'foo'));
+var_dump(property_exists('TraitsTest2', 'foo'));
+?>
+--EXPECTF--    
+PRE-CLASS-GUARD-TraitsTest
+PRE-CLASS-GUARD-TraitsTest2
+
+Strict Standards: THello1 and THello2 define the same property ($foo) in the composition of TraitsTest2. This might be incompatible, to improve maintainability consider using accessor methods in traits instead. Class was composed in %s on line %d
+bool(true)
+bool(true)
\ No newline at end of file

diff --git a/Zend/tests/traits/property002.phpt b/Zend/tests/traits/property002.phpt
new file mode 100644 (file)
index 0000000..27361e0
--- /dev/null
+++ b/Zend/tests/traits/property002.phpt
@@ -0,0 +1,32 @@
+--TEST--
+Non-conflicting properties should work just fine.
+--FILE--
+<?php
+error_reporting(E_ALL);
+
+trait THello1 {
+  public $hello = "hello";
+}
+
+trait THello2 {
+  private $world = "World!";
+}
+
+class TraitsTest {
+       use THello1;
+       use THello2;
+       function test() {
+           echo $this->hello . ' ' . $this->world;
+       }
+}
+
+var_dump(property_exists('TraitsTest', 'hello'));
+var_dump(property_exists('TraitsTest', 'world'));
+
+$t = new TraitsTest;
+$t->test();
+?>
+--EXPECTF--    
+bool(true)
+bool(true)
+hello World!
\ No newline at end of file

diff --git a/Zend/tests/traits/property003.phpt b/Zend/tests/traits/property003.phpt
new file mode 100644 (file)
index 0000000..b4f0105
--- /dev/null
+++ b/Zend/tests/traits/property003.phpt
@@ -0,0 +1,30 @@
+--TEST--
+Conflicting properties with different visibility modifiers should result in a fatal error, since this indicates that the code is incompatible.
+--FILE--
+<?php
+error_reporting(E_ALL);
+
+trait THello1 {
+  public $hello;
+}
+
+trait THello2 {
+  private $hello;
+}
+
+echo "PRE-CLASS-GUARD\n";
+
+class TraitsTest {
+       use THello1;
+       use THello2;
+}
+
+echo "POST-CLASS-GUARD\n";
+
+$t = new TraitsTest;
+$t->hello = "foo";
+?>
+--EXPECTF--    
+PRE-CLASS-GUARD
+
+Fatal error: THello1 and THello2 define the same property ($hello) in the composition of TraitsTest. However, the definition differs and is considered incompatible. Class was composed in %s on line %d
\ No newline at end of file

diff --git a/Zend/tests/traits/property004.phpt b/Zend/tests/traits/property004.phpt
new file mode 100644 (file)
index 0000000..393b492
--- /dev/null
+++ b/Zend/tests/traits/property004.phpt
@@ -0,0 +1,30 @@
+--TEST--
+Conflicting properties with different initial values are considered incompatible.
+--FILE--
+<?php
+error_reporting(E_ALL);
+
+trait THello1 {
+  public $hello = "foo";
+}
+
+trait THello2 {
+  private $hello = "bar";
+}
+
+echo "PRE-CLASS-GUARD\n";
+
+class TraitsTest {
+       use THello1;
+       use THello2;
+       public function getHello() {
+           return $this->hello;
+       }
+}
+
+$t = new TraitsTest;
+?>
+--EXPECTF--    
+PRE-CLASS-GUARD
+
+Fatal error: THello1 and THello2 define the same property ($hello) in the composition of TraitsTest. However, the definition differs and is considered incompatible. Class was composed in %s on line %d
\ No newline at end of file

diff --git a/Zend/tests/traits/property005.phpt b/Zend/tests/traits/property005.phpt
new file mode 100644 (file)
index 0000000..3c80a1f
--- /dev/null
+++ b/Zend/tests/traits/property005.phpt
@@ -0,0 +1,50 @@
+--TEST--
+The same rules are applied for properties that are defined in the class hierarchy. Thus, if the properties are compatible, a notice is issued, if not a fatal error occures.
+--FILE--
+<?php
+error_reporting(E_ALL | E_STRICT);
+
+class Base {
+  private $hello;    
+}
+
+trait THello1 {
+  private $hello;
+}
+
+echo "PRE-CLASS-GUARD\n";
+class NoticeForBase extends Base {
+    use THello1;
+}
+echo "POST-CLASS-GUARD\n";
+
+// now the same with a class that defines the property itself
+
+class Notice extends Base {
+    use THello1;
+    private $hello;
+}
+echo "POST-CLASS-GUARD2\n";
+
+// now we do the test for a fatal error
+
+class TraitsTest {
+       use THello1;
+    public $hello;
+}
+
+echo "POST-CLASS-GUARD\n";
+
+$t = new TraitsTest;
+$t->hello = "foo";
+?>
+--EXPECTF--    
+PRE-CLASS-GUARD
+
+Strict Standards: Base and THello1 define the same property ($hello) in the composition of NoticeForBase. This might be incompatible, to improve maintainability consider using accessor methods in traits instead. Class was composed in %s on line %d
+POST-CLASS-GUARD
+
+Strict Standards: Notice and THello1 define the same property ($hello) in the composition of Notice. This might be incompatible, to improve maintainability consider using accessor methods in traits instead. Class was composed in %s on line %d
+POST-CLASS-GUARD2
+
+Fatal error: TraitsTest and THello1 define the same property ($hello) in the composition of TraitsTest. However, the definition differs and is considered incompatible. Class was composed in %s on line %d

diff --git a/Zend/zend_compile.c b/Zend/zend_compile.c
index 9b928437b042710c8db6a3104e007084b7865f37..d189d608992c2f9d7619a7e2e5ad825e9dd09e48 100644 (file)
--- a/Zend/zend_compile.c
+++ b/Zend/zend_compile.c
@@ -3804,59 +3804,49 @@ static void zend_traits_compile_exclude_table(HashTable* exclude_table, zend_tra
        }
 }
 
-ZEND_API void zend_do_bind_traits(zend_class_entry *ce TSRMLS_DC) /* {{{ */
+static void zend_do_traits_method_binding(zend_class_entry *ce TSRMLS_DC) /* {{{ */
 {
        HashTable** function_tables;
        HashTable* resulting_table;
        HashTable exclude_table;
        size_t i;
 
-       if (ce->num_traits <= 0) {
-               return;
-       }
-
-/*     zend_error(E_NOTICE, "Do bind Traits on %v with %d traits.\n Class has already %d methods.\n",
-                          ce->name.s, ce->num_traits, ce->function_table.nNumOfElements); */
-
-       /* complete initialization of trait strutures in ce */
-       zend_traits_init_trait_structures(ce TSRMLS_CC);
-
        /* prepare copies of trait function tables for combination */
        function_tables = malloc(sizeof(HashTable*) * ce->num_traits);
        resulting_table = (HashTable *) malloc(sizeof(HashTable));
        zend_hash_init_ex(resulting_table, 10, /* TODO: revisit this start size, may be its not optimal */
-                                                 /* NULL, ZEND_FUNCTION_DTOR, 0, 0); */
-                                                 NULL, NULL, 0, 0);
-
+                                         /* NULL, ZEND_FUNCTION_DTOR, 0, 0); */
+                                         NULL, NULL, 0, 0);
+  
        for (i = 0; i < ce->num_traits; i++) {
                function_tables[i] = (HashTable *) malloc(sizeof(HashTable));
-       zend_hash_init_ex(function_tables[i], ce->traits[i]->function_table.nNumOfElements,
+               zend_hash_init_ex(function_tables[i], ce->traits[i]->function_table.nNumOfElements,
                                                  /* NULL, ZEND_FUNCTION_DTOR, 0, 0); */
                                                  NULL, NULL, 0, 0);
-
+    
                zend_hash_init_ex(&exclude_table, 2, /* TODO: revisit this start size, may be its not optimal */
-                                                       NULL, NULL, 0, 0);
+                                                 NULL, NULL, 0, 0);
                zend_traits_compile_exclude_table(&exclude_table, ce->trait_precedences, ce->traits[i]);
-
+    
                /* copies functions, applies defined aliasing, and excludes unused trait methods */
                zend_traits_copy_trait_function_table(function_tables[i], &ce->traits[i]->function_table, ce->trait_aliases, &exclude_table TSRMLS_CC);
                zend_hash_graceful_destroy(&exclude_table);
        }
-
+  
        /* now merge trait methods */
        for (i = 0; i < ce->num_traits; i++) {
                zend_hash_apply_with_arguments(function_tables[i] TSRMLS_CC, (apply_func_args_t)zend_traits_merge_functions, 5, /* 5 is number of args for apply_func */
-                                               i, ce->num_traits, resulting_table, function_tables, ce);
+                                                                          i, ce->num_traits, resulting_table, function_tables, ce);
        }
-
-       /* now the resulting_table contains all trait methods we would have to
-          add to the class
-          in the following step the methods are inserted into the method table
-          if there is already a method with the same name it is replaced iff ce != fn.scope
-          --> all inherited methods are overridden, methods defined in the class are leaved
-          untouched */
+  
+       /*  now the resulting_table contains all trait methods we would have to
+               add to the class
+               in the following step the methods are inserted into the method table
+               if there is already a method with the same name it is replaced iff ce != fn.scope
+               --> all inherited methods are overridden, methods defined in the class are left
+               untouched */
        zend_hash_apply_with_arguments(resulting_table TSRMLS_CC, (apply_func_args_t)zend_traits_merge_functions_to_class, 1, ce);
-
+  
        /* free temporary function tables */
        for (i = 0; i < ce->num_traits; i++) {
                /* zend_hash_destroy(function_tables[i]); */
@@ -3869,8 +3859,145 @@ ZEND_API void zend_do_bind_traits(zend_class_entry *ce TSRMLS_DC) /* {{{ */
        /* zend_hash_destroy(resulting_table); */
        zend_hash_graceful_destroy(resulting_table);
        free(resulting_table);
+}
+
+static zend_class_entry* find_first_definition(zend_class_entry *ce, size_t current_trait, char* prop_name, int prop_name_length, zend_class_entry *coliding_ce) /* {{{ */
+{
+       size_t i;
+       zend_property_info *coliding_prop;
+       for (i = 0; (i < current_trait) && (i < ce->num_traits); i++) {
+               if (zend_hash_find(&ce->traits[i]->properties_info, prop_name, prop_name_length+1, (void **) &coliding_prop) == SUCCESS) {
+                       return ce->traits[i];
+               }
+       }
+
+       return coliding_ce;
+}
+
+static void zend_do_traits_property_binding(zend_class_entry *ce TSRMLS_DC) /* {{{ */
+{
+       //HashTable* resulting_table;
+       size_t i;
+       zend_property_info *property_info;
+       zend_property_info *coliding_prop;
+       zval compare_result;
+       char* prop_name;
+       int   prop_name_length;
+
+       char* class_name_unused;
+       bool prop_found;
+       bool not_compatible;
+       zval* prop_value;
+  
+  
+       /*  in the following steps the properties are inserted into the property table
+               for that, a very strict approach is applied:
+                - check for compatibility, if not compatible with any property in class -> fatal
+                - if compatible, then strict notice
+       */
+  
+  
+       for (i = 0; i < ce->num_traits; i++) {
+               for (zend_hash_internal_pointer_reset(&ce->traits[i]->properties_info);
+                        zend_hash_get_current_data(&ce->traits[i]->properties_info, (void *) &property_info) == SUCCESS;
+                        zend_hash_move_forward(&ce->traits[i]->properties_info)) {
+                       /* property_info now contains the property */
+      
+                       /* first get the unmangeld name if necessary,
+                          then check whether the property is already there */
+                       if ((property_info->flags & ZEND_ACC_PPP_MASK) == ZEND_ACC_PUBLIC) {
+                               prop_name = property_info->name;
+                               prop_name_length = property_info->name_length;
+                               prop_found = zend_hash_quick_find(&ce->properties_info,
+                                                                                                 property_info->name, property_info->name_length+1, 
+                                                                                                 property_info->h, (void **) &coliding_prop) == SUCCESS;
+                       }
+                       else {
+                               /* for private and protected we need to unmangle the names */
+                               zend_unmangle_property_name(property_info->name, property_info->name_length,
+                                                                                       &class_name_unused, &prop_name);
+                               prop_name_length = strlen(prop_name);
+                               prop_found = zend_hash_find(&ce->properties_info, prop_name, prop_name_length+1, (void **) &coliding_prop) == SUCCESS;
+                       }
+      
+                       /* next: check for conflicts with current class */
+                       if (prop_found) {
+                               if (coliding_prop->flags & ZEND_ACC_SHADOW) {
+                                       /* this one is inherited, lets look it up in its own class */
+                                       zend_hash_find(&coliding_prop->ce->properties_info, prop_name, prop_name_length+1, (void **) &coliding_prop);
+                               }
+                               if ((coliding_prop->flags & ZEND_ACC_PPP_MASK) == (property_info->flags & ZEND_ACC_PPP_MASK)) {
+                                       /* flags are identical, now the value needs to be checked */
+                                       if (property_info->flags & ZEND_ACC_STATIC) {
+                                               not_compatible = compare_function(&compare_result,
+                                                                                                                 ce->default_static_members_table[coliding_prop->offset],
+                                                                                                                 ce->traits[i]->default_static_members_table[property_info->offset] TSRMLS_CC) == FAILURE;
+                                       }
+                                       else {
+                                               not_compatible = compare_function(&compare_result,
+                                                                 ce->default_properties_table[coliding_prop->offset],
+                                                                     ce->traits[i]->default_properties_table[property_info->offset] TSRMLS_CC) == FAILURE;
+                                       }
+                               }
+                               else {
+                                       /* the flags are not identical, thus, we assume properties are not compatible */
+                                       not_compatible = true;
+                               }
+        
+                               if (not_compatible) {
+                                       zend_error(E_COMPILE_ERROR, 
+                                                          "%s and %s define the same property ($%s) in the composition of %s. However, the definition differs and is considered incompatible. Class was composed",
+                                                               find_first_definition(ce, i, prop_name, prop_name_length, coliding_prop->ce)->name,
+                                                               property_info->ce->name,
+                                                               prop_name,
+                                                               ce->name);
+                               }
+                               else {
+                                       zend_error(E_STRICT, 
+                                                          "%s and %s define the same property ($%s) in the composition of %s. This might be incompatible, to improve maintainability consider using accessor methods in traits instead. Class was composed",
+                                                               find_first_definition(ce, i, prop_name, prop_name_length, coliding_prop->ce)->name,
+                                                               property_info->ce->name,
+                                                               prop_name,
+                                                               ce->name);
+                               }
+                       }
+      
+                       /* property not found, so lets add it */
+                       if (property_info->flags & ZEND_ACC_STATIC) {
+                               prop_value = ce->traits[i]->default_static_members_table[property_info->offset];
+                       }
+                       else {
+                               prop_value = ce->traits[i]->default_properties_table[property_info->offset];
+                       }
+
 
+                       zend_declare_property_ex(ce, prop_name, prop_name_length, 
+                                                                        prop_value, property_info->flags, 
+                                                                    property_info->doc_comment, property_info->doc_comment_len TSRMLS_CC);
+               }
+       }
+}
+
+
+ZEND_API void zend_do_bind_traits(zend_class_entry *ce TSRMLS_DC) /* {{{ */
+{
+
+       if (ce->num_traits <= 0) {
+               return;
+       }
+
+       /* complete initialization of trait strutures in ce */
+       zend_traits_init_trait_structures(ce TSRMLS_CC);
+
+       /* first care about all methods to be flattened into the class */
+       zend_do_traits_method_binding(ce TSRMLS_CC);
+
+       /* then flatten the properties into it, to, mostly to notfiy developer about problems */
+       zend_do_traits_property_binding(ce TSRMLS_CC);
+
+       /* verify that all abstract methods from traits have been implemented */
        zend_verify_abstract_class(ce TSRMLS_CC);
+  
        /* now everything should be fine and an added ZEND_ACC_IMPLICIT_ABSTRACT_CLASS should be removed */
        if (ce->ce_flags & ZEND_ACC_IMPLICIT_ABSTRACT_CLASS) {
                ce->ce_flags -= ZEND_ACC_IMPLICIT_ABSTRACT_CLASS;