> So from this POV it is good, IMO. Namely, to know that it is the concrete
case delivering the concrete exception type instead of abstract Exception,
IntlException, PharException, etc. The chosen exception class name might
make you think that it's like enforcing strict mode, but that's not the
case. Hm, so question is - are exception class names really bound to
whether strict mode is used or not?
I don't disagree with this on principle, consistency is good, but now we
actually have LESS consistency because the constructor can throw one of
many exceptions. For example, PDO can throw a \TypeError or a \PDOException
on connection failure, or if the constructor fails in some other way. This
leads to code like this:
try {
new PDO([]);
} catch (\TypeError $e) {
} catch (\PDOException $e) {
}
With the potential for duplication in the error handling (in both catch
blocks), OR code like this which is overly generic and will potentially
catch other random stuff:
try {
new PDO([]);
} catch (\Throwable $e) {
// must be throwable as it's the only common thing between \TypeError
and \PDOException
}
This is why I say the previous behavior should be preserved: Warning for
type mis-match, with a (in this case) \PDOException if the constructor
fails (possibly as a result of the mis-match, or for any other reason)
UNLESS strict types are on, then a \TypeError should be thrown, and it
wouldn't attempt to execute the constructor and no \PDOException would be
thrown.
This then gives us the following flow:
try {
new PDO([]); // Warning, array given, string expected
} catch (\PDOException $e) {
// maybe connection failed, or constructor failed because we passed an
array!
}
OR we enable strict types and you basically need the examples shown
originally, but at least that's a conscious choice and expected behavior.
I believe that the goal should be: don't surprise your users, and this
change fails that. The new behavior is not what I think most people would
expect coming from 5.x, and that is a bad thing.
On Fri, Aug 14, 2015 at 4:42 AM, Anatol Belski <anatol....@belski.net>
wrote:
> Hi Davey,
>
> > -----Original Message-----
> > From: m...@daveyshafik.com [mailto:m...@daveyshafik.com] On Behalf Of Davey
> > Shafik
> > Sent: Thursday, August 13, 2015 7:01 PM
> > To: internals@lists.php.net
> > Subject: [PHP-DEV] Warning (5.x) -> TypeError (7) for internal
> Constructors
> >
> > Hi,
> >
> > I was trying to come up with an example for the
> > https://wiki.php.net/rfc/internal_constructor_behaviour RFC and noticed
> some
> > unexpected behavior.
> >
> > Based on one of the examples:
> >
> > <?php
> > var_dump(new ReflectionFunction([]));
> > ?>
> >
> > In PHP 5.6 it will emit a Warning, and returns a unusable instance of
> > ReflectionFunction, the latter of which should be resolved by the RFC
> above.
> >
> > Warning: ReflectionFunction::__construct() expects parameter 1 to be
> string,
> > array given
> >
> > In PHP 7b3 (and going back to b1 at least), it now throws a TypeError on
> the mis-
> > match. I would expect this behavior in strict type mode, but not by
> default. (see:
> > http://3v4l.org/jQQtX). The same if you do new PDO([]); but it would
> have
> > returned a null previously.
> >
> > Yes, the result is the same, in both cases, an exception should be
> thrown, but it
> > should be Constructor failed (the exact exception seems to change
> depending
> > on the class you're instantiating).
> >
> > In some cases, this behavior is as expected:
> >
> > new MessageFormatter('en_US', null);
> >
> > Will result in an IntlException with message "Constructor failed"
> >
> > (see: http://3v4l.org/uAjUr)
> >
> > I discussed this with Anthony and he pointed out this commit by nikic as
> the
> > change that introduced this behavior:
> > https://github.com/php/php-src/pull/1215
> >
> From what I see in the patch, it is not related to strict mode. It is more
> about unifying places that already throw exceptions with different names to
> look
>
> TypeError: NumberFormatter::__construct() expects parameter 1 to be
> string, array given
>
> So from this POV it is good, IMO. Namely, to know that it is the concrete
> case delivering the concrete exception type instead of abstract Exception,
> IntlException, PharException, etc. The chosen exception class name might
> make you think that it's like enforcing strict mode, but that's not the
> case. Hm, so question is - are exception class names really bound to
> whether strict mode is used or not?
>
> The one with ReflectionFunction you've discovered - yes, that's a BC
> deviation. However fits into the scheme and didn't happen at much places.
> What is more of backward incompatibility is that the old codes won't now
> catch those TypeError as most likely they catch Exception, not Error.
>
> From what I've spotted yet, some look like
>
> TypeError: NumberFormatter::__construct() expects at least 2 parameters, 0
> given
>
> that is probably not very logic as it has nothing to do with the argument
> types. So not sure about that. Where TypeError is relatively appropriate
> (but maybe a bit confusing because it is used in the strict mode as well)
> for "expect type X, type Y given", the case "expect X params, Y given"
> could have another error class name.
>
> > I believe that internal functions and methods should only emit a warning
> (as per
> > 5.x) for incorrect argument types, unless (1) it's a constructor
> failure, and (2)
> > you're not using strict mode.
> >
> About exceptions in the functions - there's another thread. Exceptions in
> objects is quite a ordirary thing anyway. The tendency is that since ZE
> throws them instead of fatals, the behavior to extend over the whole code
> base. Though it needs quite some serious deliberations which didn't any big
> progress yet. Maybe you could contribute there, too. This however has no
> relation to the strict mode as well.
>
> Regards
>
> Anatol
>
