Only assign nil values in initialize if ivar not previously defined

[timriley]

This improves compatibility with objects initialized in unconventional ways.

In particular, it supports this case:

module SomeFramework
  class Action
    def self.new(configuration:, **args)
      # Do some trickery to make it so `#initialize` on subclasses doesn't need
      # to sorry about accepting a configuration arg and passing it to super
      allocate.tap do |obj|
        obj.instance_variable_set :@configuration, configuration
        obj.send :initialize, **args
      end
    end
  end
end

module MyApp
  class BaseAction
    # Inject the configuration object, which is passed to
    # SomeFramework::Action.new but not all the way through to any subsequent
    # `#initialize` calls
    include Import[configuration: "web.action.configuration"]
  end

  class SomeAction < BaseAction 
    include Import["some_repo"]
  end
end

MyApp::SomeAction.new
# => #<MyApp::SomeAction:0x00007f9ad53e1530
#  @configuration=#<TheConfigurationObject>,
#  @some_repo=#<SomeRepo>>

This change does not break any existing behaviour, and improves compatibility with unconventional (but not illegitimate!) scenarios like this one.

@solnic @flash-gordon any objections to me adding this?

[timriley]

@solnic @flash-gordon any objections to me adding this?

[flash-gordon]

@timriley 🤔 mb go with ||=?

[timriley]

@flash-gordon Using ||= causes breakages across the test suite.

For example, if inside #define_initialize_with_keywords I switch:

#{dependency_map.names.map { |name| "@#{name} = #{name} unless #{name}.nil? && instance_variable_defined?(:'@#{name}')" }.join("\n")}

To:

#{dependency_map.names.map { |name| "@#{name} ||= #{name}" }}

I get 10 failures. Even simple things like overriding an auto-injected dependency with an explicitly provided one don't work as they should (i.e. spec/integration/inheritance_spec.rb:52 starts to fail).

The reason I went with the long-winded approach here is that I wanted to be exceedingly precise with the condition in which we should choose not to assign an dependency ivar, i.e. exactly the condition I provided as an example in this PR's original description. I feel if we tried to go with something more conscise like ||= we may end up inadvertently destabilising the expected behaviour.

@flash-gordon what do you think?

[flash-gordon]

Oh, ok, whatever :) Actually, I aked only because the line became too long. But I can live with this ;)

[timriley]

Thanks for being thorough as always, @flash-gordon 🙏