From 733d01780089139927f873c89ecbc306fa398f03 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Janko=20Marohni=C4=87?= Date: Tue, 1 Oct 2019 21:12:51 +0200 Subject: [PATCH] Raise exception when injecting into modules To me it happened that I accidentally injected my dependencies into a namespace module instead of into a class, like this: module Operations module Articles include Import["repositories.articles"] class Create # ... end end end When I did that, I got a NoMethodError for NilClass somewhere down the line. This led me to believe that I've somehow incorrectly passed parameters to `Import[]`, and it took me a while to realize what was the error. The error occurred because the downstream code assumes the #initialize method will be defined, which is not the case for modules. To improve the developer experience, we detect that we're attempting to inject into a module and raise an explicit exception. --- lib/dry/auto_inject/dependency_map.rb | 5 ++-- lib/dry/auto_inject/errors.rb | 9 ++++++ lib/dry/auto_inject/strategies/constructor.rb | 3 ++ spec/dry/auto_inject_spec.rb | 28 +++++++++++++++++++ 4 files changed, 42 insertions(+), 3 deletions(-) create mode 100644 lib/dry/auto_inject/errors.rb diff --git a/lib/dry/auto_inject/dependency_map.rb b/lib/dry/auto_inject/dependency_map.rb index b5c9e49..c274d42 100644 --- a/lib/dry/auto_inject/dependency_map.rb +++ b/lib/dry/auto_inject/dependency_map.rb @@ -1,10 +1,9 @@ # frozen_string_literal: true +require 'dry/auto_inject/errors' + module Dry module AutoInject - DuplicateDependencyError = Class.new(StandardError) - DependencyNameInvalid = Class.new(StandardError) - VALID_NAME = /([a-z_][a-zA-Z_0-9]*)$/ class DependencyMap diff --git a/lib/dry/auto_inject/errors.rb b/lib/dry/auto_inject/errors.rb new file mode 100644 index 0000000..6145f88 --- /dev/null +++ b/lib/dry/auto_inject/errors.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +module Dry + module AutoInject + Error = Class.new(StandardError) + DuplicateDependencyError = Class.new(Error) + DependencyNameInvalid = Class.new(Error) + end +end diff --git a/lib/dry/auto_inject/strategies/constructor.rb b/lib/dry/auto_inject/strategies/constructor.rb index e2c1b05..b5f5cea 100644 --- a/lib/dry/auto_inject/strategies/constructor.rb +++ b/lib/dry/auto_inject/strategies/constructor.rb @@ -1,6 +1,7 @@ # frozen_string_literal: true require 'dry/auto_inject/dependency_map' +require 'dry/auto_inject/errors' module Dry module AutoInject @@ -23,6 +24,8 @@ def initialize(container, *dependency_names) # @api private def included(klass) + fail Error, "cannot inject dependencies into a module" unless klass.respond_to?(:new) + define_readers define_new diff --git a/spec/dry/auto_inject_spec.rb b/spec/dry/auto_inject_spec.rb index 650521e..abdc3fc 100644 --- a/spec/dry/auto_inject_spec.rb +++ b/spec/dry/auto_inject_spec.rb @@ -28,6 +28,10 @@ def initialize(*args) Class.new(child_class) end + let(:mod) do + Module.new + end + context 'with positioned args' do let(:parent_class) do Class.new do @@ -167,6 +171,14 @@ def initialize(first, *middle, last) end end end + + context 'autoinject in a module' do + it 'raises exception' do + expect { + mod.include Test::AutoInject.args[:one, :two] + }.to raise_error(Dry::AutoInject::Error, /cannot inject/) + end + end end context 'with hash arg' do @@ -312,6 +324,14 @@ def self.included(klass) end end end + + context 'autoinject in a module' do + it 'raises exception' do + expect { + mod.include Test::AutoInject.hash[:one, :two] + }.to raise_error(Dry::AutoInject::Error, /cannot inject/) + end + end end context 'with keyword args' do @@ -558,5 +578,13 @@ def initialize(other) end end end + + context 'autoinject in a module' do + it 'raises exception' do + expect { + mod.include Test::AutoInject.kwargs[:one, :two] + }.to raise_error(Dry::AutoInject::Error, /cannot inject/) + end + end end end