Integer filter accepts non-numeric strings.

[bobjflong]

Eg. '123foo' gets converted to 123, due to the use of to_i. I think that it would be preferable for the integer filter to do something like:

begin
  data = Integer(data)
rescue ArgumentError
  return [data, :integer]
end

Let me know what you think, I can cut a PR if you want.

[jkogara]

@bobjflong This would work for you I guess? #89

[bobjflong]

@jkogara looks like it!

[lorcan]

I've added a failing test case to demonstrate this issue.

The root cause of the problem is here, where we perform a to_i conversation after doing a simple regex validation (/^-?\d/):

if !data.is_a?(Fixnum)
  if data.is_a?(String) && data =~ /^-?\d/
    data = data.to_i
  else
    return [data, :integer]
  end
end

This can lead to some unexpected behaviour, as the failing test demonstrates. I think that while #89 allows us to bypass that step if we pass a strict parameter it might be better to be strict by default. What about a conversion like this:

if !data.is_a?(Fixnum)
  if data.is_a?(String)
    begin
      data = Integer(data)
    rescue ArgumentError
      return [data, :integer]
    end
  else
    return [data, :integer]
  end
end

rather than running the string through a regex and then a to_i?

[patrickod]

I like the idea of a strict by default integer filter, especially if it prevents strings like 10abc from being coerced in unexpected ways. I don't think it's that intuitive that you'd pass a string value with mixed content and somehow get a number out the other side as a value on which to operate. Given that this would be a backward-incompatible change, maybe we could use this as a means to go to 1.x.x ?

[lorcan]

#99 now has a failing test and includes the proposed solution above. This is a breaking change so we could hold it for 1.x.x.

[abhitrivedi]

Hello from 2019!

I understand this is an old thread but just curious whether there was any traction on this issue. IOW, did we add a strict option to integers?

[lorcan]

Hi @abhitrivedi, I've refreshed #99 to bring it up to date. I agree with @patrickod that rather than adding a strict option we should consider a breaking change and strictly enforce integers.

It's surprising that strings like 6iamnotanumber get converted to numbers like 6.

How would you feel about bumping mutations to 1.0 and enforcing strict number parsing? I'm happy to help :-D

(failing that we could introduce a strict option as suggested by #89, as part of a minor bump, but it feels like unnecessary complexity)

[abhitrivedi]

ping: @cypriss @eugeneius

[abhitrivedi]

How would you feel about bumping mutations to 1.0 and enforcing strict number parsing? I'm happy to help :-D

Since this is still a zero major version gem, the public API doesn't need to be backward compatible. I suggest we keep the zero major version until the authors decide that the api is stable enough.