Skip to content

Commit

Permalink
Add new Style/BisectedAttrAccessor cop
Browse files Browse the repository at this point in the history
  • Loading branch information
fatkodima authored and bbatsov committed Jul 6, 2020
1 parent e6b8ebf commit cc5f42f
Show file tree
Hide file tree
Showing 7 changed files with 245 additions and 0 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -5,6 +5,7 @@
### New features

* [#7868](https://github.com/rubocop-hq/rubocop/pull/7868): `Cop::Base` is the new recommended base class for cops. ([@marcandre][])
* [#8244](https://github.com/rubocop-hq/rubocop/pull/8244): Add new `Style/BisectedAttrAccessor` cop. ([@fatkodima][])
* [#7458](https://github.com/rubocop-hq/rubocop/issues/7458): Add new `AsciiConstants` option for `Naming/AsciiIdentifiers`. ([@fatkodima][])
* [#7373](https://github.com/rubocop-hq/rubocop/issues/7373): Add new `Style/RedundantAssignment` cop. ([@fatkodima][])
* [#8213](https://github.com/rubocop-hq/rubocop/pull/8213): Permit to specify TargetRubyVersion 2.8 (experimental). ([@koic][])
Expand Down
7 changes: 7 additions & 0 deletions config/default.yml
Expand Up @@ -2366,6 +2366,13 @@ Style/BeginBlock:
Enabled: true
VersionAdded: '0.9'

Style/BisectedAttrAccessor:
Description: >-
Checks for places where `attr_reader` and `attr_writer`
for the same method can be combined into single `attr_accessor`.
Enabled: 'pending'
VersionAdded: '0.87'

Style/BlockComments:
Description: 'Do not use block comments.'
StyleGuide: '#no-block-comments'
Expand Down
1 change: 1 addition & 0 deletions docs/modules/ROOT/pages/cops.adoc
Expand Up @@ -321,6 +321,7 @@ In the following section you find all available cops:
* xref:cops_style.adoc#styleautoresourcecleanup[Style/AutoResourceCleanup]
* xref:cops_style.adoc#stylebarepercentliterals[Style/BarePercentLiterals]
* xref:cops_style.adoc#stylebeginblock[Style/BeginBlock]
* xref:cops_style.adoc#stylebisectedattraccessor[Style/BisectedAttrAccessor]
* xref:cops_style.adoc#styleblockcomments[Style/BlockComments]
* xref:cops_style.adoc#styleblockdelimiters[Style/BlockDelimiters]
* xref:cops_style.adoc#stylecaseequality[Style/CaseEquality]
Expand Down
31 changes: 31 additions & 0 deletions docs/modules/ROOT/pages/cops_style.adoc
Expand Up @@ -446,6 +446,37 @@ BEGIN { test }

* https://rubystyle.guide#no-BEGIN-blocks

== Style/BisectedAttrAccessor

|===
| Enabled by default | Safe | Supports autocorrection | VersionAdded | VersionChanged

| Pending
| Yes
| Yes
| 0.87
| -
|===

This cop checks for places where `attr_reader` and `attr_writer`
for the same method can be combined into single `attr_accessor`.

=== Examples

[source,ruby]
----
# bad
class Foo
attr_reader :bar
attr_writer :bar
end
# good
class Foo
attr_accessor :bar
end
----

== Style/BlockComments

|===
Expand Down
1 change: 1 addition & 0 deletions lib/rubocop.rb
Expand Up @@ -364,6 +364,7 @@
require_relative 'rubocop/cop/style/auto_resource_cleanup'
require_relative 'rubocop/cop/style/bare_percent_literals'
require_relative 'rubocop/cop/style/begin_block'
require_relative 'rubocop/cop/style/bisected_attr_accessor'
require_relative 'rubocop/cop/style/block_comments'
require_relative 'rubocop/cop/style/block_delimiters'
require_relative 'rubocop/cop/style/case_equality'
Expand Down
121 changes: 121 additions & 0 deletions lib/rubocop/cop/style/bisected_attr_accessor.rb
@@ -0,0 +1,121 @@
# frozen_string_literal: true

require 'set'

module RuboCop
module Cop
module Style
# This cop checks for places where `attr_reader` and `attr_writer`
# for the same method can be combined into single `attr_accessor`.
#
# @example
# # bad
# class Foo
# attr_reader :bar
# attr_writer :bar
# end
#
# # good
# class Foo
# attr_accessor :bar
# end
#
class BisectedAttrAccessor < Cop
MSG = 'Combine both accessors into `attr_accessor :%<name>s`.'

def on_class(class_node)
reader_names, writer_names = accessor_names(class_node)

accessor_macroses(class_node).each do |macro|
check(macro, reader_names, writer_names)
end
end
alias on_module on_class

def autocorrect(node)
macro = node.parent

lambda do |corrector|
corrector.replace(macro, replacement(macro, node))
end
end

private

def accessor_names(class_node)
reader_names = Set.new
writer_names = Set.new

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

names.each do |name|
if attr_reader?(macro)
reader_names.add(name)
else
writer_names.add(name)
end
end
end

[reader_names, writer_names]
end

def accessor_macroses(class_node)
class_def = class_node.body
return [] if !class_def || class_def.def_type?

send_nodes =
if class_def.send_type?
[class_def]
else
class_def.each_child_node(:send)
end

send_nodes.select { |node| node.macro? && (attr_reader?(node) || attr_writer?(node)) }
end

def attr_reader?(send_node)
send_node.method?(:attr_reader) || send_node.method?(:attr)
end

def attr_writer?(send_node)
send_node.method?(:attr_writer)
end

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

if (attr_reader?(macro) && writer_names.include?(name)) ||
(attr_writer?(macro) && reader_names.include?(name))
add_offense(arg_node, message: format(MSG, name: name))
end
end
end

def replacement(macro, node)
rest_args = macro.arguments
rest_args.delete(node)
args = rest_args.map(&:source).join(', ')

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?
''
else
"#{indent(macro)}#{macro.method_name} #{args}"
end
end

def indent(node)
' ' * node.loc.column
end
end
end
end
end
83 changes: 83 additions & 0 deletions spec/rubocop/cop/style/bisected_attr_accessor_spec.rb
@@ -0,0 +1,83 @@
# frozen_string_literal: true

RSpec.describe RuboCop::Cop::Style::BisectedAttrAccessor do
subject(:cop) { described_class.new }

it 'registers an offense and corrects when both accessors of the name exists' do
expect_offense(<<~RUBY)
class Foo
attr_reader :bar
^^^^ Combine both accessors into `attr_accessor :bar`.
attr_writer :bar
^^^^ Combine both accessors into `attr_accessor :bar`.
other_macro :something
end
RUBY

expect_correction(<<~RUBY)
class Foo
attr_accessor :bar
other_macro :something
end
RUBY
end

it 'registers an offense and corrects when attr and attr_writer exists' do
expect_offense(<<~RUBY)
class Foo
attr :bar
^^^^ Combine both accessors into `attr_accessor :bar`.
attr_writer :bar
^^^^ Combine both accessors into `attr_accessor :bar`.
other_macro :something
end
RUBY

expect_correction(<<~RUBY)
class Foo
attr_accessor :bar
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
^^^^ Combine both accessors into `attr_accessor :bar`.
other_macro :something
end
RUBY

expect_correction(<<~RUBY)
class Foo
attr_accessor :bar
attr_reader :baz, :quux
other_macro :something
end
RUBY
end

it 'does not register an offense when only one accessor of the name exists' do
expect_no_offenses(<<~RUBY)
class Foo
attr_reader :bar
attr_writer :baz
end
RUBY
end

it 'does not register an offense when using `attr_accessor`' do
expect_no_offenses(<<~RUBY)
class Foo
attr_accessor :bar
end
RUBY
end
end

0 comments on commit cc5f42f

Please sign in to comment.