Hey everyone,
> On 2 Feb 2015, at 16:50, Andrea Faulds <[email protected]> wrote:
>
> 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’re working fine and you could add them to an existing project.
>
> Strictifying an existing library is a good idea. I’ll try “strictifying” one of my bigger
> personal projects (PictoSwap, a 3DS browser application with a PHP backend) and see how it goes. I
> expect it’ll be fairly smooth, we’ll see.
I just went and did this on PictoSwap, it was fairly painless. It’s quite a small application, to
be fair, but it is “real-world code”.
First, I compiled my branch with the following configure command:
YACC=/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=, 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’s 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=1); 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’d 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’s obvious that the results are being truncated.
Adding strict hints to include/user.php, which includes the “business logic” 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’d be fine with
switching to NULL (success) or string (error) so it’s hintable, but I can’t actually do that,
because we lack nullable returns. That means I’m 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’s only
because JavaScript is similarly merciful to PHP that my web app worked at all!
I also learned that my session data didn’t have correct types: the user ID had ended up a string,
not an integer. This was trivially fixed, but something I wouldn’t 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’t 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’t 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’t 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/