Re: Reviving scalar type hints

On Tue, Feb 17, 2015 at 9:22 AM, Rasmus Lerdorf <rasmus@lerdorf.com> wrote: > On 02/16/2015 09:48 PM, Sara Golemon wrote: > > Second, I should clarify that while the HHVM runtime performs > > coersion, the hack type checker is strict. So my original statement > > was inaccurate. As far as hack is concerned, it's simply strict. > > Period. > > With both the default (partial) type checking and strict enabled, my > number_format() example in Hack produces: > > int(1000) > 1,000 > > string(4) "1000" > 1,000 > > float(1000) > 1,000 > > string(5) "1000 " > Warning: number_format() expects parameter 1 to be double, string given > > string(5) " 1000" > 1,000 > > string(9) "1000 dogs" > Warning: number_format() expects parameter 1 to be double, string given > > string(3) "dog" > Warning: number_format() expects parameter 1 to be double, string given > > resource(4) of type (stream) > Warning: number_format() expects parameter 1 to be double, resource given > > > Basically it accepts, ints, floats and well-formed numeric strings and > the hh_client type checker is telling me I have "No errors". So the only > difference between Hack's strict mode and the current coercive behaviour > in PHP is strings with trailing chars. The "1000 dogs" case. "1000 " as > well in my example, but that is the same case. Where in PHP you get a > notice but it still does the conversion and in Hack you get a warning > and the conversion isn't done. So even though Hack has both a "partial" > type checking mode and a "strict" mode, the decision was to still do > type coercion for the others. I kind of expected it to only accept a > float in full-on strict mode to mimic the no-compromise strictness > proposed in the RFC. > > Also, looking through the code, I really don't see this "simply strict" > anywhere when it comes to calling internal functions. For example: > > $a = [1,2,3,4,5]; > print_r(array_reverse($a,"0")); > > It doesn't complain that "0" is a string and not a boolean. It doesn't > even complain about "dog" there. > > And the one everyone gets uppity about. bool->int conversion in > curl_setopt(). eg. > > $ch = curl_init("https://74.125.28.104"); > curl_setopt($ch, CURLOPT_SSL_VERIFYHOST, true); > curl_setopt($ch, CURLOPT_RETURNTRANSFER, true); > echo curl_exec($ch); > echo curl_error($ch); > > PHP obviously converts true to 1 there which has been a problem because > what people really meant was to set it to 2. We spew a notice for this, > of course: > > Notice: curl_setopt(): CURLOPT_SSL_VERIFYHOST with value 1 is deprecated > and will be removed as of libcurl 7.28.1. It is recommended to use value > 2 instead in ... > I think curl_setopt is a misleading example in the typehinting discussion, because this kind of API does not benefit from it. The third argument depends on the second argument and requires a "generic" type in code: curl_setopt(resource $ch, int $option, mixed $data); It won't be possible to change this (or any similar API) with strict type hints. The code to convert a boolean $data to integer in the VERIFYPEER case is manually implemented and therefore subject to the implementors design decisions. > > In Hack it appears that true is also converted to 1 in <?hh // strict > mode and no notice appears and the hh_client type checker doesn't > complain. If instead of true I pass in an array of strings, it still > converts it to 1 even though the type is blatantly wrong. It looks like > it was kept quite loose to match PHP and not cause too much legacy code > to break. In this particular case it is pretty dangerous to be > completely silent about it though since it actually means no host > verification is getting done. The output when properly set to 2 from > both PHP and Hack is: > > SSL: certificate subject name 'www.google.com' does not match target > host name '74.125.28.104' > > Please correct me here if I somehow ran these incorrectly. I did put > some deliberate type errors into my userspace code and hh_client caught > those nicely, so it seems like it was working, but it didn't catch > anything when it came to calling the internal API functions. > > eg. > > <?hh // strict > function test() : int { > $ch = curl_init("https://74.125.28.104"); > curl_setopt($ch, CURLOPT_SSL_VERIFYHOST, ["I have no idea what I am > doing"]); > curl_setopt($ch, CURLOPT_RETURNTRANSFER, true); > echo curl_exec($ch); > echo curl_error($ch); > return "beer"; > } > > hh_client reports: > /home/rasmus/test/a.php:8:12,17: Invalid return type (Typing[4110]) > /home/rasmus/test/a.php:2:19,21: This is an int > /home/rasmus/test/a.php:8:12,17: It is incompatible with a string > > When I change return "beer" to return 1 hh_client is happy. > > So, you keep asking what I would support. I would like to see an RFC > along the following lines: > > 1. Tighten up the type coercion for the "1000 dogs" case although we > have to look at whether there is a problem with some database APIs > returning space-padded fields so "1000 " would now break. > Hopefully that is fringe enough to not break the world. > > 2a. In strict mode, tone down the strictness and allow non-lossy > coercion including int->float. And yes, I know in really edge cases > that isn't technically non-lossy, but for all practical purposes it > is. > > or > > 2b. A much more flexible system for specifying multiple types. I should > be able to say that my function takes something that looks like a > number if I choose and still take advantage of stricter typing for > other parameters. > > 3. Don't turn on crazy-strict mode for internal functions that weren't > designed for that. Instead provide the same ability as userspace gets > for developers to gradually design their APIs to be stricter if they > so desire allowing both Hack and PHP to implement a stricter > curl_setopt(), for example. > > -Rasmus > >