Skip to content

Commit

Permalink
[Fix #9771] Fix a false positive for Style/TrivialAccessors
Browse files Browse the repository at this point in the history
Fixes #9771.

This PR changes `AllowDSLWriters` to true by default because
it's still a trivial accessor and most people are unlikely to be
creating DSLs.

By this will prevent a false positive for `Style/TrivialAccessors`
when defining a method that is not a DSL writer.
  • Loading branch information
koic authored and bbatsov committed May 10, 2021
1 parent 2f1ad5f commit 8ed41bb
Show file tree
Hide file tree
Showing 4 changed files with 92 additions and 16 deletions.
@@ -0,0 +1 @@
* [#9771](https://github.com/rubocop/rubocop/issues/9771): Change `AllowDSLWriters` to true by default for `Style/TrivialAccessors`. ([@koic][])
4 changes: 2 additions & 2 deletions config/default.yml
Expand Up @@ -4745,7 +4745,7 @@ Style/TrivialAccessors:
StyleGuide: '#attr_family'
Enabled: true
VersionAdded: '0.9'
VersionChanged: '0.77'
VersionChanged: '<<next>>'
# When set to `false` the cop will suggest the use of accessor methods
# in situations like:
#
Expand All @@ -4764,7 +4764,7 @@ Style/TrivialAccessors:
# on_exception :restart
#
# Commonly used in DSLs
AllowDSLWriters: false
AllowDSLWriters: true
IgnoreClassMethods: false
AllowedMethods:
- to_ary
Expand Down
65 changes: 65 additions & 0 deletions lib/rubocop/cop/style/trivial_accessors.rb
Expand Up @@ -27,6 +27,71 @@ module Style
# class << self
# attr_reader :baz
# end
#
# @example ExactNameMatch: true (default)
# # good
# def name
# @other_name
# end
#
# @example ExactNameMatch: false
# # bad
# def name
# @other_name
# end
#
# @example AllowPredicates: true (default)
# # good
# def foo?
# @foo
# end
#
# @example AllowPredicates: false
# # bad
# def foo?
# @foo
# end
#
# # good
# attr_reader :foo
#
# @example AllowDSLWriters: true (default)
# # good
# def on_exception(action)
# @on_exception=action
# end
#
# @example AllowDSLWriters: false
# # bad
# def on_exception(action)
# @on_exception=action
# end
#
# # good
# attr_writer :on_exception
#
# @example IgnoreClassMethods: false (default)
# # bad
# def self.foo
# @foo
# end
#
# # good
# class << self
# attr_reader :foo
# end
#
# @example IgnoreClassMethods: true
# # good
# def self.foo
# @foo
# end
#
# @exampole AllowedMethods: ['allowed_method']
# # good
# def allowed_method
# @foo
# end
class TrivialAccessors < Base
include AllowedMethods
extend AutoCorrector
Expand Down
38 changes: 24 additions & 14 deletions spec/rubocop/cop/style/trivial_accessors_spec.rb
Expand Up @@ -59,14 +59,20 @@ class << self
it 'registers an offense on class writer' do
expect_offense(<<~RUBY)
class Foo
def self.foo(val)
def self.foo=(val)
^^^ Use `attr_writer` to define trivial writer methods.
@foo = val
end
end
RUBY

expect_no_corrections
expect_correction(<<~RUBY)
class Foo
class << self
attr_writer :foo
end
end
RUBY
end

it 'registers an offense on reader with braces' do
Expand Down Expand Up @@ -121,25 +127,26 @@ class Foo
it 'registers an offense on one-liner writer' do
expect_offense(<<~RUBY)
class Foo
def foo(val); @foo=val; end
def foo=(val); @foo=val; end
^^^ Use `attr_writer` to define trivial writer methods.
end
RUBY

expect_no_corrections
expect_correction(<<~RUBY)
class Foo
attr_writer :foo
end
RUBY
end

it 'registers an offense on DSL-style trivial writer' do
expect_offense(<<~RUBY)
it 'does not register an offense on DSL-style writer' do
expect_no_offenses(<<~RUBY)
class Foo
def foo(val)
^^^ Use `attr_writer` to define trivial writer methods.
@foo = val
end
end
RUBY

expect_no_corrections
end

it 'registers an offense on reader with `private`' do
Expand Down Expand Up @@ -353,7 +360,7 @@ class << @blah
it 'registers an offense when names mismatch in writer' do
expect_offense(<<~RUBY)
class Foo
def foo(val)
def foo=(val)
^^^ Use `attr_writer` to define trivial writer methods.
@f = val
end
Expand Down Expand Up @@ -446,17 +453,20 @@ def foo?
end
end

context 'with DSL allowed' do
let(:cop_config) { { 'AllowDSLWriters' => true } }
context 'with DSL denied' do
let(:cop_config) { { 'AllowDSLWriters' => false } }

it 'accepts DSL-style writer' do
expect_no_offenses(<<~RUBY)
it 'registers an offense on DSL-style writer' do
expect_offense(<<~RUBY)
class Foo
def foo(val)
^^^ Use `attr_writer` to define trivial writer methods.
@foo = val
end
end
RUBY

expect_no_corrections
end
end

Expand Down

0 comments on commit 8ed41bb

Please sign in to comment.