[#5641] Disable Style/TrivialAccessors auto-correction for def wi…

…th `private` (#5691)

See #5641

This cop should not auto-correct `private def foo() @foo end`. Because `def foo() @foo end` returns `:foo`, but `attr_reader :foo` returns `nil`.

The `autocorrect` method checks only the parent is a `send` type. It does not check method name. Because the cop also should handle methods other than `private`, `public`, `protected`. For example, [finalist](https://github.com/joker1007/finalist) gem defines `final` method (`final def foo; end`).

Note
===

This pull-request does not close the related issue since the issue still has an problem if the auto-correction is fixed.
I think we should discuss the cop should have or not an option to ignore `private def foo() @foo end` on the issue.

CHANGELOG.md

@@ -19,6 +19,7 @@
 * [#5651](https://github.com/bbatsov/rubocop/pull/5651): Fix NoMethodError when specified config file that does not exist. ([@onk][])
 * [#5647](https://github.com/bbatsov/rubocop/pull/5647): Fix encoding method of RuboCop::MagicComment::SimpleComment. ([@htwroclau][])
 * [#5619](https://github.com/bbatsov/rubocop/issues/5619): Do not register an offense in `Style/InverseMethods` when comparing constants with `<`, `>`, `<=`, or `>=`. If the code is being used to determine class hierarchy, the correction might not be accurate. ([@rrosenblum][])
+* [#5641](https://github.com/bbatsov/rubocop/issues/5641): Disable `Style/TrivialAccessors` auto-correction for `def` with `private`. ([@pocke][])
 * Fix bug where `Style/SafeNavigation` does not auto-correct all chained methods resulting in a `Lint/SafeNavigationChain` offense. ([@rrosenblum][])
 * [#5436](https://github.com/bbatsov/rubocop/issues/5436): Allow empty kwrest args in UncommunicativeName cops. ([@pocke][])
 * [#5674](https://github.com/bbatsov/rubocop/issues/5674): Fix auto-correction of `Layout/EmptyComment` when the empty comment appears on the same line as code. ([@rrosenblum][])

lib/rubocop/cop/style/trivial_accessors.rb

@@ -39,6 +39,9 @@ def on_def(node)
         alias on_defs on_def
 
         def autocorrect(node)
+          parent = node.parent
+          return if parent && parent.send_type?
+
           if node.def_type?
             autocorrect_instance(node)
           elsif node.defs_type? && node.children.first.self_type?

spec/rubocop/cop/style/trivial_accessors_spec.rb

@@ -101,11 +101,20 @@ def foo(val); @foo=val; end
     RUBY
   end
 
-  it 'register an offense on DSL-style trivial writer' do
+  it 'registers an offense on DSL-style trivial writer' do
     expect_offense(<<-RUBY.strip_indent)
       def foo(val)
       ^^^ Use `attr_writer` to define trivial writer methods.
-       @foo = val
+        @foo = val
+      end
+    RUBY
+  end
+
+  it 'registers an offense on reader with `private`' do
+    expect_offense(<<-RUBY.strip_indent)
+      private def foo
+              ^^^ Use `attr_reader` to define trivial reader methods.
+        @foo
       end
     RUBY
   end
@@ -382,6 +391,19 @@ def self.foo(val)
       end
     end
 
+    context 'with `private`' do
+      let(:source) { <<-RUBY.strip_indent }
+        private def foo
+          @foo
+        end
+      RUBY
+
+      it 'does not autocorrect' do
+        expect(autocorrect_source(source)).to eq(source)
+        expect(cop.offenses.map(&:corrected?)).to eq [false]
+      end
+    end
+
     context 'non-matching reader' do
       let(:cop_config) { { 'ExactNameMatch' => false } }