rails icon indicating copy to clipboard operation
rails copied to clipboard

Validate option keys passed to #change()

Open vlad-pisanov opened this issue 1 year ago • 3 comments

Summary

DateTime#change and Time#change receive nearly identical keyword arguments, with one subtle difference: DateTime#change also supports start:.

The change method silently ignores invalid/unsupported keyword arguments, which makes it error-prone to typos and makes transitioning legacy codebases from DateTime to Time difficult.

The same problem affects related classes Date and TimeWithZone.

This PR enforces that all arguments passed to change are actually supported, and updates a stale comment to include :nsec and :usec as supported arguments to DateTime#change.

vlad-pisanov avatar Aug 29 '22 16:08 vlad-pisanov

👋 Hey @vlad-pisanov, would you mind rebasing your branch again please to address conflicts? Thanks!

adrianna-chang-shopify avatar Sep 01 '22 13:09 adrianna-chang-shopify

I think we also need to require "active_support/core_ext/hash/keys.rb" in both of the calculations.rb files to make use of #assert_valid_keys

adrianna-chang-shopify avatar Sep 01 '22 15:09 adrianna-chang-shopify

@adrianna-chang-shopify done! And thanks for the require reminder 🙏

vlad-pisanov avatar Sep 04 '22 07:09 vlad-pisanov