liquid-rust icon indicating copy to clipboard operation
liquid-rust copied to clipboard

Make number coercion behave more like Ruby

Open nbudin opened this issue 2 years ago • 3 comments

  • [x] Tests created for any new feature or regression tests for bugfixes.

liquid-rust doesn't quite behave like the Ruby implementation of Liquid when it comes to coercing other types into integers and floats.

There are three major differences I've found between Rust's behavior and Ruby's here, and I've tried to account for all of them in this PR:

  • Ruby's behavior is to return 0 when converting an unparsable string to an int, and return 0.0 when converting an unparsable string to a float. Example:
    irb(main):004:0> Liquid::Template.parse("{{ 'something' | plus: 0 }}").render
    => "0"
    irb(main):005:0> 'something'.to_i
    => 0
    
  • Ruby truncates floating point numbers when converting to integers.
  • Similarly, if asked to convert a string representation of a floating point number to an integer, Ruby will truncate it.

Fixing these behaviors does constitute a breaking change for liquid-rust, since in order to fix them, I had to change several existing tests. I'm also not really sure that my approach to parsing floating-point number strings as integers is the best one possible - it feels a little hacky and perhaps there's a better way.

nbudin avatar Jul 03 '22 02:07 nbudin

Looks like this broke some other cases somehow.

epage avatar Jul 11 '22 21:07 epage

Looks like this broke some other cases somehow.

Ah yeah, I see that it did. I'll look into those and see if I can figure out what's up.

nbudin avatar Jul 11 '22 21:07 nbudin

@epage First off, sorry for breaking the tests! This is my first time working with a workspace project, and I didn't realize that simply running cargo test without --workspace would skip most of the tests.

I've worked through all the test failures. It appears that a bunch of logic internal to the stdlib as well as the derive macros was assuming that an unparsable integer would come back as None. In order to keep compatibility with this logic, I've introduced to_integer_strict and to_float_strict methods that preserve the prior behavior, and changed all the places that seemed to rely on it to use these instead. The tests are now passing.

nbudin avatar Jul 11 '22 23:07 nbudin