Skip to content

Fix some specs regarding method visibility #6712

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.

Sign up for GitHub

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

Merged
headius merged 1 commit into from
Jun 14, 2021
Merged

Fix some specs regarding method visibility #6712

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 11 additions & 6 deletions core/src/main/java/org/jruby/RubyModule.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2370,15 +2370,18 @@ public IRubyObject defineMethodFromBlock(ThreadContext context, IRubyObject arg0
IRMethod method = closure.convertToMethod(name.getBytes());
if (method != null) {
newMethod = new DefineMethodMethod(method, visibility, this, context.getFrameBlock());
Helpers.addInstanceMethod(this, name, newMethod, visibility, context, runtime);
Visibility newVisibility = Helpers.performNormalMethodChecksAndDetermineVisibility(runtime, this, runtime.newSymbol(newMethod.getName()), newMethod.getVisibility(), false);
newMethod.setVisibility(newVisibility);
Helpers.addInstanceMethod(this, name, newMethod, newVisibility, context, runtime);
return name;
}
}
}

newMethod = createProcMethod(runtime, name.idString(), visibility, block);

Helpers.addInstanceMethod(this, name, newMethod, visibility, context, runtime);
Visibility newVisibility = Helpers.performNormalMethodChecksAndDetermineVisibility(runtime, this, runtime.newSymbol(newMethod.getName()), newMethod.getVisibility(), false);
newMethod.setVisibility(newVisibility);
Helpers.addInstanceMethod(this, name, newMethod, newVisibility, context, runtime);

return name;
}
Expand All @@ -2395,24 +2398,26 @@ public IRubyObject defineMethodFromCallable(ThreadContext context, IRubyObject a
RubySymbol name = TypeConverter.checkID(arg0);
DynamicMethod newMethod;

Visibility newVisibility = Helpers.performNormalMethodChecksAndDetermineVisibility(runtime, this, name, visibility, false);

if (runtime.getProc().isInstance(arg1)) {
// double-testing args.length here, but it avoids duplicating the proc-setup code in two places
RubyProc proc = (RubyProc)arg1;

newMethod = createProcMethod(runtime, name.idString(), visibility, proc.getBlock());
newMethod = createProcMethod(runtime, name.idString(), newVisibility, proc.getBlock());
} else if (arg1 instanceof AbstractRubyMethod) {
AbstractRubyMethod method = (AbstractRubyMethod)arg1;

checkValidBindTargetFrom(context, (RubyModule) method.owner(context));

newMethod = method.getMethod().dup();
newMethod.setImplementationClass(this);
newMethod.setVisibility(visibility);
newMethod.setVisibility(newVisibility);
} else {
throw runtime.newTypeError("wrong argument type " + arg1.getType().getName() + " (expected Proc/Method)");
}

Helpers.addInstanceMethod(this, name, newMethod, visibility, context, runtime);
Helpers.addInstanceMethod(this, name, newMethod, newVisibility, context, runtime);

return name;
}
Expand Down
6 changes: 3 additions & 3 deletions core/src/main/java/org/jruby/ir/runtime/IRRuntimeHelpers.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1633,7 +1633,7 @@ public static void defInterpretedInstanceMethod(ThreadContext context, IRScope m
RubyModule rubyClass = findInstanceMethodContainer(context, currDynScope, self);

Visibility currVisibility = context.getCurrentVisibility();
Visibility newVisibility = Helpers.performNormalMethodChecksAndDetermineVisibility(runtime, rubyClass, methodName, currVisibility);
Visibility newVisibility = Helpers.performNormalMethodChecksAndDetermineVisibility(runtime, rubyClass, methodName, currVisibility, true);

if (method.maybeUsingRefinements()) method.getStaticScope().captureParentRefinements(context);

Expand All @@ -1657,7 +1657,7 @@ public static void defCompiledInstanceMethod(ThreadContext context, MethodHandle
RubyModule clazz = findInstanceMethodContainer(context, currDynScope, self);

Visibility currVisibility = context.getCurrentVisibility();
Visibility newVisibility = Helpers.performNormalMethodChecksAndDetermineVisibility(runtime, clazz, methodName, currVisibility);
Visibility newVisibility = Helpers.performNormalMethodChecksAndDetermineVisibility(runtime, clazz, methodName, currVisibility, true);

if (maybeRefined) scope.captureParentRefinements(context);

Expand All @@ -1679,7 +1679,7 @@ public static void defCompiledInstanceMethod(ThreadContext context, MethodHandle
RubyModule clazz = findInstanceMethodContainer(context, currDynScope, self);

Visibility currVisibility = context.getCurrentVisibility();
Visibility newVisibility = Helpers.performNormalMethodChecksAndDetermineVisibility(runtime, clazz, methodName, currVisibility);
Visibility newVisibility = Helpers.performNormalMethodChecksAndDetermineVisibility(runtime, clazz, methodName, currVisibility, true);

if (maybeRefined) scope.captureParentRefinements(context);

Expand Down
20 changes: 13 additions & 7 deletions core/src/main/java/org/jruby/runtime/Helpers.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,6 @@
import org.jcodings.specific.UTF8Encoding;
import org.jcodings.unicode.UnicodeEncoding;

import static org.jruby.RubyBasicObject.UNDEF;
import static org.jruby.RubyBasicObject.getMetaClass;
import static org.jruby.runtime.Visibility.PRIVATE;
import static org.jruby.runtime.Visibility.PROTECTED;
Expand Down Expand Up @@ -2083,23 +2082,30 @@ public static StaticScope restoreScope(String descriptor, StaticScope enclosingS
}

public static Visibility performNormalMethodChecksAndDetermineVisibility(Ruby runtime, RubyModule clazz,
RubySymbol symbol, Visibility visibility) throws RaiseException {
String name = symbol.asJavaString(); // We just assume simple ascii string since that is all we are examining.
RubySymbol symbol, Visibility visibility, boolean checkSingleton) throws RaiseException {
String methodName = symbol.asJavaString(); // We just assume simple ascii string since that is all we are examining.

if (clazz == runtime.getDummy()) {
throw runtime.newTypeError("no class/module to add method");
}

if (clazz == runtime.getObject() && "initialize".equals(name)) {
if (clazz == runtime.getObject() && "initialize".equals(methodName)) {
runtime.getWarnings().warn(ID.REDEFINING_DANGEROUS, "redefining Object#initialize may cause infinite loop");
}

if ("__id__".equals(name) || "__send__".equals(name)) {
if ("__id__".equals(methodName) || "__send__".equals(methodName)) {
runtime.getWarnings().warn(ID.REDEFINING_DANGEROUS, str(runtime, "redefining `", ids(runtime, symbol), "' may cause serious problem"));
}

if ("initialize".equals(name) || "initialize_copy".equals(name) || name.equals("initialize_dup") || name.equals("initialize_clone") || name.equals("respond_to_missing?") || visibility == Visibility.MODULE_FUNCTION) {
visibility = Visibility.PRIVATE;
if ("initialize".equals(methodName) || "initialize_copy".equals(methodName) || methodName.equals("initialize_dup") || methodName.equals("initialize_clone") || methodName.equals("respond_to_missing?") || visibility == Visibility.MODULE_FUNCTION) {
if(checkSingleton) {
if(!clazz.isSingleton()){
visibility = Visibility.PRIVATE;
}
} else {
visibility = Visibility.PRIVATE;
}

}

return visibility;
Expand Down
4 changes: 1 addition & 3 deletions spec/tags/ruby/core/module/define_method_tags.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,4 @@
fails:Module#define_method when name is :initialize passed a block sets visibility to private when method name is :initialize
fails:Module#define_method when name is :initialize given an UnboundMethod sets the visibility to private when method is named :initialize
fails:Method#define_method when passed a Proc object and a method is defined inside defines the nested method in the default definee where the Proc was created
fails:Module#define_method raises a TypeError when an UnboundMethod from a child class is defined on a parent class
fails:Module#define_method raises a TypeError when an UnboundMethod from one class is defined on an unrelated class
fails:Module#define_method when the default definee is not the same as the module sets the visibility of the method to public
fails:Module#define_method when the default definee is not the same as the module sets the visibility of the method to public