From: merch-redmine@... Date: 2017-10-26T07:05:08+00:00 Subject: [ruby-core:83573] [Ruby trunk Feature#14056] Add Module#deprecate_public for deprecation of public methods Issue #14056 has been updated by 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 > > > ~~~ ruby > 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: ~~~ruby 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: ~~~ruby 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. ---------------------------------------- Feature #14056: Add Module#deprecate_public for deprecation of public methods https://bugs.ruby-lang.org/issues/14056#change-67600 * Author: jeremyevans0 (Jeremy Evans) * Status: Open * Priority: Normal * Assignee: matz (Yukihiro Matsumoto) * Target version: ---------------------------------------- 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: ~~~ruby class MyClass def meth 1 end deprecate_public :meth end MyClass.new.meth # warning: calling private method meth on # \ # 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: ~~~ruby 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. ---Files-------------------------------- 0001-Add-Module-deprecate_public-for-deprecation-of-publi.patch (11.1 KB) -- https://bugs.ruby-lang.org/ Unsubscribe: