From 838cf85f7b6bbb7198dde28ec02c9dc6211e69ea Mon Sep 17 00:00:00 2001 From: Jeremy Evans Date: Wed, 25 Oct 2017 16:57:25 -0700 Subject: [PATCH] Add Module#deprecate_public for deprecation of public methods This 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 interpreter (as this 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 # \ # 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. --- method.h | 11 +++++++++-- test/ruby/test_method.rb | 47 +++++++++++++++++++++++++++++++++++++++++++++++ test/ruby/test_object.rb | 6 ++++++ vm_eval.c | 14 +++++++++++++- vm_insnhelper.c | 19 +++++++++++++------ vm_method.c | 38 ++++++++++++++++++++++++++++++++++++++ 6 files changed, 126 insertions(+), 9 deletions(-) diff --git a/method.h b/method.h index fe02db3000..5b82d8bfbd 100644 --- a/method.h +++ b/method.h @@ -66,6 +66,7 @@ typedef struct rb_callable_method_entry_struct { /* same fields with rb_method_e #define METHOD_ENTRY_VISI(me) (rb_method_visibility_t)(((me)->flags & (IMEMO_FL_USER0 | IMEMO_FL_USER1)) >> (IMEMO_FL_USHIFT+0)) #define METHOD_ENTRY_BASIC(me) (int) (((me)->flags & (IMEMO_FL_USER2 )) >> (IMEMO_FL_USHIFT+2)) +#define METHOD_ENTRY_DEPRECATED(me) (int) (((me)->flags & (IMEMO_FL_USER4 )) >> (IMEMO_FL_USHIFT+4)) #define METHOD_ENTRY_COMPLEMENTED(me) ((me)->flags & IMEMO_FL_USER3) #define METHOD_ENTRY_COMPLEMENTED_SET(me) ((me)->flags = (me)->flags | IMEMO_FL_USER3) @@ -82,6 +83,12 @@ METHOD_ENTRY_BASIC_SET(rb_method_entry_t *me, unsigned int basic) me->flags = (me->flags & ~(IMEMO_FL_USER2 )) | (basic << (IMEMO_FL_USHIFT+2)); } static inline void +METHOD_ENTRY_DEPRECATED_SET(rb_method_entry_t *me, unsigned int deprecated) +{ + VM_ASSERT(deprecated <= 1); + me->flags = (me->flags & ~(IMEMO_FL_USER4 )) | (deprecated << (IMEMO_FL_USHIFT+4)); +} +static inline void METHOD_ENTRY_FLAGS_SET(rb_method_entry_t *me, rb_method_visibility_t visi, unsigned int basic) { VM_ASSERT((int)visi >= 0 && visi <= 3); @@ -94,8 +101,8 @@ static inline void METHOD_ENTRY_FLAGS_COPY(rb_method_entry_t *dst, const rb_method_entry_t *src) { dst->flags = - (dst->flags & ~(IMEMO_FL_USER0|IMEMO_FL_USER1|IMEMO_FL_USER2)) | - (src->flags & (IMEMO_FL_USER0|IMEMO_FL_USER1|IMEMO_FL_USER2)); + (dst->flags & ~(IMEMO_FL_USER0|IMEMO_FL_USER1|IMEMO_FL_USER2|IMEMO_FL_USER4)) | + (src->flags & (IMEMO_FL_USER0|IMEMO_FL_USER1|IMEMO_FL_USER2|IMEMO_FL_USER4)); } typedef enum { diff --git a/test/ruby/test_method.rb b/test/ruby/test_method.rb index ce67aada2b..02e151b54b 100644 --- a/test/ruby/test_method.rb +++ b/test/ruby/test_method.rb @@ -420,6 +420,53 @@ def m assert_equal(:m1, c2.new.m.call, 'see [Bug #4881] and [Bug #3136]') end + def test_deprecate_public + c1 = Class.new do + def dp + :dp + end + deprecate_public :dp + end + + refute_includes c1.instance_methods, :dp + assert_includes c1.private_instance_methods, :dp + assert_equal false, c1.public_method_defined?(:dp) + assert_equal true, c1.private_method_defined?(:dp) + + o = c1.new + assert_equal :dp, o.dp + assert_equal :dp, o.public_send(:dp) + refute_includes o.public_methods, :dp + assert_includes o.private_methods, :dp + assert_equal false, o.respond_to?(:dp) + assert_equal true, o.respond_to?(:dp, true) + end + + def test_deprecate_public_module + m1 = Module.new do + def dp + :dp + end + end + + c1 = Class.new + c1.include m1 + m1.send :deprecate_public, :dp + + refute_includes c1.instance_methods, :dp + assert_includes c1.private_instance_methods, :dp + assert_equal false, c1.public_method_defined?(:dp) + assert_equal true, c1.private_method_defined?(:dp) + + o = c1.new + assert_equal :dp, o.dp + assert_equal :dp, o.public_send(:dp) + refute_includes o.public_methods, :dp + assert_includes o.private_methods, :dp + assert_equal false, o.respond_to?(:dp) + assert_equal true, o.respond_to?(:dp, true) + end + def test_clone o = Object.new def o.foo; :foo; end diff --git a/test/ruby/test_object.rb b/test/ruby/test_object.rb index e55a09dc23..d345ea2a3e 100644 --- a/test/ruby/test_object.rb +++ b/test/ruby/test_object.rb @@ -686,6 +686,11 @@ def invoke(m) public_send(m) end + def dep_pub + :ok + end + deprecate_public :dep_pub + protected def prot :ng @@ -699,6 +704,7 @@ def priv assert_equal(:ok, c.public_send(:pub)) assert_raise(NoMethodError) {c.public_send(:priv)} assert_raise(NoMethodError) {c.public_send(:prot)} + assert_equal(:ok, c.public_send(:dep_pub)) assert_raise(NoMethodError) {c.invoke(:priv)} bug7499 = '[ruby-core:50489]' assert_raise(NoMethodError, bug7499) {c.invoke(:prot)} diff --git a/vm_eval.c b/vm_eval.c index 31344cd305..46ce9aa37c 100644 --- a/vm_eval.c +++ b/vm_eval.c @@ -548,7 +548,19 @@ rb_method_call_status(rb_thread_t *th, const rb_callable_method_entry_t *me, cal /* receiver specified form for private method */ if (UNLIKELY(visi != METHOD_VISI_PUBLIC)) { if (visi == METHOD_VISI_PRIVATE && scope == CALL_PUBLIC) { - return MISSING_PRIVATE; + if (METHOD_ENTRY_DEPRECATED(me)) { + if (RB_TYPE_P(klass, T_ICLASS)) { + klass = RBASIC(klass)->klass; + } + VALUE calling_str = rb_inspect(klass); + rb_warn("calling private method %s on %s via deprecated public interface", + rb_id2name(me->called_id), + rb_string_value_ptr(&calling_str)); + return MISSING_NONE; + } + else { + return MISSING_PRIVATE; + } } /* self must be kind of a specified form for protected method */ diff --git a/vm_insnhelper.c b/vm_insnhelper.c index 3e3e7d5d6d..391a0c5452 100644 --- a/vm_insnhelper.c +++ b/vm_insnhelper.c @@ -2375,12 +2375,19 @@ vm_call_method(rb_thread_t *th, rb_control_frame_t *cfp, struct rb_calling_info case METHOD_VISI_PRIVATE: if (!(ci->flag & VM_CALL_FCALL)) { - enum method_missing_reason stat = MISSING_PRIVATE; - if (ci->flag & VM_CALL_VCALL) stat |= MISSING_VCALL; - - cc->aux.method_missing_reason = stat; - CI_SET_FASTPATH(cc, vm_call_method_missing, 1); - return vm_call_method_missing(th, cfp, calling, ci, cc); + if (METHOD_ENTRY_DEPRECATED(cc->me)) { + VALUE calling_str = rb_inspect(calling->recv); + rb_warn("calling private method %s on %s via deprecated public interface", + rb_id2name(cc->me->called_id), + rb_string_value_ptr(&calling_str)); + } else { + enum method_missing_reason stat = MISSING_PRIVATE; + if (ci->flag & VM_CALL_VCALL) stat |= MISSING_VCALL; + + cc->aux.method_missing_reason = stat; + CI_SET_FASTPATH(cc, vm_call_method_missing, 1); + return vm_call_method_missing(th, cfp, calling, ci, cc); + } } return vm_call_method_each_type(th, cfp, calling, ci, cc); diff --git a/vm_method.c b/vm_method.c index 2320b54aed..fe504858ee 100644 --- a/vm_method.c +++ b/vm_method.c @@ -1641,6 +1641,43 @@ set_visibility(int argc, const VALUE *argv, VALUE module, rb_method_visibility_t return module; } +/* + * call-seq: + * deprecate_public(symbol, ...) -> self + * deprecate_public(string, ...) -> self + * + * Sets the named methods to be private but be + * callable as public methods, issuing a deprecation warning + * when they are called in a non-private context. + * String arguments are converted to symbols. + */ + +static VALUE +rb_mod_deprecate_public(int argc, VALUE *argv, VALUE self) +{ + int i; + + if (argc == 0) { + return self; + } + + for (i = 0; i < argc; i++) { + VALUE v = argv[i]; + ID id = rb_check_id(&v); + rb_method_entry_t *meth; + if (!id) { + rb_print_undef_str(self, v); + } + rb_export_method(self, id, METHOD_VISI_PUBLIC); + rb_export_method(self, id, METHOD_VISI_PRIVATE); + meth = search_method(self, id, 0); + METHOD_ENTRY_DEPRECATED_SET(meth, 1); + } + + rb_clear_method_cache_by_class(self); + return self; +} + /* * call-seq: * public -> self @@ -2094,6 +2131,7 @@ Init_eval_method(void) rb_define_private_method(rb_cModule, "remove_method", rb_mod_remove_method, -1); rb_define_private_method(rb_cModule, "undef_method", rb_mod_undef_method, -1); rb_define_private_method(rb_cModule, "alias_method", rb_mod_alias_method, 2); + rb_define_private_method(rb_cModule, "deprecate_public", rb_mod_deprecate_public, -1); rb_define_private_method(rb_cModule, "public", rb_mod_public, -1); rb_define_private_method(rb_cModule, "protected", rb_mod_protected, -1); rb_define_private_method(rb_cModule, "private", rb_mod_private, -1); -- 2.14.2