Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:82443
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain gmail.com designates 209.85.192.42 as permitted sender)
MIME-Version: 1.0
In-Reply-To: <54DAF884.7000508@lerdorf.com>
References: <8703B53E-2C4A-4AC6-95C4-D4F19C6D5221@ajf.me>
	<CABBJUpf=wKvGY9+xORRv6WVhCb5+LfRZXGYNasHFS=fz_mwaZQ@mail.gmail.com>
	<CABBJUpeceN1uDK870eV2=znC9f3eaxqYCmaEk0ErH2cbFxTNEw@mail.gmail.com>
	<CABBJUpdYnaR2nztPwFUdDAiAneRrAWzGo-36-DA9hqiBjEHXNA@mail.gmail.com>
	<54DAF884.7000508@lerdorf.com>
Date: Wed, 11 Feb 2015 08:41:51 +0100
Message-ID: <CAB6YZuy37OGL+1HHSWScPQGV1fJ_uvc=YewVsw9gPwHtRZnfLQ@mail.gmail.com>
To: Rasmus Lerdorf <rasmus@lerdorf.com>
Cc: Xinchen Hui <laruence@php.net>, Andrea Faulds <ajf@ajf.me>, 
	PHP Internals <internals@lists.php.net>
Content-Type: text/plain; charset=UTF-8
Subject: Re: [PHP-DEV] [VOTE] Scalar Type Hints
From: pajousek@gmail.com (=?UTF-8?Q?Pavel_Kou=C5=99il?=)

On Wed, Feb 11, 2015 at 7:36 AM, Rasmus Lerdorf <rasmus@lerdorf.com> wrote:
> On 02/10/2015 07:57 PM, Xinchen Hui wrote:
>>> am I wrong?!
>> seems I am wrong with this, it's a false alarm...  it can restore automatically.
>
> Yeah, declare() doesn't span files so that isn't a problem.
>
> My worry is still the lack of type coercion for internal calls. I tested
> it on some apps and it will take quite a bit of tedious and rather
> useless effort to fix these to run in strict mode. Some examples of
> common things that are fatal errors in strict mode:
>
>   ini_set('precision', 14);
>
>   ini_set('display_errors', 1);
>
>   ini_set('display_errors', true);
>
>   ok, not common, but tan(4/2) fatal, tan(5/2) no error
>
>   Wordpress has this function, spot the error:
>
>   function add_magic_quotes( $array ) {
>         foreach ( (array) $array as $k => $v ) {
>                 if ( is_array( $v ) ) {
>                         $array[$k] = add_magic_quotes( $v );
>                 } else {
>                         $array[$k] = addslashes( $v );
>                 }
>         }
>         return $array;
>   }
>
>   $v may not always be a string (it died with a float for me), so the
>   fix is to cast:
>
>                         $array[$k] = addslashes( (string)$v );
>
>   Also from Wordpress:
>
>   $formatted = number_format( $number, absint( $decimals ),
> $wp_locale->number_format['decimal_point'],
> $wp_locale->number_format['thousands_sep'] );
>
>   Here number_format() is expecting a float but $number is a string. So
>   again, the only real fix is to cast.
>
>   And in Drupal8 *without turning on strict*:
>
>   use Drupal\Component\Utility\String;
>
>   it dies with: "Fatal error: "string" cannot be used as a class name in
> /var/www/drupal/core/includes/bootstrap.inc on line 11"
>
>   That String class is everywhere in Drupal. They are going to have a
>   fun time with that. See
> https://gist.githubusercontent.com/anonymous/d9252deeeb2aae1a5af5/raw/053155130d22551b1404d0a9b94e27424544b6d1/gistfile1
>
>   From Geeklog:
>
>   if (strtolower($topic) != strtolower($archivetid)) {
>     $sql .= " AND ta.tid != '{$archivetid}' ";
>   }
>
>   $topic can be null there. Looking at the logic, not really a bug,
>   just a natural reliance on null being coerced to ""
>
>   Also from Geeklog:
>
>     if( empty( $date )) {
>         // Date is empty, get current date/time
>         $stamp = time();
>     } else if( is_numeric( $date )) {
>         // This is a timestamp
>         $stamp = $date;
>     } else {
>         // This is a string representation of a date/time
>         $stamp = strtotime( $date );
>     }
>     // Format the date
>     $date = strftime( $dateformat, $stamp );
>
>     strftime() expects an integer for the timestamp there, but as the
>     above logic shows, they are expecting a numeric string to be fine.
>     No bug, just another forced cast.
>
>     And another number_format() instance where arg1 is not necessarily
>     a float. Obviously not a bug, so we have to cast again:
>
>     return number_format( $number, $dc, $ds, $ts );
>
>     In Opencart:
>
>     $this->image = imagecreatetruecolor($width, $height);
>     imagecreatetruecolor() expects parameter 1 to be integer, string
> given in /var/www/opencart/system/library/image.php on line 89
>
>     You could argue this is a bug, I guess, but the width and height are
>     coming from a database and are integers in the db, but since the db
>     returns strings. Another cast...
>
> I was genuinely hoping to find some bugs with this exercise. I suppose
> it is because I did it on mature projects and at this stage those sorts
> of bugs have already been fixed. But I still fear that people are going
> to want to be enable strictness everywhere and then they will quickly
> learn that they better cast stuff to be safe which makes the whole thing
> rather pointless. And I feel pretty sorry for the Drupal folks. That
> list of 1000+ instances of their String class is going to suck to fix.
>
> -Rasmus
>


Helo,

the Drupal thing seems scary.

Also, I realized one thing why I think the strict version is a bad
idea for PHP (in the state PHP is now - in an ideal world I would love
to have nothing but strongly typed PHP, but that's offtopic) - PHP has
many functions that return multiple types, so you cannot do foo(bar())
if bar() has multiple return types. :(

Regards
Pavel Kouril