Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add attribute readers for unknown attributes #89

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 44 additions & 0 deletions docsite/source/tolerance-to-unknown-arguments.html.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,3 +20,47 @@ user.respond_to? :role # => false
User.dry_initializer.attributes(user)
# => {}
```

Unknown arguments are stored in a pair of properties

```ruby
user.__dry_initializer_unknown_params__ # => ['Joe']
user.__dry_initializer_unknown_options__ # => {role: 'admin'}
```

The names of the rest params and the rest options are configurable.

```ruby
require 'dry-initializer'

class User
extend Dry::Initializer

rest_params :args
rest_options :kwargs
end

user = User.new 'Joe', role: 'admin'
user.args # => ['Joe']
user.kwargs # => {role: 'admin'}
user.respond_to? :__dry_initializer_unknown_params__ # => false
user.respond_to? :__dry_initializer_unknown_options__ # => false
```

Setting rest params or rest options to false results in strict argument checking.
Unknown arguments will result in ArgumentErrors.

```ruby
require 'dry-initializer'

class User
extend Dry::Initializer

rest_params false
rest_options false
end

user = User.new 'Joe' # => raises ArgumentError
user = User.new role: 'admin' # => raises ArgumentError

```
14 changes: 14 additions & 0 deletions lib/dry/initializer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,20 @@ def option(name, type = nil, **opts, &block)
self
end

# Sets rest params name or turns off rest params of [#dry_initializer]
# @param [Symbol, nil] name
def rest_params(name)
dry_initializer.rest_params = name
self
end

# Sets rest options name or turns off rest options of [#dry_initializer]
# @param [Symbol, nil] name
def rest_options(name)
dry_initializer.rest_options = name
self
end

private

def inherited(klass)
Expand Down
19 changes: 1 addition & 18 deletions lib/dry/initializer/builders/attribute.rb
Original file line number Diff line number Diff line change
Expand Up @@ -21,39 +21,22 @@ def initialize(definition)
@source = definition.source
@ivar = definition.ivar
@null = definition.null ? 'Dry::Initializer::UNDEFINED' : 'nil'
@opts = '__dry_initializer_options__'
@congif = '__dry_initializer_config__'
@item = '__dry_initializer_definition__'
@val = @option ? '__dry_initializer_value__' : @source
@val = @source
end
# rubocop: enable Metrics/MethodLength

def lines
[
'',
definition_line,
reader_line,
default_line,
coercion_line,
assignment_line
]
end

def reader_line
return unless @option

@optional ? optional_reader : required_reader
end

def optional_reader
"#{@val} = #{@opts}.fetch(:'#{@source}', #{@null})"
end

def required_reader
"#{@val} = #{@opts}.fetch(:'#{@source}')" \
" { raise KeyError, \"\#{self.class}: #{@definition} is required\" }"
end

def definition_line
return unless @type || @default

Expand Down
11 changes: 11 additions & 0 deletions lib/dry/initializer/builders/initializer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ def lines
define_line,
params_lines,
options_lines,
rest_lines,
end_line
]
end
Expand All @@ -50,6 +51,16 @@ def options_lines
.map { |line| ' ' << line }
end

def rest_lines
lines = []
lines << "@#{@config.rest_params} = #{@config.rest_params}" if @config.rest_params

if @config.rest_params && @definitions.any?(&:option)
lines << "@#{@config.rest_options} = #{@config.rest_options}"
end
lines.map { |line| " " << line }
end

def end_line
'end'
end
Expand Down
25 changes: 22 additions & 3 deletions lib/dry/initializer/builders/signature.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,14 @@ def self.[](config)
end

def call
[*required_params, *optional_params, '*', options].compact.join(', ')
[
*required_params,
*optional_params,
rest_params,
*required_options,
*optional_options,
rest_options
].compact.join(", ")
end

private
Expand All @@ -25,8 +32,20 @@ def optional_params
@config.params.select(&:optional).map { |rec| "#{rec.source} = #{@null}" }
end

def options
'**__dry_initializer_options__' if @options
def rest_params
"*#{@config.rest_params}" if @config.rest_params
end

def required_options
@config.options.reject(&:optional).map { |rec| "#{rec.source}:" }
end

def optional_options
@config.options.select(&:optional).map { |rec| "#{rec.source}: #{@null}" }
end

def rest_options
"**#{@config.rest_options}" if @options && @config.rest_options
end
end
end
35 changes: 33 additions & 2 deletions lib/dry/initializer/config.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ class Config
# @return [Hash<Symbol, Dry::Initializer::Definition>]
# hash of attribute definitions with their source names

attr_reader :null, :extended_class, :parent, :definitions
attr_reader :null, :extended_class, :parent, :definitions, :rest_params, :rest_options

# @!attribute [r] mixin
# @return [Module] reference to the module to be included into class
Expand Down Expand Up @@ -71,6 +71,24 @@ def option(name, type = nil, **opts, &block)
add_definition(true, name, type, block, **opts)
end

# Sets rest parameter name or turns rest params off
# @param [Symbol, nil] name
def rest_params=(name)
# remove the old attr_reader
mixin.class_eval(undef_method_code(@rest_params)) if @rest_params
@rest_params = name
finalize
end

# Sets rest options name or turns rest options off
# @param [Symbol, nil] name
def rest_options=(name)
# remove the old attr_reader
mixin.class_eval(undef_method_code(@rest_options)) if @rest_options
@rest_options = name
finalize
end

# The hash of public attributes for an instance of the [#extended_class]
# @param [Dry::Initializer::Instance] instance
# @return [Hash<Symbol, Object>]
Expand Down Expand Up @@ -107,6 +125,7 @@ def finalize
@definitions = final_definitions
check_order_of_params
mixin.class_eval(code)
mixin.class_eval(unknowns_code)
children.each(&:finalize)
self
end
Expand All @@ -115,7 +134,6 @@ def finalize
# @return [String]
def inch
line = Builders::Signature[self]
line = line.gsub('__dry_initializer_options__', 'options')
lines = ["@!method initialize(#{line})"]
lines += ["Initializes an instance of #{extended_class}"]
lines += definitions.values.map(&:inch)
Expand All @@ -131,6 +149,8 @@ def initialize(extended_class = nil, null: UNDEFINED)
@parent = sklass.dry_initializer if sklass.is_a? Dry::Initializer
@null = null || parent&.null
@definitions = {}
@rest_params = "__dry_initializer_unknown_params__"
@rest_options = "__dry_initializer_unknown_options__"
finalize
end

Expand Down Expand Up @@ -161,6 +181,17 @@ def final_definitions
end
end

def unknowns_code
lines = [undef_method_code(rest_params), undef_method_code(rest_options)]
lines << "attr_reader :#{rest_params}" if rest_params
lines << "attr_reader :#{rest_options}" if rest_options
lines.join("\n")
end

def undef_method_code(name)
"undef :#{name} if method_defined? :#{name}"
end

def check_type(previous, current)
return current unless previous
return current if previous.option == current.option
Expand Down
3 changes: 2 additions & 1 deletion lib/dry/initializer/dispatchers/prepare_target.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,8 @@ module Dry::Initializer::Dispatchers::PrepareTarget

# List of variable names reserved by the gem
RESERVED = %i[
__dry_initializer_options__
__dry_initializer_unkown_params__
__dry_initializer_unkown_options__
__dry_initializer_config__
__dry_initializer_value__
__dry_initializer_definition__
Expand Down
22 changes: 20 additions & 2 deletions spec/attributes_spec.rb
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
# frozen_string_literal: true

describe Dry::Initializer, "dry_initializer.attributes" do
subject { instance.class.dry_initializer.attributes(instance) }

Expand All @@ -14,7 +16,15 @@ class Test::Foo
let(:instance) { Test::Foo.new(:FOO) }

it "collects coerced params with default values" do
expect(subject).to eq({ foo: "FOO", bar: 1 })
expect(subject).to eq({foo: "FOO", bar: 1})
end

context "with unknown params" do
let(:instance) { Test::Foo.new(:FOO, :BAR, :BAZ, :FUTZ) }

it "ignores extra params" do
expect(subject).to eq({foo: "FOO", bar: :BAR, baz: :BAZ})
end
end
end

Expand All @@ -32,7 +42,15 @@ class Test::Foo
let(:instance) { Test::Foo.new(foo: :FOO, qux: :QUX) }

it "collects coerced and renamed options with default values" do
expect(subject).to eq({ foo: :FOO, bar: 1, quxx: "QUX" })
expect(subject).to eq({foo: :FOO, bar: 1, quxx: "QUX"})
end

context "with extra unknown options" do
let(:instance) { Test::Foo.new(foo: :FOO, qux: :QUX, futz: :FUTZ) }

it "ignores extra options" do
expect(subject).to eq({foo: :FOO, bar: 1, quxx: "QUX"})
end
end
end
end
31 changes: 31 additions & 0 deletions spec/required_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
# frozen_string_literal: true

describe "required param" do
before do
class Test::Foo
extend Dry::Initializer

param :foo
param :bar, optional: true
end
end

it "raise ArgumentError" do
expect { Test::Foo.new }.to raise_exception(ArgumentError)
end
end

describe "required option" do
before do
class Test::Foo
extend Dry::Initializer

option :foo
option :bar, optional: true
end
end

it "raise ArgumentError" do
expect { Test::Foo.new }.to raise_exception(ArgumentError)
end
end
10 changes: 6 additions & 4 deletions spec/several_assignments_spec.rb
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
# frozen_string_literal: true

describe "attribute with several assignments" do
before do
class Test::Foo
extend Dry::Initializer

option :bar, proc(&:to_s), optional: true
option :"some foo", as: :bar, optional: true
option :bar, proc(&:to_s), optional: true
option :some_foo, as: :bar, optional: true
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

String keyword argument names aren't allowed in 2.6. This is perhaps a very small breaking change but frankly its unlikely anyone on 2.6 was taking advantage of it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think here is a potential for the backward incompatibility. This test is about not string, but symbol keys. What is essential is that the keys don't need to be a valid ruby methods.

For example, we should can handle this assignment:

class Foo
  extend Dry::Initializer
  
  option :"!!! my weird key", as: :weird_key
end

foo = Foo.new("!!! my weird key": 1) # notice the key is a symbol, not a string
foo.weird_key # => 1

The goal of this feature to make it possible handling any keys in the income hash (maybe with a forced symbolization like below)

data = { 1 => 2 }.transform_keys { |k| k.to_s.to_sym }

end
end

Expand All @@ -13,7 +15,7 @@ class Test::Foo

it "is left undefined" do
expect(subject.bar).to be_nil
expect(subject.instance_variable_get :@bar)
expect(subject.instance_variable_get(:@bar))
.to eq Dry::Initializer::UNDEFINED
end
end
Expand All @@ -27,7 +29,7 @@ class Test::Foo
end

context "when renamed" do
subject { Test::Foo.new "some foo": :BAZ }
subject { Test::Foo.new some_foo: :BAZ }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And the same problem is here

Copy link
Author

@jonspalmer jonspalmer Jul 26, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure why I originally made this change. Perhaps some intermediate state. I've reverted its and specs still pass. Can you approve the actions here so we can get spec output?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nope, I meant a different change. The original version got a space inside "some foo" to empasize this is not a proper Ruby method name. In your version you just switched from symbol :some_foo to the string "some_foo", but both satisfy the metnod name requirements.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK I missed that. The challenge with supporting that is we can't have options like that explicitly defined in the signature. We'd have to bury that logic in unpacking the ** arguments. Honestly I find this feature pretty surprising - I'm unclear on the real world use cases for it. Aliasing just in the constructor call when all other interactions are through the legal ruby method name passed to as is a little peculiar. Can you explain more?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The particular challenge being Introspecting the dry initializer doesn't accurately return the constructor arguments my_foo.method(:__dry_initializer_initialize__).parameters


it "renames the attribute" do
expect(subject.bar).to eq :BAZ
Expand Down
Loading