Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:81658
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain ajf.me designates 192.64.116.200 as permitted sender)
Content-Type: text/plain; charset=utf-8
Mime-Version: 1.0 (Mac OS X Mail 8.2 \(2070.6\))
In-Reply-To: <E70D71FD-04CD-40D2-9EFA-92246FFD5509@ajf.me>
Date: Mon, 2 Feb 2015 21:40:21 +0000
Cc: PHP Internals List <internals@lists.php.net>
Content-Transfer-Encoding: quoted-printable
Message-ID: <868C399B-F49B-482C-A7BE-9D2645506BA2@ajf.me>
References: <8DCD1B72-C81D-499E-B455-E4A042CD76E6@ajf.me> <B0C5801F-7D46-4CB0-9B2E-AF18C44DAF14@ajf.me> <CAN=kQJcaeH9n3vQH0BKyK+03ZuV2bO_cPio9yd-A4UT+mCVGkA@mail.gmail.com> <E70D71FD-04CD-40D2-9EFA-92246FFD5509@ajf.me>
To: Matthew Leverton <leverton@gmail.com>
Subject: Re: [PHP-DEV] [RFC] Scalar Type Hints v0.2
From: ajf@ajf.me (Andrea Faulds)

Hey everyone,

> On 2 Feb 2015, at 16:50, Andrea Faulds <ajf@ajf.me> wrote:
>=20
> The implementation does work for testing. I still need to write tests =
for return types but they seem to work. Parameter types are =
fully-working, though, and they have extensive tests, so I know =
they=E2=80=99re working fine and you could add them to an existing =
project.
>=20
> Strictifying an existing library is a good idea. I=E2=80=99ll try =
=E2=80=9Cstrictifying=E2=80=9D one of my bigger personal projects =
(PictoSwap, a 3DS browser application with a PHP backend) and see how it =
goes. I expect it=E2=80=99ll be fairly smooth, we=E2=80=99ll see.

I just went and did this on PictoSwap, it was fairly painless. It=E2=80=99=
s quite a small application, to be fair, but it is =E2=80=9Creal-world =
code=E2=80=9D.

First, I compiled my branch with the following configure command:

    YACC=3D/usr/local/opt/bison27/bin/bison ./configure --enable-debug =
--enable-phpdbg --disable-all --with-gd --enable-pdo --with-sqlite3 =
--with-pdo-sqlite --enable-session --enable-json

Ignore the YACC=3D, I only need to do that because OS X is weird. As you =
can see, PictoSwap requires Gd, PDO-SQLite, Session and JSON.

Then, I ran my site with the freshly-compiled version of PHP. Actually, =
that=E2=80=99s not quite true - I compiled several times, each time =
finding an error because of a missing extension. Eventually I got to =
this configure line which included all the ones my app needed.

Second, I added declare(strict_types=3D1); to each file along with type =
hints. I did this file by file, and tested with each file.

For most of the files, nothing was broken by the addition of strict type =
hints. However, some files did cause issues.

When I added hints to my graphics functions in include/graphics.php and =
turned on strict types, I got an error because of a type mismatch, which =
lead to me discovering a bug. It turns out I=E2=80=99d been assuming =
that gd takes float pixel positions, but it actually takes integer =
pixels! This means that those crucial .5s had been truncated off all =
this time, and I was none-the-wiser. I added explicit integer casts. Now =
it=E2=80=99s obvious that the results are being truncated.

Adding strict hints to include/user.php, which includes the =E2=80=9Cbusin=
ess logic=E2=80=9D as such turned up the most issues. It showed me a few =
different things.

One thing I learned was that return types are crippled by the lack of =
nullable returns. For most of my functions, I need to return TRUE =
(success) or a string (error message). I=E2=80=99d be fine with =
switching to NULL (success) or string (error) so it=E2=80=99s hintable, =
but I can=E2=80=99t actually do that, because we lack nullable returns. =
That means I=E2=80=99m omitting return types on most of my functions, =
unfortunately. I hope that the Nullable Types RFC can pass and fix this.

Another thing I learned was how I really needed to convert the values =
going into and coming out of my database (SQLite 3, in this case). Turns =
out most of the data in there was strings, as SQLite is =
dynamically-typed, and so my JSON output was ending up filled with =
strings, where it should have had numbers or booleans. Type mismatch =
errors allowed me to spot where this was happening. It=E2=80=99s only =
because JavaScript is similarly merciful to PHP that my web app worked =
at all!=20

I also learned that my session data didn=E2=80=99t have correct types: =
the user ID had ended up a string, not an integer. This was trivially =
fixed, but something I wouldn=E2=80=99t have noticed without strict =
typing.

Now, the addition of type hints to include/user.php broke my web-facing =
code (htdocs/api.php), because I wasn=E2=80=99t converting types =
properly. However, this only broke it when it used strict typing mode. =
If I turned off strict typing mode, as I expected, PHP happily converted =
the types and everything worked swimmingly. The fixes weren=E2=80=99t =
that difficult, but not having to make everything strict at once made =
adding type hints easier, because I could disable strict types in code =
that wasn=E2=80=99t yet ready and my app would keep running fine, then =
turn on strict types one it was updated.

The end result of this was better code, and I spotted some errors. The =
migration was eased, as I had hoped, by the ability to make some files =
strict and others weak.

It also shows that my web app works fine without modifications on PHP 7, =
which is great.

Admittedly, my web app is quite small. But I think this makes a good =
case for the usefulness of this RFC, and in particular of the declare() =
system, and strict hints vs. weak hints. :)

You can see the diff here: =
https://github.com/TazeTSchnitzel/PictoSwap/compare/php7-strict

Thanks!
--
Andrea Faulds
http://ajf.me/