Improve error from path search during Marshal.load

headius · headius · commit c54c1633445e · 2025-04-28T09:44:09.000-05:00
Fixes one failure from DRb tests in ruby/drb#36
diff --git a/core/src/main/java/org/jruby/Ruby.java b/core/src/main/java/org/jruby/Ruby.java
@@ -2862,7 +2862,7 @@ public RubyModule getClassFromPath(final String path, RubyClass undefinedExcepti
         var context = getCurrentContext();
 
         if (path.length() == 0 || path.charAt(0) == '#') {
-            throw typeError(context, str(this, "can't retrieve anonymous class ", ids(this, path)));
+            throw argumentError(context, str(this, "can't retrieve anonymous class ", ids(this, path)));
         }
 
         RubyModule clazz = getObject();
@@ -2874,17 +2874,19 @@ public RubyModule getClassFromPath(final String path, RubyClass undefinedExcepti
 
             if ( p < length && path.charAt(p) == ':' ) {
                 if ( ++p < length && path.charAt(p) != ':' ) {
-                    throw newRaiseException(undefinedExceptionClass, str(this, "undefined class/module ", ids(this, path)));
+                    throw classPathUndefinedException(path, undefinedExceptionClass, p);
                 }
                 pbeg = ++p;
             }
 
             // FIXME: JI depends on const_missing getting called from Marshal.load (ruby objests do not).  We should marshal JI objects differently so we do not differentiate here.
             IRubyObject cc = flexibleSearch || isJavaPackageOrJavaClassProxyType(clazz) ?
-                    clazz.getConstant(context, str) : clazz.getConstantAt(context, str);
+                    clazz.constantTableFetch(str) : clazz.getConstantAt(context, str);
 
             if (!flexibleSearch && cc == null) return null;
 
+            if (cc == null) throw classPathUndefinedException(path, undefinedExceptionClass, p);
+
             if (!(cc instanceof RubyModule mod)) {
                 throw typeError(context, str(this, ids(this, path), " does not refer to class/module"));
             }
@@ -2894,6 +2896,10 @@ public RubyModule getClassFromPath(final String path, RubyClass undefinedExcepti
         return clazz;
     }
 
+    private RaiseException classPathUndefinedException(String path, RubyClass undefinedExceptionClass, int p) {
+        return newRaiseException(undefinedExceptionClass, str(this, "undefined class/module ", ids(this, path.substring(0, p))));
+    }
+
     private static boolean isJavaPackageOrJavaClassProxyType(final RubyModule type) {
         return type instanceof JavaPackage || ClassUtils.isJavaClassProxyType(type);
     }
diff --git a/core/src/main/java/org/jruby/runtime/marshal/MarshalLoader.java b/core/src/main/java/org/jruby/runtime/marshal/MarshalLoader.java
@@ -147,8 +147,7 @@ public static RubyModule getModuleFromPath(ThreadContext context, String path) {
 
     public static RubyClass getClassFromPath(ThreadContext context, String path) {
         Ruby runtime = context.runtime;
-        final RubyModule value = runtime.getClassFromPath(path, runtime.getArgumentError(), false);
-        if (value == null) throw argumentError(context, "undefined class/module " + path);
+        final RubyModule value = runtime.getClassFromPath(path, runtime.getArgumentError(), true);
         if ( ! value.isClass() ) throw argumentError(context, path + " does not refer class");
         return (RubyClass) value;
     }

Original file line number	Diff line number	Diff line change
`@@ -2862,7 +2862,7 @@ public RubyModule getClassFromPath(final String path, RubyClass undefinedExcepti`
`2862`	`2862`	`var context = getCurrentContext();`
`2863`	`2863`
`2864`	`2864`	`if (path.length() == 0 \|\| path.charAt(0) == '#') {`
`2865`		`- throw typeError(context, str(this, "can't retrieve anonymous class ", ids(this, path)));`
	`2865`	`+ throw argumentError(context, str(this, "can't retrieve anonymous class ", ids(this, path)));`
`2866`	`2866`	`}`
`2867`	`2867`
`2868`	`2868`	`RubyModule clazz = getObject();`
`@@ -2874,17 +2874,19 @@ public RubyModule getClassFromPath(final String path, RubyClass undefinedExcepti`
`2874`	`2874`
`2875`	`2875`	`if ( p < length && path.charAt(p) == ':' ) {`
`2876`	`2876`	`if ( ++p < length && path.charAt(p) != ':' ) {`
`2877`		`- throw newRaiseException(undefinedExceptionClass, str(this, "undefined class/module ", ids(this, path)));`
	`2877`	`+ throw classPathUndefinedException(path, undefinedExceptionClass, p);`
`2878`	`2878`	`}`
`2879`	`2879`	`pbeg = ++p;`
`2880`	`2880`	`}`
`2881`	`2881`
`2882`	`2882`	`// FIXME: JI depends on const_missing getting called from Marshal.load (ruby objests do not). We should marshal JI objects differently so we do not differentiate here.`
`2883`	`2883`	`IRubyObject cc = flexibleSearch \|\| isJavaPackageOrJavaClassProxyType(clazz) ?`
`2884`		`- clazz.getConstant(context, str) : clazz.getConstantAt(context, str);`
	`2884`	`+ clazz.constantTableFetch(str) : clazz.getConstantAt(context, str);`
`2885`	`2885`
`2886`	`2886`	`if (!flexibleSearch && cc == null) return null;`
`2887`	`2887`
	`2888`	`+ if (cc == null) throw classPathUndefinedException(path, undefinedExceptionClass, p);`
	`2889`	`+`
`2888`	`2890`	`if (!(cc instanceof RubyModule mod)) {`
`2889`	`2891`	`throw typeError(context, str(this, ids(this, path), " does not refer to class/module"));`
`2890`	`2892`	`}`
`@@ -2894,6 +2896,10 @@ public RubyModule getClassFromPath(final String path, RubyClass undefinedExcepti`
`2894`	`2896`	`return clazz;`
`2895`	`2897`	`}`
`2896`	`2898`
	`2899`	`+ private RaiseException classPathUndefinedException(String path, RubyClass undefinedExceptionClass, int p) {`
	`2900`	`+ return newRaiseException(undefinedExceptionClass, str(this, "undefined class/module ", ids(this, path.substring(0, p))));`
	`2901`	`+ }`
	`2902`	`+`
`2897`	`2903`	`private static boolean isJavaPackageOrJavaClassProxyType(final RubyModule type) {`
`2898`	`2904`	`return type instanceof JavaPackage \|\| ClassUtils.isJavaClassProxyType(type);`
`2899`	`2905`	`}`
Original file line number	Diff line number	Diff line change
`@@ -147,8 +147,7 @@ public static RubyModule getModuleFromPath(ThreadContext context, String path) {`
`147`	`147`
`148`	`148`	`public static RubyClass getClassFromPath(ThreadContext context, String path) {`
`149`	`149`	`Ruby runtime = context.runtime;`
`150`		`- final RubyModule value = runtime.getClassFromPath(path, runtime.getArgumentError(), false);`
`151`		`- if (value == null) throw argumentError(context, "undefined class/module " + path);`
	`150`	`+ final RubyModule value = runtime.getClassFromPath(path, runtime.getArgumentError(), true);`
`152`	`151`	`if ( ! value.isClass() ) throw argumentError(context, path + " does not refer class");`
`153`	`152`	`return (RubyClass) value;`
`154`	`153`	`}`