Change ScriptContexts to use needs instead of uses$. (#25036)

modules/lang-painless/src/main/java/org/elasticsearch/painless/GenericElasticsearchScript.java

@@ -34,6 +34,6 @@ public abstract class GenericElasticsearchScript {
     public abstract Object execute(
         Map<String, Object> params, double _score, Map<String, ScriptDocValues<?>> doc, Object _value, Map<?, ?> ctx);
 
-    public abstract boolean uses$_score();
+    public abstract boolean needs_score();
-    public abstract boolean uses$ctx();
+    public abstract boolean needsCtx();
 }

modules/lang-painless/src/main/java/org/elasticsearch/painless/PainlessScriptEngine.java

@@ -125,7 +125,7 @@ public final class PainlessScriptEngine extends AbstractComponent implements Scr
                 }
                 @Override
                 public boolean needsScores() {
-                    return painlessScript.uses$_score();
+                    return painlessScript.needs_score();
                 }
             };
             return context.factoryClazz.cast(factory);

modules/lang-painless/src/main/java/org/elasticsearch/painless/ScriptClassInfo.java

@@ -29,7 +29,7 @@ import java.util.Set;
 import java.util.function.Function;
 
 import static java.util.Collections.unmodifiableList;
-import static org.elasticsearch.painless.WriterConstants.USES_PARAMETER_METHOD_TYPE;
+import static org.elasticsearch.painless.WriterConstants.NEEDS_PARAMETER_METHOD_TYPE;
 
 /**
  * Information about the interface being implemented by the painless script.
@@ -40,7 +40,7 @@ public class ScriptClassInfo {
     private final org.objectweb.asm.commons.Method executeMethod;
     private final Definition.Type executeMethodReturnType;
     private final List<MethodArgument> executeArguments;
-    private final List<org.objectweb.asm.commons.Method> usesMethods;
+    private final List<org.objectweb.asm.commons.Method> needsMethods;
     private final List<org.objectweb.asm.commons.Method> getMethods;
     private final List<Definition.Type> getReturns;
 
@@ -49,7 +49,7 @@ public class ScriptClassInfo {
 
         // Find the main method and the uses$argName methods
         java.lang.reflect.Method executeMethod = null;
-        List<org.objectweb.asm.commons.Method> usesMethods = new ArrayList<>();
+        List<org.objectweb.asm.commons.Method> needsMethods = new ArrayList<>();
         List<org.objectweb.asm.commons.Method> getMethods = new ArrayList<>();
         List<Definition.Type> getReturns = new ArrayList<>();
         for (java.lang.reflect.Method m : baseClass.getMethods()) {
@@ -65,16 +65,8 @@ public class ScriptClassInfo {
                                     + "] has more than one.");
                 }
             }
-            if (m.getName().startsWith("uses$")) {
+            if (m.getName().startsWith("needs") && m.getReturnType().equals(boolean.class) && m.getParameterTypes().length == 0) {
-                if (false == m.getReturnType().equals(boolean.class)) {
+                needsMethods.add(new org.objectweb.asm.commons.Method(m.getName(), NEEDS_PARAMETER_METHOD_TYPE.toMethodDescriptorString()));
-                    throw new IllegalArgumentException("Painless can only implement uses$ methods that return boolean but ["
-                            + baseClass.getName() + "#" + m.getName() + "] returns [" + m.getReturnType().getName() + "].");
-                }
-                if (m.getParameterTypes().length > 0) {
-                    throw new IllegalArgumentException("Painless can only implement uses$ methods that do not take parameters but ["
-                            + baseClass.getName() + "#" + m.getName() + "] does.");
-                }
-                usesMethods.add(new org.objectweb.asm.commons.Method(m.getName(), USES_PARAMETER_METHOD_TYPE.toMethodDescriptorString()));
             }
             if (m.getName().startsWith("get") && m.getName().equals("getClass") == false && Modifier.isStatic(m.getModifiers()) == false) {
                 getReturns.add(
@@ -106,15 +98,7 @@ public class ScriptClassInfo {
             argumentNames.add(argumentNamesConstant[arg]);
         }
         this.executeArguments = unmodifiableList(arguments);
-
+        this.needsMethods = unmodifiableList(needsMethods);
-        // Validate that the uses$argName methods reference argument names
-        for (org.objectweb.asm.commons.Method usesMethod : usesMethods) {
-            if (false == argumentNames.contains(usesMethod.getName().substring("uses$".length()))) {
-                throw new IllegalArgumentException("Painless can only implement uses$ methods that match a parameter name but ["
-                        + baseClass.getName() + "#" + usesMethod.getName() + "] doesn't match any of " + argumentNames + ".");
-            }
-        }
-        this.usesMethods = unmodifiableList(usesMethods);
         this.getMethods = unmodifiableList(getMethods);
         this.getReturns = unmodifiableList(getReturns);
     }
@@ -152,8 +136,8 @@ public class ScriptClassInfo {
     /**
      * The {@code uses$varName} methods that must be implemented by Painless to complete implementing the interface.
      */
-    public List<org.objectweb.asm.commons.Method> getUsesMethods() {
+    public List<org.objectweb.asm.commons.Method> getNeedsMethods() {
-        return usesMethods;
+        return needsMethods;
     }
 
     /**

modules/lang-painless/src/main/java/org/elasticsearch/painless/ScriptImpl.java

@@ -81,8 +81,8 @@ final class ScriptImpl extends SearchScript {
             variables.putAll(leafLookup.asMap());
         }
 
-        scoreLookup = script.uses$_score() ? this::getScore : () -> 0.0;
+        scoreLookup = script.needs_score() ? this::getScore : () -> 0.0;
-        ctxLookup = script.uses$ctx() ? variables -> (Map<?, ?>) variables.get("ctx") : variables -> null;
+        ctxLookup = script.needsCtx() ? variables -> (Map<?, ?>) variables.get("ctx") : variables -> null;
     }
 
     @Override

modules/lang-painless/src/main/java/org/elasticsearch/painless/WriterConstants.java

@@ -84,7 +84,7 @@ public final class WriterConstants {
     public static final Type COLLECTIONS_TYPE = Type.getType(Collections.class);
     public static final Method EMPTY_MAP_METHOD = getAsmMethod(Map.class, "emptyMap");
 
-    public static final MethodType USES_PARAMETER_METHOD_TYPE = MethodType.methodType(boolean.class);
+    public static final MethodType NEEDS_PARAMETER_METHOD_TYPE = MethodType.methodType(boolean.class);
 
     public static final Type MAP_TYPE  = Type.getType(Map.class);
     public static final Method MAP_GET = getAsmMethod(Object.class, "get", Object.class);

modules/lang-painless/src/main/java/org/elasticsearch/painless/node/SSource.java

@@ -347,11 +347,14 @@ public final class SSource extends AStatement {
             clinit.endMethod();
         }
 
-        // Write any uses$varName methods for used variables
+        // Write any needsVarName methods for used variables
-        for (org.objectweb.asm.commons.Method usesMethod : scriptClassInfo.getUsesMethods()) {
+        for (org.objectweb.asm.commons.Method needsMethod : scriptClassInfo.getNeedsMethods()) {
-            MethodWriter ifaceMethod = new MethodWriter(Opcodes.ACC_PUBLIC, usesMethod, visitor, globals.getStatements(), settings);
+            String name = needsMethod.getName();
+            name = name.substring(5);
+            name = Character.toLowerCase(name.charAt(0)) + name.substring(1);
+            MethodWriter ifaceMethod = new MethodWriter(Opcodes.ACC_PUBLIC, needsMethod, visitor, globals.getStatements(), settings);
             ifaceMethod.visitCode();
-            ifaceMethod.push(reserved.getUsedVariables().contains(usesMethod.getName().substring("uses$".length())));
+            ifaceMethod.push(reserved.getUsedVariables().contains(name));
             ifaceMethod.returnValue();
             ifaceMethod.endMethod();
         }

modules/lang-painless/src/test/java/org/elasticsearch/painless/BaseClassTests.java

@@ -161,10 +161,10 @@ public class BaseClassTests extends ScriptTestCase {
     public abstract static class ManyArgs {
         public static final String[] PARAMETERS = new String[] {"a", "b", "c", "d"};
         public abstract Object execute(int a, int b, int c, int d);
-        public abstract boolean uses$a();
+        public abstract boolean needsA();
-        public abstract boolean uses$b();
+        public abstract boolean needsB();
-        public abstract boolean uses$c();
+        public abstract boolean needsC();
-        public abstract boolean uses$d();
+        public abstract boolean needsD();
     }
     public void testManyArgs() {
         Compiler compiler = new Compiler(ManyArgs.class, Definition.BUILTINS);
@@ -174,20 +174,20 @@ public class BaseClassTests extends ScriptTestCase {
 
         // While we're here we can verify that painless correctly finds used variables
         ManyArgs script = (ManyArgs)scriptEngine.compile(compiler, null, "a", emptyMap());
-        assertTrue(script.uses$a());
+        assertTrue(script.needsA());
-        assertFalse(script.uses$b());
+        assertFalse(script.needsB());
-        assertFalse(script.uses$c());
+        assertFalse(script.needsC());
-        assertFalse(script.uses$d());
+        assertFalse(script.needsD());
         script = (ManyArgs)scriptEngine.compile(compiler, null, "a + b + c", emptyMap());
-        assertTrue(script.uses$a());
+        assertTrue(script.needsA());
-        assertTrue(script.uses$b());
+        assertTrue(script.needsB());
-        assertTrue(script.uses$c());
+        assertTrue(script.needsC());
-        assertFalse(script.uses$d());
+        assertFalse(script.needsD());
         script = (ManyArgs)scriptEngine.compile(compiler, null, "a + b + c + d", emptyMap());
-        assertTrue(script.uses$a());
+        assertTrue(script.needsA());
-        assertTrue(script.uses$b());
+        assertTrue(script.needsB());
-        assertTrue(script.uses$c());
+        assertTrue(script.needsC());
-        assertTrue(script.uses$d());
+        assertTrue(script.needsD());
     }
 
     public abstract static class VarargTest {
@@ -473,43 +473,4 @@ public class BaseClassTests extends ScriptTestCase {
         assertEquals("Painless can only implement interfaces that have a single method named [execute] but ["
                 + TwoExecuteMethods.class.getName() + "] has more than one.", e.getMessage());
     }
-
-    public abstract static class BadUsesReturn {
-        public static final String[] PARAMETERS = new String[] {"foo"};
-        public abstract Object execute(String foo);
-        public abstract Object uses$foo();
-    }
-    public void testBadUsesReturn() {
-        Compiler compiler = new Compiler(BadUsesReturn.class, Definition.BUILTINS);
-        Exception e = expectScriptThrows(IllegalArgumentException.class, false, () ->
-            scriptEngine.compile(compiler, null, "null", emptyMap()));
-        assertEquals("Painless can only implement uses$ methods that return boolean but [" + BadUsesReturn.class.getName()
-                + "#uses$foo] returns [java.lang.Object].", e.getMessage());
-    }
-
-    public abstract static class BadUsesParameter {
-        public static final String[] PARAMETERS = new String[] {"foo", "bar"};
-        public abstract Object execute(String foo, String bar);
-        public abstract boolean uses$bar(boolean foo);
-    }
-    public void testBadUsesParameter() {
-        Compiler compiler = new Compiler(BadUsesParameter.class, Definition.BUILTINS);
-        Exception e = expectScriptThrows(IllegalArgumentException.class, false, () ->
-            scriptEngine.compile(compiler, null, "null", emptyMap()));
-        assertEquals("Painless can only implement uses$ methods that do not take parameters but [" + BadUsesParameter.class.getName()
-                + "#uses$bar] does.", e.getMessage());
-    }
-
-    public abstract static class BadUsesName {
-        public static final String[] PARAMETERS = new String[] {"foo", "bar"};
-        public abstract Object execute(String foo, String bar);
-        public abstract boolean uses$baz();
-    }
-    public void testBadUsesName() {
-        Compiler compiler = new Compiler(BadUsesName.class, Definition.BUILTINS);
-        Exception e = expectScriptThrows(IllegalArgumentException.class, false, () ->
-            scriptEngine.compile(compiler, null, "null", emptyMap()));
-        assertEquals("Painless can only implement uses$ methods that match a parameter name but [" + BadUsesName.class.getName()
-                + "#uses$baz] doesn't match any of [foo, bar].", e.getMessage());
-    }
 }