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

Fix Module with initialize method #1746

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
18 changes: 10 additions & 8 deletions lib/reek/smell_detectors/module_initialize.rb
Original file line number Diff line number Diff line change
Expand Up @@ -21,14 +21,16 @@ def self.contexts # :nodoc:
# @return [Array<SmellWarning>]
#
def sniff
context.defined_instance_methods.each do |node|
Copy link
Collaborator

Choose a reason for hiding this comment

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

Apparently, the cause of #1505 is that ModuleContext#defined_instance_methods includes methods from dynamically defined nested classes. Instead of dropping down to bare sexp manipulation, I think this method should be adjusted instead.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do you have any clue where can I start?

if node.name == :initialize
return smell_warning(
lines: [source_line],
message: 'has initialize method')
end
end
[]
return [] if does_not_have_initializer_method?
JuanVqz marked this conversation as resolved.
Show resolved Hide resolved

smell_warning(lines: [source_line], message: 'has initialize method')
end

private

# :reek:FeatureEnvy
def does_not_have_initializer_method?
context.exp.direct_children.none? { |child| child.type == :def && child.name == :initialize }
end
end
end
Expand Down
40 changes: 40 additions & 0 deletions spec/reek/smell_detectors/module_initialize_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -86,4 +86,44 @@ def initialize; end

expect(src).not_to reek_of(:ModuleInitialize)
end

it 'reports nothing for a method named initialize in a class assigned to a let variable' do
src = <<-RUBY
module Alfa
RSpec.describe Bravo do
let(:klass) do
Class.new do
def initialize; end
end
end
end
end
RUBY

expect(src).not_to reek_of(:ModuleInitialize)
end

it 'reports nothing for a DSL-based method instantiating a class' do
src = <<-RUBY
module Alfa
bravo Class.new do
def initialize; end
end
end
RUBY

expect(src).not_to reek_of(:ModuleInitialize)
end

it 'reports nothing for a method named initialize in a variable assignment' do
src = <<-RUBY
module Alfa
bravo = Class.new do
def initialize; end
end
end
RUBY

expect(src).not_to reek_of(:ModuleInitialize)
end
end