CGI powered standard filters to handle non string inputs

[tjoyal]

Problem

Some standard filters to not properly handle unexpected argument.

Liquid::Template.parse("{{ 1 | url_encode }}").render

NoMethodError: undefined method `encoding' for 1:Fixnum
    /System/Library/Frameworks/Ruby.framework/Versions/2.0/usr/lib/ruby/2.0.0/cgi/util.rb:7:in `escape'
    /Users/tjoyal/projects/liquid/lib/liquid/standardfilters.rb:45:in `url_encode'

Solution

I'm usually aim to raise exceptions when the received parameters is not of the proper type(eg.: Liquid::StandardError), but most of the standard filters do explicit to_s before processing the input (for the same reason we also do an explicit check for nil?).

I ended up adding explicit to_s.

Thoughts?
Is this going in the right direction or should all non string parameters raise an exception?
Why is nil treated differently here compared to other filters? Seems to be a behaviour split introduced by 704937b#diff-844406140a08cdeddd843cfbdf0222d6L45

[dylanahsmith]

liquid tries to automatically coerce data types so designers don't need to think about them.

I don't really like the using .to_s to do that coercion, since it means even non-scalars would get coerced to a string. I would prefer we used some Liquid::Utils.to_string method that raised a Liquid::ArgumentError exception if a non-scalar was provided, but that would be a backwards incompatible change. Although you could do that for new coercions like this if you want to be more strict.

Since we do .to_s elsewhere, this is also fine for consistency reasons.

[dylanahsmith]

Why is nil treated differently here compared to other filters? Seems to be a behaviour split introduced by 704937b#diff-844406140a08cdeddd843cfbdf0222d6L45

That commit didn't cause a behaviour split, previously it would rescue input which would return nil if the input was nil.