Skip to content

Commit

Permalink
Fix an error for Style/BisectedAttrAccessor when using `attr_reader…
Browse files Browse the repository at this point in the history
…` and `attr_writer` with splat arguments
  • Loading branch information
fatkodima authored and bbatsov committed Jul 7, 2020
1 parent 0d09c1b commit 85ff4c3
Show file tree
Hide file tree
Showing 3 changed files with 51 additions and 15 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -5,6 +5,7 @@
### Bug fixes

* [#8252](https://github.com/rubocop-hq/rubocop/issues/8252): Fix a command line option name from `--safe-autocorrect` to `--safe-auto-correct`, which is compatible with RuboCop 0.86 and lower. ([@koic][])
* [#8257](https://github.com/rubocop-hq/rubocop/issues/8257): Fix an error for `Style/BisectedAttrAccessor` when using `attr_reader` and `attr_writer` with splat arguments. ([@fatkodima][])
* [#8239](https://github.com/rubocop-hq/rubocop/pull/8239): Don't load `.rubocop.yml` from personal folders to check for exclusions if given a custom configuration file. ([@deivid-rodriguez][])
* [#8256](https://github.com/rubocop-hq/rubocop/issues/8256): Fix an error for `--auto-gen-config` when running a cop who do not support auto-correction. ([@koic][])

Expand Down
39 changes: 26 additions & 13 deletions lib/rubocop/cop/style/bisected_attr_accessor.rb
Expand Up @@ -21,7 +21,7 @@ module Style
# end
#
class BisectedAttrAccessor < Cop
MSG = 'Combine both accessors into `attr_accessor :%<name>s`.'
MSG = 'Combine both accessors into `attr_accessor %<name>s`.'

def on_class(class_node)
reader_names, writer_names = accessor_names(class_node)
Expand All @@ -47,7 +47,7 @@ def accessor_names(class_node)
writer_names = Set.new

accessor_macroses(class_node).each do |macro|
names = macro.arguments.map(&:value)
names = macro.arguments.map(&:source)

names.each do |name|
if attr_reader?(macro)
Expand Down Expand Up @@ -85,7 +85,7 @@ def attr_writer?(send_node)

def check(macro, reader_names, writer_names)
macro.arguments.each do |arg_node|
name = arg_node.value
name = arg_node.source

if (attr_reader?(macro) && writer_names.include?(name)) ||
(attr_writer?(macro) && reader_names.include?(name))
Expand All @@ -95,20 +95,33 @@ def check(macro, reader_names, writer_names)
end

def replacement(macro, node)
rest_args = macro.arguments
rest_args.delete(node)
args = rest_args.map(&:source).join(', ')
class_node = macro.each_ancestor(:class, :module).first
reader_names, writer_names = accessor_names(class_node)

rest_args = rest_args(macro.arguments, reader_names, writer_names)

if attr_reader?(macro)
if args.empty?
"attr_accessor #{node.source}"
else
"attr_accessor #{node.source}\n#{indent(macro)}#{macro.method_name} #{args}"
end
elsif args.empty?
attr_reader_replacement(macro, node, rest_args)
elsif rest_args.empty?
''
else
"#{indent(macro)}#{macro.method_name} #{args}"
"#{macro.method_name} #{rest_args.map(&:source).join(', ')}"
end
end

def rest_args(args, reader_names, writer_names)
args.reject do |arg|
name = arg.source
reader_names.include?(name) && writer_names.include?(name)
end
end

def attr_reader_replacement(macro, node, rest_args)
if rest_args.empty?
"attr_accessor #{node.source}"
else
"attr_accessor #{node.source}\n"\
"#{indent(macro)}#{macro.method_name} #{rest_args.map(&:source).join(', ')}"
end
end

Expand Down
26 changes: 24 additions & 2 deletions spec/rubocop/cop/style/bisected_attr_accessor_spec.rb
Expand Up @@ -43,12 +43,34 @@ class Foo
RUBY
end

it 'registers an offense and corrects when both accessors of the splat exists' do
expect_offense(<<~RUBY)
class Foo
ATTRIBUTES = %i[foo bar]
attr_reader *ATTRIBUTES
^^^^^^^^^^^ Combine both accessors into `attr_accessor *ATTRIBUTES`.
attr_writer *ATTRIBUTES
^^^^^^^^^^^ Combine both accessors into `attr_accessor *ATTRIBUTES`.
other_macro :something
end
RUBY

expect_correction(<<~RUBY)
class Foo
ATTRIBUTES = %i[foo bar]
attr_accessor *ATTRIBUTES
other_macro :something
end
RUBY
end

it 'registers an offense and corrects when both accessors of the name exists and accessor contains multiple names' do
expect_offense(<<~RUBY)
class Foo
attr_reader :baz, :bar, :quux
^^^^ Combine both accessors into `attr_accessor :bar`.
attr_writer :bar
attr_writer :bar, :zoo
^^^^ Combine both accessors into `attr_accessor :bar`.
other_macro :something
end
Expand All @@ -58,7 +80,7 @@ class Foo
class Foo
attr_accessor :bar
attr_reader :baz, :quux
attr_writer :zoo
other_macro :something
end
RUBY
Expand Down

0 comments on commit 85ff4c3

Please sign in to comment.