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

Add new Style/BisectedAttrAccessor cop #8244

Merged
merged 1 commit into from Jul 6, 2020
Merged
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
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