Hey Internals,
In our work converting the Zend Framework library to namespaces, we came
across some inconsistencies in constructor namings.
Clearly, with namespace support making class names shorter, we come back
full circle where some class names collide with reserved words, thus we
are forced to become creative with class names:
What was once:
class Foo_Bar_Array {}
would become (1 to 1 mapping):
namespace Foo\Bar; class Array {}
but must become
namespace Foo\Bar; class ArrayBar {} (or ArrayObj, ArrayClass, etc)
We can work around these limitations.
With that in mind though, this is the bigger challenge we are faced with...
The following seems, to me, to be a BC break with regards to notices and
E_STRICT usage:
namespace Foo\Bar;
class Filter {
public function __construct() { /* construct stuff */ }
public function filter($value) { /* return filtered */ }
}
Produces:
PHP Strict Standards: Redefining already defined constructor for
class Zend\Filter\Filter in [snip file] on line [snip line]
This worked in 5.2.x.
This worked in 5.3.0.
This stopped working in 5.3.1
Moreover, I do think that this is NOT the correct behavior. While I
know somewhere in the history of PHP we had an engine that preferred
class names as constructor names, but thankfully- those days (one could
only hope) are gone.
IMO, the engine should be backwards compatible as best as possbile in
the 5.x series and should (with the respect of PHP4 tendencies), reward
programmers using proper PHP5 constructs, like __construct.
(While I hate to use the english language as a crutch for the argument..
PHP is after all written in english, ... it is considered a best
practice do have class names as nouns and methods as verbs. Since there
are a lot of cases where the same words are both the noun and verb.. the
verb being the method being acted on in the class, the noun - combined
with the shorter class names due to namespaces, I can see this becoming
a bigger problem as more people embrace namespaces.)
Furthermore, static functions of the same name should not be marked as
constructors by the engine in any situation (is there some BC we are
keeping here? b/c I cannot see the use case.)
Attached is a patch that fixes this behavior (against PHP_5_3). What it
does:
* If __construct is declared before a method of the same name as the
class, you WILL NOT get a notice
* If a method of the same name as the class appears before
__construct(), you WILL get a notice, as the current behavior.
(The idea here is that if you define __construct before anything
else, as most people do, it will take priority and allow you to use the
method name of the class name below that as normal)
* Static methods of the same name as the class will NOT be marked as
constructors (they currently are). This might invalidate the check done
inside zend_do_end_class_declaration() that checks to see if the
constructor is static. (Perhaps that first check becomes dead code?)
I am not sure where the current behavior came from. I looks like it got
merged into 5_2_x but then backed out and merged into 5_3_x somewhere
along the line after 5.3.0 was released.
In any case, considering we are not gonna get a context aware parser
that will allow us to use reserved words for symbols like classes and
functions, it would be nice to not have this seemingly artificial
limitation on having method names that do not match the class name.
Thanks,
Ralph Schindler
References:
http://bugs.php.net/bug.php?id=35634
http://bugs.php.net/bug.php?id=48215
diff --git a/Zend/zend_compile.c b/Zend/zend_compile.c
index 13b6c55..b7cd7b0 100644
--- a/Zend/zend_compile.c
+++ b/Zend/zend_compile.c
@@ -1285,13 +1285,11 @@ void zend_do_begin_function_declaration(znode
*function_token, znode *function_n
zend_str_tolower_copy(short_class_lcname,
short_class_name, short_class_name_length);
/* Improve after RC: cache the lowercase class name */
- if ((short_class_name_length == name_len) &&
(!memcmp(short_class_lcname, lcname, name_len))) {
- if (CG(active_class_entry)->constructor) {
- zend_error(E_STRICT, "Redefining
already defined constructor for class %s", CG(active_class_entry)->name);
- } else {
+ if ((short_class_name_length == name_len) &&
(!memcmp(short_class_lcname, lcname, name_len)) && ((fn_flags &
ZEND_ACC_STATIC) == 0)) {
+ if (!CG(active_class_entry)->constructor) {
CG(active_class_entry)->constructor =
(zend_function *) CG(active_op_array);
}
- } else if ((name_len ==
sizeof(ZEND_CONSTRUCTOR_FUNC_NAME)-1) && (!memcmp(lcname,
ZEND_CONSTRUCTOR_FUNC_NAME, sizeof(ZEND_CONSTRUCTOR_FUNC_NAME)))) {
+ } else if ((name_len ==
sizeof(ZEND_CONSTRUCTOR_FUNC_NAME)-1) && (!memcmp(lcname,
ZEND_CONSTRUCTOR_FUNC_NAME, sizeof(ZEND_CONSTRUCTOR_FUNC_NAME))) && ((fn_flags
& ZEND_ACC_STATIC) == 0)) {
if (CG(active_class_entry)->constructor) {
zend_error(E_STRICT, "Redefining
already defined constructor for class %s", CG(active_class_entry)->name);
}
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
