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
    headius committed Aug 23, 2017
    Copy the full SHA
    c90582c View commit details

  2. Included module's prepend should not obscure reflection. 

    See #4723.
    @headius
    headius committed Aug 23, 2017
    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
View file
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
View file
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
View file
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