Project

General

Profile

Actions

Bug #17519

closed

set_visibility fails when a prepended module and a refinement both exist

Added by fledman (David Feldman) about 3 years ago. Updated almost 3 years ago.

Status:
Closed
Assignee:
-
Target version:
-
ruby -v:
ruby 3.0.0p0 (2020-12-25 revision 95aff21468) [x86_64-darwin19]
[ruby-core:101981]

Description

the set_visibility functions (aka public/private/protected) fail with NameError when:

  • called on a specific object's singleton class
  • for a specific method
  • and both of the following are true
  • any module has been prepended to the object's class
  • a refinement exists for the specific method

note that the refinement does not need to ever be used

I have reproduced this on 3.0.0, 2.7.2, and 2.6.6 (those were the only 3 version I tested)

def test_visibility(function)
  h1 = {x:1, y:1}
  h2 = {x:2, y:2}

  h1.singleton_class.send(:private, function)
  h2.singleton_class.send(:public, function)

  begin
    puts h1.public_send(function, :x)
  rescue NoMethodError => err
    puts "hit NoMethodError as expected: #{err.inspect}"
  end

  puts h2.public_send(function, :x)
end

succeeds without any prepended modules or refinements:

irb> test_visibility :except
hit NoMethodError as expected: #<NoMethodError: private method `except' called for {:x=>1, :y=>1}:Hash>
{:y=>2}
=> nil

succeeds with only a prepended module:

irb> module Nothing; end; Hash.prepend(Nothing); Hash.ancestors
=> [Nothing, Hash, Enumerable, Object, PP::ObjectMixin, Kernel, BasicObject]

irb> test_visibility :except
hit NoMethodError as expected: #<NoMethodError: private method `except' called for {:x=>1, :y=>1}:Hash>
{:y=>2}
=> nil

succeeds with only a refinement:

irb> module NeverUsed
  refine Hash do
    def except(*keys)
      {never: 'used'}
    end
  end
end
=> #<refinement:Hash@NeverUsed>

irb> test_visibility :except
hit NoMethodError as expected: #<NoMethodError: private method `except' called for {:x=>1, :y=>1}:Hash>
{:y=>2}
=> nil

fails with both a refinement and a prepended module:

irb> module Nothing; end; Hash.prepend(Nothing); Hash.ancestors
=> [Nothing, Hash, Enumerable, Object, PP::ObjectMixin, Kernel, BasicObject]

irb> module NeverUsed
  refine Hash do
    def except(*keys)
      {never: 'used'}
    end
  end
end
=> #<refinement:Hash@NeverUsed>

irb> test_visibility :except
Traceback (most recent call last):
        6: from .rubies/ruby-3.0.0/bin/irb:23:in `<main>'
        5: from .rubies/ruby-3.0.0/bin/irb:23:in `load'
        4: from .rubies/ruby-3.0.0/lib/ruby/gems/3.0.0/gems/irb-1.3.0/exe/irb:11:in `<top (required)>'
        3: from (irb):25:in `<main>'
        2: from (irb):5:in `test_visibility'
        1: from (irb):5:in `private'
NameError (undefined method `except' for class `#<Class:#<Hash:0x00007ffd6b176f18>>')
Did you mean?  exec

# non-refined method still works
irb> test_visibility :fetch
hit NoMethodError as expected: #<NoMethodError: private method `fetch' called for {:x=>1, :y=>1}:Hash>
2
=> nil

Updated by fledman (David Feldman) about 3 years ago

since :except is only available natively on Ruby3, here is a Ruby2 demonstration:

irb> RUBY_VERSION
=> "2.7.2"

irb> module Nothing; end; Hash.prepend(Nothing); Hash.ancestors
=> [Nothing, Hash, Enumerable, Object, PP::ObjectMixin, Kernel, BasicObject]

irb> module NeverUsed
  refine Hash do
    def fetch(key)
      42
    end
  end
end
=> #<refinement:Hash@NeverUsed>

irb> test_visibility :fetch
Traceback (most recent call last):
        6: from /Users/davidfeldman/.rubies/ruby-2.7.2/bin/irb:23:in `<main>'
        5: from /Users/davidfeldman/.rubies/ruby-2.7.2/bin/irb:23:in `load'
        4: from /Users/davidfeldman/.rubies/ruby-2.7.2/lib/ruby/gems/2.7.0/gems/irb-1.2.6/exe/irb:11:in `<top (required)>'
        3: from (irb):25
        2: from (irb):14:in `test_visibility'
        1: from (irb):14:in `private'
NameError (undefined method `fetch` for class `#<Class:#<Hash:0x00007fcdf3ac7ec0>>')

# non-refined method still works
irb> test_visibility :[]
hit NoMethodError as expected: #<NoMethodError: private method `[]` called for {:x=>1, :y=>1}:Hash Did you mean? []=>
2
=> nil

Updated by fledman (David Feldman) about 3 years ago

and a demonstration with a custom class:

RUBY_VERSION

class Thing
  def direction
    'LEFT'
  end
end

def test_thing_visibility
  t1 = Thing.new
  t2 = Thing.new

  t1.singleton_class.send(:private, :direction)
  t2.singleton_class.send(:public, :direction)

  begin
    puts t1.direction
  rescue NoMethodError => err
    puts "hit NoMethodError as expected: #{err.inspect}"
  end

  puts t2.direction
end

test_thing_visibility

module NeverUsed
  refine Thing do
    def direction
      'UP'
    end
  end
end

test_thing_visibility

module Nothing; end; Thing.prepend(Nothing); Thing.ancestors

test_thing_visibility

outputs:

=> "2.6.6"
=> :direction
=> :test_thing_visibility
hit NoMethodError as expected: #<NoMethodError: private method `direction' called for #<Thing:0x00007f8c6f871ce0>>
LEFT
=> nil
=> #<refinement:Thing@NeverUsed>
hit NoMethodError as expected: #<NoMethodError: private method `direction' called for #<Thing:0x00007f8c6f8b4a68>>
LEFT
=> nil
=> [Nothing, Thing, Object, PP::ObjectMixin, Kernel, BasicObject]
Traceback (most recent call last):
        6: from /Users/davidfeldman/.rubies/ruby-2.6.6/bin/irb:23:in `<main>'
        5: from /Users/davidfeldman/.rubies/ruby-2.6.6/bin/irb:23:in `load'
        4: from /Users/davidfeldman/.rubies/ruby-2.6.6/lib/ruby/gems/2.6.0/gems/irb-1.0.0/exe/irb:11:in `<top (required)>'
        3: from (irb):37
        2: from (irb):11:in `test_thing_visibility'
        1: from (irb):11:in `private'
NameError (undefined method `direction' for class `#<Class:#<Thing:0x00007f8c6f8dcd10>>')

Updated by fledman (David Feldman) about 3 years ago

although the title makes it sound obscure, this bug is actually fairly easy to trigger when using rspec and rails:

  • activesupport >= 5 prepends a module onto Hash
  • i18n >= 1.3 refines several methods on Hash
  • power_assert (used by test-unit and minitest) refines most of the basic operators of the core classes

e.g. try to expect(some_hash).to receive(:except)

Actions #4

Updated by jeremyevans0 (Jeremy Evans) about 3 years ago

I've added a pull request to fix this: https://github.com/ruby/ruby/pull/4200

Actions #5

Updated by jeremyevans (Jeremy Evans) about 3 years ago

  • Status changed from Open to Closed

Applied in changeset git|58660e943488778563b9e41005a601e9660ce21f.


Skip refined method when exporting methods with changed visibility

Previously, attempting to change the visibility of a method in a
singleton class for a class/module that is prepended to and refined
would raise a NoMethodError.

Fixes [Bug #17519]

Updated by fledman (David Feldman) about 3 years ago

do you plan to backport the fix to any of the supported 2.x versions?

Updated by jeremyevans0 (Jeremy Evans) about 3 years ago

  • Backport changed from 2.5: UNKNOWN, 2.6: UNKNOWN, 2.7: UNKNOWN, 3.0: UNKNOWN to 2.5: UNKNOWN, 2.6: REQUIRED, 2.7: REQUIRED, 3.0: REQUIRED

fledman (David Feldman) wrote in #note-6:

do you plan to backport the fix to any of the supported 2.x versions?

The choice of which patches to backport is up to the branch maintainer. I've marked this for backporting, but it is their decision.

Updated by nagachika (Tomoyuki Chikanaga) almost 3 years ago

  • Backport changed from 2.5: UNKNOWN, 2.6: REQUIRED, 2.7: REQUIRED, 3.0: REQUIRED to 2.5: UNKNOWN, 2.6: REQUIRED, 2.7: DONE, 3.0: REQUIRED

ruby_2_7 6e962f02b266c3a6c47e50cf2e9ab7b1db25e515 merged revision(s) 58660e943488778563b9e41005a601e9660ce21f.

Updated by naruse (Yui NARUSE) almost 3 years ago

  • Backport changed from 2.5: UNKNOWN, 2.6: REQUIRED, 2.7: DONE, 3.0: REQUIRED to 2.5: UNKNOWN, 2.6: REQUIRED, 2.7: DONE, 3.0: DONE

ruby_3_0 d1cec0bca588266b9af1d55e592016c45ee68fbb merged revision(s) 58660e943488778563b9e41005a601e9660ce21f.

Updated by fledman (David Feldman) almost 3 years ago

this fix seems to have introduced a new but similar bug: https://github.com/ruby/ruby/pull/4200#issuecomment-813671308

Updated by jeremyevans0 (Jeremy Evans) almost 3 years ago

fledman (David Feldman) wrote in #note-10:

this fix seems to have introduced a new but similar bug: https://github.com/ruby/ruby/pull/4200#issuecomment-813671308

I can confirm the issue. Here's a pull request that fixes it: https://github.com/ruby/ruby/pull/4357

Actions #12

Updated by jeremyevans0 (Jeremy Evans) almost 3 years ago

  • Status changed from Closed to Open
Actions #13

Updated by jeremyevans (Jeremy Evans) almost 3 years ago

  • Status changed from Open to Closed

Applied in changeset git|4b36a597f48c857aa5eb9ed80fec0d02f6284646.


Fix setting method visibility for a refinement without an origin class

If a class has been refined but does not have an origin class,
there is a single method entry marked with VM_METHOD_TYPE_REFINED,
but it contains the original method entry. If the original method
entry is present, we shouldn't skip the method when searching even
when skipping refined methods.

Fixes [Bug #17519]

Updated by nagachika (Tomoyuki Chikanaga) almost 3 years ago

  • Backport changed from 2.5: UNKNOWN, 2.6: REQUIRED, 2.7: DONE, 3.0: DONE to 2.5: UNKNOWN, 2.6: REQUIRED, 2.7: REQUIRED, 3.0: REQUIRED

reset Backport field to backport git|4b36a597f48c857aa5eb9ed80fec0d02f6284646.

Updated by nagachika (Tomoyuki Chikanaga) almost 3 years ago

  • Backport changed from 2.5: UNKNOWN, 2.6: REQUIRED, 2.7: REQUIRED, 3.0: REQUIRED to 2.5: UNKNOWN, 2.6: REQUIRED, 2.7: REQUIRED, 3.0: DONE

ruby_3_0 7b6a2ad04a3272a31323493133498dfc60d77d76 merged revision(s) 4b36a597f48c857aa5eb9ed80fec0d02f6284646.

Actions

Also available in: Atom PDF

Like0
Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0