Skip to content

Fix Time.new keyword arguments on JRuby 10#443

Merged
joshuacronemeyer merged 3 commits into
travisjeffery:masterfrom
johnnyshields:fix-jruby10-time-new-kwargs
Apr 12, 2026
Merged

Fix Time.new keyword arguments on JRuby 10#443
joshuacronemeyer merged 3 commits into
travisjeffery:masterfrom
johnnyshields:fix-jruby10-time-new-kwargs

Conversation

@johnnyshields
Copy link
Copy Markdown
Contributor

Summary

  • Replace ruby2_keywords approach with explicit **kwargs in new_with_mock_time
  • On JRuby 10 (Ruby 3.4 compat), ruby2_keywords doesn't properly forward keyword arguments through new_without_mock_time, causing TypeError: no implicit conversion of Hash into Integer
  • This breaks any Rails app using timecop on JRuby 10, since ActiveModel's datetime deserialization calls Time.new with keyword arguments internally

Reproduction

Any Rails 7.2+ app on JRuby 10 that has timecop loaded and reads a datetime column:

TypeError: no implicit conversion of Hash into Integer
# timecop/time_extensions.rb:22:in 'new_with_mock_time'
# activemodel/type/helpers/time_value.rb:93:in 'fast_string_to_time'

Fix

Use explicit **kwargs instead of ruby2_keywords. This works correctly across all Ruby versions and implementations:

def new_with_mock_time(*args, **kwargs)
  if args.empty? && kwargs.empty?
    now
  elsif kwargs.any?
    new_without_mock_time(*args, **kwargs)
  else
    new_without_mock_time(*args)
  end
end

Test plan

  • Added test for Time.new with in: keyword argument (Ruby 3.1+)
  • Added test for Time.new with positional arguments (regression check)
  • All 78 existing tests pass

🤖 Generated with Claude Code

johnnyshields and others added 2 commits April 12, 2026 03:49
Replace `ruby2_keywords` approach with explicit `**kwargs` in
`new_with_mock_time`. On JRuby 10 (Ruby 3.4 compat), the
`ruby2_keywords` flag doesn't properly forward keyword arguments
through `new_without_mock_time`, causing `TypeError: no implicit
conversion of Hash into Integer` when ActiveModel deserializes
datetime columns.

The fix uses explicit `**kwargs` which works correctly across all
Ruby versions and implementations.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
* master:
  Fix time-only DateTime.parse under freeze (travisjeffery#440)
  Improve and fix CI (travisjeffery#439)
@joshuacronemeyer
Copy link
Copy Markdown
Collaborator

joshuacronemeyer commented Apr 11, 2026

Hi @johnnyshields! Thank you for this thoughtful contribution. I'd love to get this merged. Just a couple tweaks to the tests would help.

Regarding test_time_new_with_keyword_arguments rescue ArgumentError doesn't catch TypeError, which is what older Ruby raises when a hash gets passed as the UTC offset positional argument. So maybe that test needs a conditional so it doesn't run for rubies that don't do keyword arguments

Also, the elsif kwargs.any? branch covers Time.new(in: "+05:00") (kwargs only, no positional args) but there's no test for that specific case. Not a blocker, but worth adding for completeness.

Holding our next release: #444 for this PR

… a TypeError (or any other error) on older Ruby won't silently mask a failure. Also new test covering kwargs only, no positional args), which exercises the args.empty? && kwargs.any? branch
@joshuacronemeyer joshuacronemeyer merged commit 4e84492 into travisjeffery:master Apr 12, 2026
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants