From cc5f42ff66c585efb8c800817295de733afbfd62 Mon Sep 17 00:00:00 2001 From: fatkodima Date: Sun, 5 Jul 2020 22:08:54 +0300 Subject: [PATCH] Add new `Style/BisectedAttrAccessor` cop --- CHANGELOG.md | 1 + config/default.yml | 7 + docs/modules/ROOT/pages/cops.adoc | 1 + docs/modules/ROOT/pages/cops_style.adoc | 31 +++++ lib/rubocop.rb | 1 + .../cop/style/bisected_attr_accessor.rb | 121 ++++++++++++++++++ .../cop/style/bisected_attr_accessor_spec.rb | 83 ++++++++++++ 7 files changed, 245 insertions(+) create mode 100644 lib/rubocop/cop/style/bisected_attr_accessor.rb create mode 100644 spec/rubocop/cop/style/bisected_attr_accessor_spec.rb diff --git a/CHANGELOG.md b/CHANGELOG.md index d9cc75237f4..22f73d71152 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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][]) diff --git a/config/default.yml b/config/default.yml index d6eb9340762..3f08bae0abe 100644 --- a/config/default.yml +++ b/config/default.yml @@ -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' diff --git a/docs/modules/ROOT/pages/cops.adoc b/docs/modules/ROOT/pages/cops.adoc index b9c968e9320..634d797cfcb 100644 --- a/docs/modules/ROOT/pages/cops.adoc +++ b/docs/modules/ROOT/pages/cops.adoc @@ -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] diff --git a/docs/modules/ROOT/pages/cops_style.adoc b/docs/modules/ROOT/pages/cops_style.adoc index bd16b140969..7456cbe08bc 100644 --- a/docs/modules/ROOT/pages/cops_style.adoc +++ b/docs/modules/ROOT/pages/cops_style.adoc @@ -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 |=== diff --git a/lib/rubocop.rb b/lib/rubocop.rb index deae1e0a60f..9045ab19861 100644 --- a/lib/rubocop.rb +++ b/lib/rubocop.rb @@ -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' diff --git a/lib/rubocop/cop/style/bisected_attr_accessor.rb b/lib/rubocop/cop/style/bisected_attr_accessor.rb new file mode 100644 index 00000000000..cf543973e6f --- /dev/null +++ b/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 :%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 diff --git a/spec/rubocop/cop/style/bisected_attr_accessor_spec.rb b/spec/rubocop/cop/style/bisected_attr_accessor_spec.rb new file mode 100644 index 00000000000..31095e00194 --- /dev/null +++ b/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