Skip to content
Permalink

Comparing changes

Choose two branches to see what’s changed or to start a new pull request. If you need to, you can also or learn more about diff comparisons.

Open a pull request

Create a new pull request by comparing changes across two branches. If you need to, you can also . Learn more about diff comparisons here.
base repository: jruby/jruby
Failed to load repositories. Confirm that selected base ref is valid, then try again.
Loading
base: a3765d89b4b6
Choose a base ref
...
head repository: jruby/jruby
Failed to load repositories. Confirm that selected head ref is valid, then try again.
Loading
compare: 125be53d0e8d
Choose a head ref
  • 2 commits
  • 3 files changed
  • 1 contributor

Commits on Aug 23, 2017

  1. Include hierarchy of prepends when adding method names from mod.

    Fixes #4723
    
    Method searching for calls used overridden logic in the included
    module wrapper, but we did not do the same logic when
    traversing all methods to add them to a reflective method list
    (such as instance_methods). This patch adds overridden logic in
    the wrapper to walk the prepends just as in the call site case.
    headius committed Aug 23, 2017
    Copy the full SHA
    c90582c View commit details
  2. Copy the full SHA
    125be53 View commit details
Showing with 48 additions and 13 deletions.
  1. +16 −0 core/src/main/java/org/jruby/IncludedModuleWrapper.java
  2. +16 −13 core/src/main/java/org/jruby/RubyModule.java
  3. +16 −0 spec/ruby/core/module/prepend_spec.rb
16 changes: 16 additions & 0 deletions core/src/main/java/org/jruby/IncludedModuleWrapper.java
Original file line number Diff line number Diff line change
@@ -35,8 +35,10 @@
import java.util.Collection;
import java.util.List;
import java.util.Map;
import java.util.Set;

import org.jruby.internal.runtime.methods.DynamicMethod;
import org.jruby.runtime.Visibility;
import org.jruby.runtime.builtin.IRubyObject;
import org.jruby.runtime.builtin.Variable;

@@ -225,4 +227,18 @@ protected DynamicMethod searchMethodCommon(String name) {

return null;
}

@Override
protected void addMethodSymbols(Ruby runtime, Set<String> seen, RubyArray ary, boolean not, Visibility visibility) {
// IncludedModuleWrapper needs to search prepended modules too, so search until we find methodLocation
RubyModule module = origin;
RubyModule methodLoc = origin.getMethodLocation();

for (; module != methodLoc; module = module.getSuperClass()) {
module.addMethodSymbols(runtime, seen, ary, not, visibility);
}

// one last add for method location
module.addMethodSymbols(runtime, seen, ary, not, visibility);
}
}
29 changes: 16 additions & 13 deletions core/src/main/java/org/jruby/RubyModule.java
Original file line number Diff line number Diff line change
@@ -2453,26 +2453,29 @@ public void populateInstanceMethodNames(Set<String> seen, RubyArray ary, Visibil
}

for (; mod != null; mod = mod.getSuperClass()) {
RubyModule realType = mod.getNonIncludedClass();
for (Map.Entry entry : mod.getMethods().entrySet()) {
String methodName = (String) entry.getKey();
mod.addMethodSymbols(runtime, seen, ary, not, visibility);

if (! seen.contains(methodName)) {
seen.add(methodName);
if (mod.isIncluded() && !prepended) continue;
if (obj && mod.isSingleton()) continue;
if (!recur) break;
}
}

protected void addMethodSymbols(Ruby runtime, Set<String> seen, RubyArray ary, boolean not, Visibility visibility) {
for (Map.Entry entry : getMethods().entrySet()) {
String methodName = (String) entry.getKey();

DynamicMethod method = (DynamicMethod) entry.getValue();
if ((method.isImplementedBy(realType) || method.isImplementedBy(mod)) &&
if (! seen.contains(methodName)) {
seen.add(methodName);

DynamicMethod method = (DynamicMethod) entry.getValue();
if (//(method.isImplementedBy(realType) || method.isImplementedBy(mod)) &&
(!not && method.getVisibility() == visibility || (not && method.getVisibility() != visibility)) &&
! method.isUndefined()) {

ary.append(runtime.newSymbol(methodName));
}
ary.append(runtime.newSymbol(methodName));
}
}

if (mod.isIncluded() && !prepended) continue;
if (obj && mod.isSingleton()) continue;
if (!recur) break;
}
}

16 changes: 16 additions & 0 deletions spec/ruby/core/module/prepend_spec.rb
Original file line number Diff line number Diff line change
@@ -342,4 +342,20 @@ def foo(ary)
child_class.new.foo(ary)
ary.should == [3, 2, 1]
end

describe "called on a module" do
describe "included into a class"
it "does not obscure the module's methods from reflective access" do
mod = Module.new do
def foo; end
end
cls = Class.new do
include mod
end
pre = Module.new
mod.prepend pre

cls.instance_methods.should include(:foo)
end
end
end