Bug #13249
closedAccess modifiers don't have an effect inside class methods in Ruby >= 2.3
Description
Simple example:
class C
def self.foo
private
def bar
end
end
end
C.foo
C.new.bar
This code runs fine on Ruby 2.3 and Ruby 2.4. It raises NoMethodError on Ruby 2.2 and prior versions.
I would expect an error to be raised.
Here is some code that actually uses private access modifier inside a class method - https://github.com/evolve75/RubyTree/blob/db48c35b0a3b96e4da473b095cc00e454d8a9996/lib/tree/utils/camel_case_method_handler.rb#L60
By the way, this code raises an error as expected on Ruby 2.3 and Ruby 2.4:
class C
def self.foo
private def bar
end
end
end
C.foo
C.new.bar # NoMethodError: private method `bar' called
Files
Updated by abotalov (Andrei Botalov) over 7 years ago
- Description updated (diff)
Updated by abotalov (Andrei Botalov) over 7 years ago
- Description updated (diff)
Updated by nobu (Nobuyoshi Nakada) over 7 years ago
- Related to Bug #11571: シングルトンメソッドの中で def を使用した時の可視性が変わっている added
Updated by nobu (Nobuyoshi Nakada) over 7 years ago
- Related to Bug #11754: Visibility scope is kept after lexical scope is closed added
Updated by nobu (Nobuyoshi Nakada) over 7 years ago
Hmmm, a class singleton method should have its own visibility per invocations?
Updated by abotalov (Andrei Botalov) over 7 years ago
Well, ability to declare private
methods inside class methods seems strange given that it's not possible to declare private methods inside instance methods:
class C
def foo
private def bar
end
end
end
C.new.foo # NoMethodError is raised
So I'm not sure which route would be better.
Updated by shevegen (Robert A. Heiler) over 7 years ago
The examples confuse me a bit.
Does private actually make sense on any class-method / singleton method?
I understand it as a limitation for methods on the class, where outside
calls are not allowed, only internal ones (though ruby allows one to
bypass these anyway via .send).
I am also confused by the second example:
class C
def self.foo
private def bar
end
end
end
C.foo
C.new.bar
Is that not equivalent to
private
def self.bar
?
So why would this work on the C.new.bar()
level?
It is however had interesting that this worked on 2.2 and below but
was changed past that point.
Updated by abotalov (Andrei Botalov) over 7 years ago
- Description updated (diff)
Updated by shyouhei (Shyouhei Urabe) over 7 years ago
- Status changed from Open to Assigned
- Assignee set to ko1 (Koichi Sasada)
We looked at this issue in yesterday's developer meeting.
The use of private in evolve75/RubyTree shown in the description is in fact wrong (methods are defined in a wrong place). That example made us think that the use of private in a method is a code smell.
We would forbid such usage in a future. For the time being, let us show warning message.
Updated by ko1 (Koichi Sasada) over 7 years ago
I will insert warning.
I will not change the current behavior.
Updated by jeremyevans0 (Jeremy Evans) about 5 years ago
The warning discussed has not been added yet. Attached is a patch that implements it. While it passes make check
, there may be corner cases it doesn't handle. I would appreciate review of the changes to vm_cref_set_visibility
.
By adding this warning, I found a related issue in irb, which I submitted a pull request for: https://github.com/ruby/irb/pull/23
Updated by mame (Yusuke Endoh) about 5 years ago
- Assignee changed from ko1 (Koichi Sasada) to nobu (Nobuyoshi Nakada)
@nobu (Nobuyoshi Nakada) Could you review the patch?
Updated by nobu (Nobuyoshi Nakada) about 5 years ago
-
Is it necessary that check is placed inside the function
vm_cref_set_visibility
?
What about calling a separate function wherecheck_method
flag is 1? -
rb_frame_callee
returns the called name, that may be an aliased name.
Is it intentional?
Updated by jeremyevans0 (Jeremy Evans) about 5 years ago
nobu (Nobuyoshi Nakada) wrote:
- Is it necessary that check is placed inside the function
vm_cref_set_visibility
?
What about calling a separate function wherecheck_method
flag is 1?
I agree, that makes more sense.
rb_frame_callee
returns the called name, that may be an aliased name.
Is it intentional?
No. It would be better to use rb_frame_this_func
instead, I think.
Thank you very much for your review. I've added the modified patch as a pull request (https://github.com/ruby/ruby/pull/2562). Assuming it passes CI, I will merge it.
Updated by jeremyevans (Jeremy Evans) about 5 years ago
- Status changed from Assigned to Closed
Applied in changeset git|2993b24a1ecc5fa3cc9f140bfd80669c3a3b7b9c.
Warn for calling public/protected/private/module_function without arguments inside method
Calling these methods without an argument does not have the
desired effect inside a method.
Fixes [Bug #13249]