Merge pull request #13401 from akshay-vishnoi/refactor

Prevent creation of instance methods when `instance_reader = false`, Grammar checks, Conditional statements combined

2 files changed, 13 insertions, 9 deletions

diff --git a/activesupport/lib/active_support/core_ext/class/delegating_attributes.rb b/activesupport/lib/active_support/core_ext/class/delegating_attributes.rb
index ff870f5fd1..c2219beb5a 100644
--- a/activesupport/lib/active_support/core_ext/class/delegating_attributes.rb
+++ b/activesupport/lib/active_support/core_ext/class/delegating_attributes.rb
@@ -4,20 +4,21 @@ require 'active_support/core_ext/module/remove_method'
 class Class
   def superclass_delegating_accessor(name, options = {})
     # Create private _name and _name= methods that can still be used if the public
-    # methods are overridden. This allows
-    _superclass_delegating_accessor("_#{name}")
+    # methods are overridden.
+    _superclass_delegating_accessor("_#{name}", options)
 
-    # Generate the public methods name, name=, and name?
+    # Generate the public methods name, name=, and name?.
     # These methods dispatch to the private _name, and _name= methods, making them
-    # overridable
+    # overridable.
     singleton_class.send(:define_method, name) { send("_#{name}") }
     singleton_class.send(:define_method, "#{name}?") { !!send("_#{name}") }
     singleton_class.send(:define_method, "#{name}=") { |value| send("_#{name}=", value) }
 
-    # If an instance_reader is needed, generate methods for name and name= on the
-    # class itself, so instances will be able to see them
-    define_method(name) { send("_#{name}") } if options[:instance_reader] != false
-    define_method("#{name}?") { !!send("#{name}") } if options[:instance_reader] != false
+    # If an instance_reader is needed, generate public instance methods name and name?.
+    if options[:instance_reader] != false
+      define_method(name) { send("_#{name}") }
+      define_method("#{name}?") { !!send("#{name}") }
+    end
   end
 
   private
diff --git a/activesupport/test/core_ext/class/delegating_attributes_test.rb b/activesupport/test/core_ext/class/delegating_attributes_test.rb
index 148f82946c..0e0742d147 100644
--- a/activesupport/test/core_ext/class/delegating_attributes_test.rb
+++ b/activesupport/test/core_ext/class/delegating_attributes_test.rb
@@ -39,10 +39,13 @@ class DelegatingAttributesTest < ActiveSupport::TestCase
   end
 
   def test_simple_accessor_declaration_with_instance_reader_false
+    _instance_methods = single_class.public_instance_methods
     single_class.superclass_delegating_accessor :no_instance_reader, :instance_reader => false
     assert_respond_to single_class, :no_instance_reader
     assert_respond_to single_class, :no_instance_reader=
-    assert !single_class.public_instance_methods.map(&:to_s).include?("no_instance_reader")
+    assert !_instance_methods.include?(:no_instance_reader)
+    assert !_instance_methods.include?(:no_instance_reader?)
+    assert !_instance_methods.include?(:_no_instance_reader)
   end
 
   def test_working_with_simple_attributes