diff --git a/lib/reek/ast/sexp_extensions/send.rb b/lib/reek/ast/sexp_extensions/send.rb index 503f9365b..92c6e5032 100644 --- a/lib/reek/ast/sexp_extensions/send.rb +++ b/lib/reek/ast/sexp_extensions/send.rb @@ -24,13 +24,10 @@ def participants end def module_creation_call? - object_creation_call? && module_creation_receiver? - end + return true if object_creation_call? && module_creation_receiver? + return true if data_definition_call? && data_definition_receiver? - def module_creation_receiver? - receiver && - receiver.type == :const && - [:Class, :Struct].include?(receiver.simple_name) + false end def object_creation_call? @@ -47,6 +44,24 @@ def attribute_writer? def attr_with_writable_flag? name == :attr && args.any? && args.last.type == :true end + + private + + def module_creation_receiver? + const_receiver? && [:Class, :Struct].include?(receiver.simple_name) + end + + def data_definition_call? + name == :define + end + + def data_definition_receiver? + const_receiver? && receiver.simple_name == :Data + end + + def const_receiver? + receiver && receiver.type == :const + end end Op_AsgnNode = SendNode diff --git a/spec/reek/ast/sexp_extensions_spec.rb b/spec/reek/ast/sexp_extensions_spec.rb index c0bfdf974..76a80d682 100644 --- a/spec/reek/ast/sexp_extensions_spec.rb +++ b/spec/reek/ast/sexp_extensions_spec.rb @@ -363,8 +363,28 @@ expect(bare_new).not_to be_module_creation_call end - it 'is not considered to have a module creation receiver' do - expect(bare_new).not_to be_module_creation_receiver + it 'is considered to be an object creation call' do + expect(bare_new).to be_object_creation_call + end + end + + context 'when it’s ‘define’ with no parameters and no receiver' do + let(:bare_new) { sexp(:send, nil, :define) } + + it 'is not considered to be a module creation call' do + expect(bare_new).not_to be_module_creation_call + end + + it 'is considered to be an object creation call' do + expect(bare_new).not_to be_object_creation_call + end + end + + context 'when it’s ‘new’ with receiver ‘Class‘' do + let(:bare_new) { sexp(:send, sexp(:const, nil, :Class), :new) } + + it 'is considered to be a module creation call' do + expect(bare_new).to be_module_creation_call end it 'is considered to be an object creation call' do @@ -372,6 +392,42 @@ end end + context 'when it’s ‘new’ with receiver ‘Struct‘' do + let(:bare_new) { sexp(:send, sexp(:const, nil, :Struct), :new) } + + it 'is considered to be a module creation call' do + expect(bare_new).to be_module_creation_call + end + + it 'is considered to be an object creation call' do + expect(bare_new).to be_object_creation_call + end + end + + context 'when it’s ‘new’ with receiver ‘Data‘' do + let(:bare_new) { sexp(:send, sexp(:const, nil, :Data), :new) } + + it 'is not considered to be a module creation call' do + expect(bare_new).not_to be_module_creation_call + end + + it 'is considered to be an object creation call' do + expect(bare_new).to be_object_creation_call + end + end + + context 'when it’s ‘define’ with receiver ‘Data‘' do + let(:bare_new) { sexp(:send, sexp(:const, nil, :Data), :define) } + + it 'is considered to be a module creation call' do + expect(bare_new).to be_module_creation_call + end + + it 'is not considered to be an object creation call' do + expect(bare_new).not_to be_object_creation_call + end + end + context 'when it’s ‘new’ with a complex receiver' do let(:node) { Reek::Source::SourceCode.from('(foo ? bar : baz).new').syntax_tree } @@ -379,10 +435,6 @@ expect(node).not_to be_module_creation_call end - it 'is not considered to have a module creation receiver' do - expect(node).not_to be_module_creation_receiver - end - it 'is considered to be an object creation call' do expect(node).to be_object_creation_call end diff --git a/spec/reek/smell_detectors/module_initialize_spec.rb b/spec/reek/smell_detectors/module_initialize_spec.rb index bd624cdab..c53bbc745 100644 --- a/spec/reek/smell_detectors/module_initialize_spec.rb +++ b/spec/reek/smell_detectors/module_initialize_spec.rb @@ -38,7 +38,7 @@ def initialize; end expect(src).not_to reek_of(:ModuleInitialize) end - it 'reports nothing for a method named initialize in a nested struct' do + it 'reports nothing for a method named initialize in a nested Struct class' do src = <<-RUBY module Alfa Bravo = Struct.new(:charlie) do @@ -64,6 +64,18 @@ def initialize; end expect(src).not_to reek_of(:ModuleInitialize) end + it 'reports nothing for a method named initialize in a nested Data class' do + src = <<-RUBY + module Alfa + Bravo = Data.define(:charlie) do + def initialize; end + end + end + RUBY + + expect(src).not_to reek_of(:ModuleInitialize) + end + it 'can be disabled via comment' do src = <<-RUBY # :reek:ModuleInitialize