ActiveSupport::TimeZone.strptime has an empty? for nil exception

[nbibler]

Steps to reproduce

Parse an ActiveSupport::TimeZone using strptime with a malformed string.

require "active_support/all"

tz = ActiveSupport::TimeZone.new("Eastern Time (US & Canada)")

puts %|Good: tz.strptime("2014-04-13", "%Y-%m-%d")|
p tz.strptime("2014-04-13", "%Y-%m-%d")

puts %|Good: tz.strptime("2014-04-13 21:20:50", "%Y-%m-%d")|
p tz.strptime("2014-04-13 21:20:50", "%Y-%m-%d")

# slashes and wrong part order.
puts %|Bad: tz.strptime("2014-04-13 21:20:50", "%m/%d/%Y")|
p tz.strptime("2014-04-13 21:20:50", "%m/%d/%Y")

Yields:

Good: tz.strptime("2014-04-13", "%Y-%m-%d")
Sun, 13 Apr 2014 00:00:00 EDT -04:00
Good: tz.strptime("2014-04-13 21:20:50", "%Y-%m-%d")
Sun, 13 Apr 2014 00:00:00 EDT -04:00
Bad: tz.strptime("2014-04-13 21:20:50", "%m/%d/%Y")
/ruby/2.3.1/gems/activesupport-5.0.0/lib/active_support/values/time_zone.rb:450:in `parts_to_time': undefined method `empty?' for nil:NilClass (NoMethodError)
        from /ruby/2.3.1/gems/activesupport-5.0.0/lib/active_support/values/time_zone.rb:383:in `strptime'
        from test.rb:12:in `<main>'

Expected behavior

It should either raise a reasonable exception or return nil.

Actual behavior

It raises a NoMethodError: undefined method empty? for nil:NilClass.

System configuration

Rails version: 4.2.6, 5.0.0

Ruby version: 2.1.8, 2.3.1

[prakashmurthy]

Standard ruby version of strptime returns a ArgumentError: invalid date error message in such cases.

> Date.strptime("2016-09-15", "%Y-%d-%m")
ArgumentError: invalid date
> DateTime.strptime("2014-04-13 21:20:50", "%m/%d/%Y")
ArgumentError: invalid date

[nbibler]

ArgumentError would be fine. The current error is due to a coding problem / bug (parts.empty?) in ActiveSupport:

module ActiveSupport
  class TimeZone
    private
      def parts_to_time(parts, now)
        return if parts.empty?
        # …
      end
  end
end

The exception coming out of ActiveSupport does not give an application developer any indication of the actual problem and loses the "malformed string / unmatching format" indication due to the nil handling issue.

[jmondo]

@nbibler are you working on this? if not, I'd be happy to submit a PR. I've been working my way through ActiveSupport as part of a learning exercise and fixing this bug would help my learning! LMK

[nbibler]

You're welcome to go for it. I imagine it'll be straightforward. Worst case, it's just a return if parts.nil? || parts.empty?…

[jmondo]

Sweet. Yep I think so too :)