Feature #14056: Add Module#deprecate_public for deprecation of public methods

• Status: Rejected
• Assignee: matz (Yukihiro Matsumoto)
• Target version: -

[jeremyevans0 (Jeremy Evans)]

The attached patch allows you to make a method private, but have
calling it publicly (either directly or via public_send) emit a
warning instead of raising a NoMethodError.

This is mostly useful in library development, where you want to
continue using a method internally, and you want to disallow external
use, but you want existing external users who are using the method to
receive a warning during a deprecation period instead of breaking their
code immediately.

I believe this feature is not possible without modifying the VM
(as the patch does). You can emit a deprecation message inside the
method, but I don't believe you can make it conditional on whether the
method was called publicly or privately, as that information isn't exposed.

Use is similar to public/protected/private when called with arguments:

  class MyClass
    def meth
      1
    end
    deprecate_public :meth
  end

  MyClass.new.meth
  # warning: calling private method meth on #<MyClass:0x00001c896dfb1a90> \
  #   via deprecated public interface
  # => 1

Module#deprecate_public makes the method private, but sets a flag on
the method entry that it is deprecated, and if calling the method would
usually raise a NoMethodError due to visibility, instead a warning is
printed and then method call works as if it were declared public.

There are some issues with this implementation of deprecate_public:

1. It doesn't handle scope visibility, so the following
   does not work like public/protected/private:

  class MyClass
    deprecate_public

    def meth
      1
    end
  end

Currently, this deprecate_public call would do nothing
as no arguments were given. It's probably possible to
handle scope visibility as well, but it would require
additional internal changes.

2. It is rather inefficient, as it first exports the method
   in the module as public and then private, before setting
   the deprecation flag. However, this method is not likely
   to be a bottleneck in any reasonable code. It was done this
   way to reuse the most existing code and still ensure that
   methods will be setup in the class itself and that method
   caches will be cleared appropriately.

3. When public_send is used, this does not print the receiver
   of the public_send method, instead it prints the module
   that used the deprecate_public call. I'm not sure whether
   the calling information is available from rb_method_call_status,
   or how to access it if it is available.

4. This doesn't currently handle protected methods, but I
   think changing it to do so isn't too difficult.

5. The method name isn't great, hopefully someone can think of
   a better one that isn't much longer.

[marcandre (Marc-Andre Lafortune)]

• Assignee set to matz (Yukihiro Matsumoto)

Just curious about the use case: since you have the control over private methods, why not simply do something like

 class MyClass
    def meth
      warn "meth is deprecated, please call instead some_other_meth"
      _meth 
    end
    
    private
    def _meth
      1
    end
 end

Internally, you use _meth until the next major release. The, you're free to delete meth and rename usage of _meth to meth in your code.

Also, FWIW I think of two ways to achieve your feature without modifying the VM.

One way would be to check caller_locations.first.path. Another way is by using method_missing and respond_to_missing?.

A third way would be using the gem binding_of_caller and check who the caller is, but that's not portable though.

Nevertheless, I'm not opposed to the feature, I'm just not convinced of it's necessity. I would much prefer having something like binding_of_caller become part of the language.

[jeremyevans0 (Jeremy Evans)]

marcandre (Marc-Andre Lafortune) wrote:

Just curious about the use case: since you have the control over private methods, why not simply do something like

 class MyClass
    def meth
      warn "meth is deprecated, please call instead some_other_meth"
      _meth 
    end
    
    private
    def _meth
      1
    end
 end

In libraries where the expected use case has the user subclassing a class provided by the library or including a module provided by the library, and then overriding methods defined in the class/module and calling super to get the default behavior, "control" over methods is something that does not necessary fall on simple public/private bounds. Think of such methods as extension points. There may be extension points where the method should be called publicly, and extension points where the method should not be called publicly. A similar issue occurs in plugin systems, where users provide plugins that override/augment the default behavior of the library.

For example, one thing I would consider using this for is before_* and after_* hook methods in Sequel. These methods have always been public, but they should really be private as they shouldn't be called outside of an internal method (save). It is expected that users would override them in their own model classes and plugins. Renaming in this way would cause a ton of churn.

Internally, you use _meth until the next major release. The, you're free to delete meth and rename usage of _meth to meth in your code.

This can require much more work if the methods are used extensively internally in a private manner. Additionally, in the example given above, it's unreasonable to ask all users of your library to rename all methods operating as extension points.

Also, FWIW I think of two ways to achieve your feature without modifying the VM.

One way would be to check caller_locations.first.path. Another way is by using method_missing and respond_to_missing?.

I don't think checking caller will work, since it can't tell you if the call was made private or public. We'll use caller.first in this example, as caller_locations.first.path only tells you the file, and tells you nothing about whether the call was public or private:

def b; p caller.first end
def a; self.b; b; send(:b); public_send(:b); end

result:

"(irb):6:in `a'"
"(irb):6:in `a'"
"(irb):6:in `a'"
"(irb):6:in `public_send'"

So you can tell the difference between public_send and the others, but not between self.b, b, and send(:b). send is optimized in this case I believe, which is why it doesn't have it's own entry in caller.

It is true you can use method_missing for this:

class A
  def method_missing(meth, *a, &block)
    if meth == :b
      warn("b called publicly, not privately")
      b(*a, &block)
    else
      super
    end
  end
  def a; self.b; b; send(:b); public_send(:b); end
  private
  def b; p :b end
end
A.new.a

That seems like a reasonable solution, especially since you can implement Module#deprecate_public using it.

A third way would be using the gem binding_of_caller and check who the caller is, but that's not portable though.

Nevertheless, I'm not opposed to the feature, I'm just not convinced of it's necessity. I would much prefer having something like binding_of_caller become part of the language.

I'm not sure if binding_of_caller can tell the difference between foo.send(:b) (private allowed) and foo.b (private not allowed) calls.

Thanks for taking the time to review and respond. With your recommendation to use method_missing, I agree there is not a significant need for VM changes as this functionality can be accomplished in pure ruby assuming you know the details in how method lookup, visibility, and method_missing work together.

[marcandre (Marc-Andre Lafortune)]

You're completely right for the rest, I got confused as to how private Ruby methods are.

Glad I could help. And now, even for methods with blocks there wouldn't even be a performance issue :-)

So we should close the issue, then?

[shevegen (Robert A. Heiler)]

To me the suggestion by Jeremy appears to be reasonable.

I don't really need it myself, but I am all for more fine-tuned
control of ruby in regards to messages (including warnings) and
errors, and I think that Jeremy has made a few other suggestions
that fall into that area too. I guess it depends on how matz
feels about the suggested change. (On a side note, when I write
more fine-tuned control in general, I also refers to great
introspection e. g. see what Martin Duerst proposed in another
issue about more introspection-capabilities in regards to
Unicode/Encoding; this also falls very close to more introspection
power in general, including being able to easily modify ruby's
behaviour in general when it comes to warnings and errors, but
I digress so I stop here).

[jeremyevans0 (Jeremy Evans)]

• Status changed from Open to Rejected

marcandre (Marc-Andre Lafortune) wrote:

You're completely right for the rest, I got confused as to how private Ruby methods are.

Glad I could help. And now, even for methods with blocks there wouldn't even be a performance issue :-)

So we should close the issue, then?

Yes, I'll close it.