Reload method types

Fixes beanshell#707 generated return types are reloaded
Fixes beanshell#708 generated parameter types are reloaded
Related to beanshell#659 improved scripted object detection, fixes is method bug

src/conf/version.properties

@@ -14,6 +14,6 @@
 #
 #Project version number - populated by maven
 #Project build number - incremented by beanshell-maven-plugin
-#Fri Jan 13 08:11:40 SAST 2023
-build=3927
+#Fri Jan 13 17:12:25 SAST 2023
+build=4018
 release=${project.version}

src/main/java/bsh/BSHFormalParameters.java

@@ -35,7 +35,7 @@ class BSHFormalParameters extends SimpleNode
         For loose type parameters the paramTypes are null.
     */
     // unsafe caching of types
-    Class [] paramTypes;
+    Class<?> [] paramTypes;
     int numArgs;
     String [] typeDescriptors;
     boolean isVarArgs;
@@ -105,12 +105,12 @@ public Object eval( CallStack callstack, Interpreter interpreter )
             return paramTypes;
 
         insureParsed();
-        Class [] paramTypes = new Class[numArgs];
+        Class<?> [] paramTypes = new Class[numArgs];
 
         for(int i=0; i<numArgs; i++)
         {
             BSHFormalParameter param = (BSHFormalParameter)jjtGetChild(i);
-            paramTypes[i] = (Class)param.eval( callstack, interpreter );
+            paramTypes[i] = (Class<?>)param.eval( callstack, interpreter );
         }
 
         this.paramTypes = paramTypes;

src/main/java/bsh/BSHMethodDeclaration.java

@@ -44,10 +44,12 @@ class BSHMethodDeclaration extends SimpleNode
     public Modifiers modifiers = new Modifiers(Modifiers.METHOD);
 
     // Unsafe caching of type here.
-    Class returnType;  // null (none), Void.TYPE, or a Class
+    Class<?> returnType;  // null (none), Void.TYPE, or a Class
     int numThrows = 0;
     boolean isVarArgs;
-    boolean isThisContext;
+    private boolean isScriptedObject;
+    private transient CallStack callstack;
+    private transient Interpreter interpreter;
 
     BSHMethodDeclaration(int id) { super(id); }
 
@@ -81,18 +83,37 @@ synchronized void insureNodesParsed()
             if (crnt instanceof BSHReturnStatement)
                 while (crnt.hasNext())
                     if ((crnt = crnt.next()) instanceof BSHAmbiguousName)
-                        isThisContext = "this".equals(((BSHAmbiguousName)crnt).text);
+                        isScriptedObject = ((BSHAmbiguousName)crnt).text.startsWith("this");
         }
 
         paramsNode.insureParsed();
         isVarArgs = paramsNode.isVarArgs;
     }
 
+    /** Used to reevaluate the parameter types on class loader changed.
+     * @return reevaluated types */
+    Class<?>[] reevalParamTypes() {
+        try {
+            paramsNode.paramTypes = null;
+            paramsNode.eval(callstack, interpreter);
+            return paramsNode.paramTypes;
+        } catch (EvalError e) { return paramsNode.paramTypes; }
+    }
+
+    /** Used to reevaluate the return type on class loader changed.
+     * @return a reevaluated type */
+    Class<?> reevalReturnType() {
+        try {
+            returnType = evalReturnType(callstack, interpreter);
+            return returnType;
+        } catch (EvalError e) { return returnType; }
+    }
+
     /**
         Evaluate the return type node.
         @return the type or null indicating loosely typed return
     */
-    Class evalReturnType( CallStack callstack, Interpreter interpreter )
+    Class<?> evalReturnType( CallStack callstack, Interpreter interpreter )
         throws EvalError
     {
         insureNodesParsed();
@@ -125,6 +146,8 @@ BSHReturnType getReturnTypeNode() {
     public Object eval( CallStack callstack, Interpreter interpreter )
         throws EvalError
     {
+        this.interpreter = interpreter;
+        this.callstack = callstack;
         returnType = evalReturnType( callstack, interpreter );
         evalNodes( callstack, interpreter );
 
@@ -135,10 +158,9 @@ public Object eval( CallStack callstack, Interpreter interpreter )
 // need a way to update eval without re-installing...
 // so that we can re-eval params, etc. when classloader changes
 // look into this
-
         NameSpace namespace = callstack.top();
-        namespace.isMethod = isThisContext;
-        BshMethod bshMethod = new BshMethod( this, namespace, modifiers );
+        BshMethod bshMethod = new BshMethod( this, namespace, modifiers, isScriptedObject );
+        interpreter.getClassManager().addListener(bshMethod);
 
         namespace.setMethod( bshMethod );
 

src/main/java/bsh/BSHReturnType.java

@@ -48,7 +48,7 @@ public String getTypeDescriptor(
                 callstack, interpreter, defaultPackage );
     }
 
-    public Class evalReturnType(
+    public Class<?> evalReturnType(
         CallStack callstack, Interpreter interpreter ) throws EvalError
     {
         if ( isVoid )

src/main/java/bsh/BshMethod.java

@@ -26,6 +26,7 @@
 
 package bsh;
 
+import java.util.Arrays;
 import java.util.stream.IntStream;
 import java.io.Serializable;
 import java.lang.reflect.Array;
@@ -46,7 +47,7 @@
     Note: this method incorrectly caches the method structure.  It needs to
     be cleared when the classloader changes.
 */
-public class BshMethod implements Serializable, Cloneable {
+public class BshMethod implements Serializable, Cloneable, BshClassManager.Listener {
 
     private static final long serialVersionUID = 1L;
 
@@ -71,22 +72,25 @@ public class BshMethod implements Serializable, Cloneable {
     private Modifiers [] paramModifiers;
 
     // Scripted method body
-    BSHBlock methodBody;
-
+    protected BSHBlock methodBody;
+    private BSHMethodDeclaration methodNode;
     // Java Method, for a BshObject that delegates to a real Java method
     private Invocable javaMethod;
     private Object javaObject;
     protected boolean isVarArgs;
+    boolean isScriptedObject = false;
 
     // End method components
 
     BshMethod(
         BSHMethodDeclaration method,
-        NameSpace declaringNameSpace, Modifiers modifiers )
+        NameSpace declaringNameSpace, Modifiers modifiers, boolean isScriptedObject )
     {
         this( method.name, method.returnType, method.paramsNode.getParamNames(),
             method.paramsNode.paramTypes, method.paramsNode.getParamModifiers(),
             method.blockNode, declaringNameSpace, modifiers, method.isVarArgs );
+        this.isScriptedObject = isScriptedObject;
+        this.methodNode = method;
     }
 
     BshMethod(
@@ -142,8 +146,10 @@ loosely typed (untyped) arguments will be represented by null argument
         This is broken.
     */
     public Class<?> [] getParameterTypes() {
-        if (null == this.javaMethod)
+        if (null == this.javaMethod) {
+            reloadTypes();
             return cparamTypes;
+        }
         return this.javaMethod.getParameterTypes();
     }
 
@@ -176,8 +182,10 @@ public int getParameterCount() {
         This is broken.
     */
     public Class<?> getReturnType() {
-        if (null == this.javaMethod)
+        if (null == this.javaMethod) {
+            reloadTypes();
             return creturnType;
+        }
         return this.javaMethod.getReturnType();
     }
 
@@ -263,6 +271,7 @@ Object invoke(
         throws EvalError
     {
         Interpreter.debug("Bsh method invoke: ", this.name, " overrideNameSpace: ", overrideNameSpace);
+        reloadTypes();
         if ( argValues != null )
             for (int i=0; i<argValues.length; i++)
                 if ( argValues[i] == null )
@@ -559,4 +568,22 @@ public int hashCode() {
             h += 3 + (cparamType == null ? 0 : cparamType.hashCode());
         return h + getParameterCount();
     }
+
+    private boolean reload = false;
+
+    /** Reload types if reload is true */
+    private void reloadTypes() {
+        if (!reload) return;
+        reload = false;
+        creturnType = methodNode.reevalReturnType();
+        cparamTypes = methodNode.reevalParamTypes();
+    }
+
+    /** {@inheritDoc} */
+    @Override
+    public void classLoaderChanged() {
+        reload = Reflect.isGeneratedClass(creturnType)
+            || Arrays.asList(cparamTypes).stream()
+                .anyMatch(Reflect::isGeneratedClass);
+    }
 }

src/main/java/bsh/Name.java

@@ -859,7 +859,7 @@ public Object invokeMethod(
         to load it as a resource from the imported command path (e.g.
         /bsh/commands)
     */
-    private static final Pattern noOverride = Pattern.compile("eval|extend|source|exec|assert|isEvalError");
+    private static final Pattern noOverride = Pattern.compile("eval|assert");
     private Object invokeLocalMethod(
         Interpreter interpreter, Object[] args, CallStack callstack, Node callerInfo)
         throws EvalError
@@ -886,6 +886,7 @@ private Object invokeLocalMethod(
             // whether to use callstack.top or new child of declared name space
             // enables late binding for closures and namespace chaining #676
             boolean overrideChild = !namespace.isMethod
+                    && !meth.isScriptedObject
                     && namespace.isChildOf(meth.declaringNameSpace)
                     && !namespace.getParent().isClass
                     && !noOverride.matcher(meth.getName()).matches();

src/test/java/bsh/InterpreterTest.java

@@ -26,6 +26,8 @@
 import java.io.Reader;
 import java.io.StringReader;
 import java.lang.ref.WeakReference;
+import java.util.stream.IntStream;
+
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertNull;
@@ -328,10 +330,17 @@ public void overwrite_get_prompt_exception() throws Exception {
     public void reset_interpreter() throws Exception {
         final Interpreter bsh = new Interpreter();
         assertEquals("test123", bsh.eval("'test' + (100 + 20 + 3)"));
+        for (String cls:IntStream.range(65, 65+10) // A - Z
+                .boxed().map(n->String.valueOf((char) n.intValue()))
+                .toArray(String[]::new)) {
+            assertEquals(cls, bsh.eval("class "+cls+" { static "+cls+" a(){ return new "+cls+"(); }"
+            +" b(){} static "+cls+" c=a(); d=2; } return new "+cls+"().getClass().getName();"));
+        }
         long b4 = Runtime.getRuntime().freeMemory();
         bsh.reset();
         TestUtil.cleanUp();
         assertThat(b4, lessThan(Runtime.getRuntime().freeMemory()));
+        System.out.println(b4+ " "+Runtime.getRuntime().freeMemory() );
     }
 
 }

src/test/resources/test-scripts/classreload3.bsh

@@ -0,0 +1,57 @@
+#!/bin/java bsh.Interpreter
+
+source("TestHarness.bsh");
+source("Assert.bsh");
+
+// method return and parameter types reload
+
+// return type #707
+class A {}
+A returnA(a) {
+    return a;
+}
+a = new A();
+b4 = a.getClass();
+assertSame("Returns the same typed instance untyped parameter", a, returnA(a));
+class A {}
+a = new A();
+assertSame("Returns the same typed instance untyped parameter reloaded", a, returnA(a));
+assertNotSame("A and A before is not the same", b4, a.getClass());
+
+// parameter type #708
+paramA(A a) {
+    return a;
+}
+a = new A();
+b4 = a.getClass();
+assertSame("Returns the same untyped instance typed parameter", a, paramA(a));
+class A {}
+a = new A();
+assertSame("Returns the same untyped instance typed parameter reloaded", a, paramA(a));
+assertNotSame("A and A before is not the same", b4, a.getClass());
+
+// return and parameter type
+A returnParamA(A a) {
+    return a;
+}
+a = new A();
+b4 = a.getClass();
+assertSame("Returns the same typed instance typed parameter", a, returnParamA(a));
+class A {}
+a = new A();
+assertSame("Returns the same typed instance typed parameter reloaded", a, returnParamA(a));
+assertNotSame("A and A before is not the same", b4, a.getClass());
+
+// non generated types
+Object objectA(final Object a) {
+    return a;
+}
+a = new A();
+b4 = a.getClass();
+assertSame("Returns the same typed instance untyped parameter", a, objectA(a));
+class A {}
+a = new A();
+assertSame("Returns the same typed instance untyped parameter reloaded", a, objectA(a));
+assertNotSame("A and A before is not the same", b4, a.getClass());
+
+complete();

src/test/resources/test-scripts/modifiers.bsh

@@ -42,8 +42,10 @@ assert( isEvalError("public/private/protected cannot be used in combination.", "
 
 foo(final int bar) {
     assert( Reflect.getVariable(this.namespace, 'bar').getModifiers().hasModifier('final'));
+    assert( isEvalError("Cannot re-assign final variable bar", "bar = 2;"));
 }
 foo(1);
+assert( Reflect.getMethod(this.namespace, 'foo', {Integer.TYPE}).getParameterModifiers()[0].hasModifier('final'));
 
 assert( isEvalError("Constructor cannot be declared 'static'", "class A { public static A() {} }"));
 assert( isEvalError("Constructor cannot be declared 'synchronized'", "class B { synchronized B() {} }"));

src/test/resources/test-scripts/scripted_object2.bsh

@@ -0,0 +1,46 @@
+#!/bin/java bsh.Interpreter
+
+source("TestHarness.bsh");
+source("Assert.bsh");
+
+/*
+ * closure called from block with additional method does not break
+ * Related to #659
+ */
+
+Object closure1() {
+   list = new ArrayList();
+   this.list.add(1);
+
+   return this;
+}
+
+double doesntMatter() {
+    return 1;
+}
+
+try {
+    t = closure1();
+    assertEquals("list size is equal te 1", 1, t.list.size());
+} catch (Exception ex) {
+    assert(false);
+}
+
+/*
+ * sudo scripted object returning from this reported #659
+ */
+closure2() {
+   list = new ArrayList();
+   this.list.add(1);
+
+   return this.list;
+}
+
+assertEquals("has one element from global", 1, closure2().size());
+{ assertEquals("has one element from block", 1, closure2().size()); }
+{
+   t = closure2();
+   assertEquals("has one element from block assigned", 1, t.size());
+}
+
+complete();