instance variables are overwritten with nil value when DSL and block contexts are same

[taichi-ishitani]

Instance variables of DSL context object are overwritten with nil value when DSL conetext object and block context object are the same object.

Here is example code:

require 'docile'

class DSLContext
  def foo(v = nil)
    @foo = v if v
    @foo
  end

  def process_code(code)
    Docile.dsl_eval(self, &binding.eval("proc { #{code} }"))
  end
end

dsl_context = DSLContext.new
dsl_context.process_code 'foo 1; p foo'
p dsl_context.foo

I think the result of above code should be like below:

1
1

but the actual result is below:

1
nil

[ms-ati]

Hi @taichi-ishitani firstly thank you for your bug report and pull request!

Can you help me understand a little about what this issue means in the real world? The code you've described above, where you provide a custom DSLContext and then do a manual eval, seem very unlike and real world usage of Docile that I know of.

Basically, what real-world problem does this cause?

[taichi-ishitani]

I'm creating a configuration DSL class by using Docile gem. However my configuration DSL class does not work well because of this issue.
Here is simplified code of my configuration DSL class.

# config_dsl.rb
require 'docile'

class ConfigDSL
  def project_name(name = nil)
    @project_name = name if name
    @project_name
  end

  def data_width(width = nil)
    @data_width = width if width
    @data_width
  end

  def load_file(file)
    block = binding.eval("proc { #{File.binread(file)} }", file)
    Docile.dsl_eval(self, &block)
  end
end

if $0 == __FILE__
  dsl = ConfigDSL.new
  dsl.load_file(ARGV[0])
  p dsl.project_name, dsl.data_width
end

In ConfigDSL#load_file method, docile gem and eval method are used to apply config settings from an external config file.

Here is a example config file and usage:

# my_config.rb
project_name :my_project
data_width 64

$ ruby config_dsl.rb my_config.rb

After loading my_config.rb config file, I expect config settings (project_name & data_width) are set and their values are :my_project and 64 but both config settings are nil.

If you want more information, please let me know.

[ms-ati]

Thank you so much @taichi-ishitani for such a clear problem description. Please give me just a moment to investigate this usage, to make sure I fully understand what is happening, and why the PR fixes it :)

[ms-ati]

Ah ok. Is this understanding correct:

Use case: DSL method sets instance variable on DSL object, and also the DSL object is the block's context

My understanding is that this will only occur when the block's context is the same as the DSL object instance -- which is why we hadn't seen this before. Does that match your understanding? Or have I missed something?

[ms-ati]

@taichi-ishitani I think my understanding is correct, because please observe that the following variation works as expected:

# config_dsl.rb
require 'docile'

class ConfigDSL
  def project_name(name = nil)
    @project_name = name if name
    @project_name
  end

  def data_width(width = nil)
    @data_width = width if width
    @data_width
  end

  def self.load_file(file)
    block = binding.eval("proc { #{File.binread(file)} }", file)
    Docile.dsl_eval(new, &block)
  end
end

if $0 == __FILE__
  dsl = ConfigDSL.load_file(ARGV[0])
  p dsl.project_name, dsl.data_width
end

What you see above is more similar to how Docile is often used in practice, which is probably why this hasn't come up before. The difference is that the block's context is NOT the same as the DSL object instance here.

Does this make sense?

Regardless, I will still accept your PR -- but knowing the above now, can you think about how to provide a clear RSpec example which covers this use case?

[taichi-ishitani]

@ms-ati
Yes, your understanding is correct.

RSpec example

OK, I will create RSpec exmple for this issue tomorrow.
Should I add the example to docile/spec/docile_spec.rb ?

[ms-ati]

Yes, please! :) Thank you

[taichi-ishitani]

I have added a RSpec example for this issue: b589c46
How about this example?

[ms-ati]

Thanks again @taichi-ishitani! This is merged.

[taichi-ishitani]

@ms-ati Thank you for merging my PR!