Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Warn for calling public/protected/private/module_function without arguments inside method #2562

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
42 changes: 42 additions & 0 deletions test/ruby/test_class.rb
Expand Up @@ -131,6 +131,48 @@ def test_module_specific_methods
[:module_function, :extend_object, :append_features, :prepend_features])
end

def test_visibility_inside_method
assert_warn(/calling private without arguments inside a method may not have the intended effect/, '[ruby-core:79751]') do
Class.new do
def self.foo
private
end
foo
end
end

assert_warn(/calling protected without arguments inside a method may not have the intended effect/, '[ruby-core:79751]') do
Class.new do
def self.foo
protected
end
foo
end
end

assert_warn(/calling public without arguments inside a method may not have the intended effect/, '[ruby-core:79751]') do
Class.new do
def self.foo
public
end
foo
end
end

assert_warn(/calling private without arguments inside a method may not have the intended effect/, '[ruby-core:79751]') do
Class.new do
class << self
alias priv private
end

def self.foo
priv
end
foo
end
end
end

def test_method_redefinition
feature2155 = '[ruby-dev:39400]'

Expand Down
11 changes: 11 additions & 0 deletions test/ruby/test_module.rb
Expand Up @@ -1394,6 +1394,17 @@ def foo; end
assert_match(/: warning: previous definition of foo/, stderr)
end

def test_module_function_inside_method
assert_warn(/calling module_function without arguments inside a method may not have the intended effect/, '[ruby-core:79751]') do
Module.new do
def self.foo
module_function
end
foo
end
end
end

def test_protected_singleton_method
klass = Class.new
x = klass.new
Expand Down
15 changes: 14 additions & 1 deletion vm_method.c
Expand Up @@ -1126,9 +1126,21 @@ rb_scope_visibility_set(rb_method_visibility_t visi)
vm_cref_set_visibility(visi, FALSE);
}

static void
scope_visibility_check(void)
{
/* Check for public/protected/private/module_function called inside a method */
rb_control_frame_t *cfp = rb_current_execution_context()->cfp+1;
if (cfp && cfp->iseq && cfp->iseq->body->type == ISEQ_TYPE_METHOD) {
rb_warn("calling %s without arguments inside a method may not have the intended effect",
rb_id2name(rb_frame_this_func()));
}
}

static void
rb_scope_module_func_set(void)
{
scope_visibility_check();
vm_cref_set_visibility(METHOD_VISI_PRIVATE, TRUE);
}

Expand Down Expand Up @@ -1663,7 +1675,8 @@ static VALUE
set_visibility(int argc, const VALUE *argv, VALUE module, rb_method_visibility_t visi)
{
if (argc == 0) {
rb_scope_visibility_set(visi);
scope_visibility_check();
rb_scope_visibility_set(visi);
}
else {
set_method_visibility(module, argc, argv, visi);
Expand Down