From 46bf087a40786a80b12a4ea2729ea26e7106608a Mon Sep 17 00:00:00 2001 From: Jack Conradson Date: Wed, 5 Sep 2018 12:41:09 -0700 Subject: [PATCH 1/3] Painless: Add Static Methods Shortcut --- .../elasticsearch/painless/spi/Whitelist.java | 9 +- .../painless/spi/WhitelistLoader.java | 18 +- .../elasticsearch/painless/FeatureTest.java | 26 +-- .../painless/lookup/PainlessLookup.java | 15 ++ .../lookup/PainlessLookupBuilder.java | 164 +++++++++++++++++- .../painless/node/ECallLocal.java | 51 ++++-- .../painless/spi/org.elasticsearch.txt | 1 + .../elasticsearch/painless/BasicAPITests.java | 4 + .../painless/ScriptTestCase.java | 8 +- 9 files changed, 260 insertions(+), 36 deletions(-) diff --git a/modules/lang-painless/spi/src/main/java/org/elasticsearch/painless/spi/Whitelist.java b/modules/lang-painless/spi/src/main/java/org/elasticsearch/painless/spi/Whitelist.java index 7acbff6cb0b93..428935b7e895b 100644 --- a/modules/lang-painless/spi/src/main/java/org/elasticsearch/painless/spi/Whitelist.java +++ b/modules/lang-painless/spi/src/main/java/org/elasticsearch/painless/spi/Whitelist.java @@ -61,12 +61,19 @@ public final class Whitelist { /** The {@link List} of all the whitelisted Painless classes. */ public final List whitelistClasses; + /** The {@link List} of all the whitelisted static Painless methods. */ + public final List whitelistStatics; + + /** The {@link List} of all the whitelisted Painless bindings. */ public final List whitelistBindings; /** Standard constructor. All values must be not {@code null}. */ - public Whitelist(ClassLoader classLoader, List whitelistClasses, List whitelistBindings) { + public Whitelist(ClassLoader classLoader, List whitelistClasses, + List whitelistStatics, List whitelistBindings) { + this.classLoader = Objects.requireNonNull(classLoader); this.whitelistClasses = Collections.unmodifiableList(Objects.requireNonNull(whitelistClasses)); + this.whitelistStatics = Collections.unmodifiableList(Objects.requireNonNull(whitelistStatics)); this.whitelistBindings = Collections.unmodifiableList(Objects.requireNonNull(whitelistBindings)); } } diff --git a/modules/lang-painless/spi/src/main/java/org/elasticsearch/painless/spi/WhitelistLoader.java b/modules/lang-painless/spi/src/main/java/org/elasticsearch/painless/spi/WhitelistLoader.java index 0279c82f1b67b..1c9c9134b38db 100644 --- a/modules/lang-painless/spi/src/main/java/org/elasticsearch/painless/spi/WhitelistLoader.java +++ b/modules/lang-painless/spi/src/main/java/org/elasticsearch/painless/spi/WhitelistLoader.java @@ -133,6 +133,7 @@ public final class WhitelistLoader { */ public static Whitelist loadFromResourceFiles(Class resource, String... filepaths) { List whitelistClasses = new ArrayList<>(); + List whitelistStatics = new ArrayList<>(); List whitelistBindings = new ArrayList<>(); // Execute a single pass through the whitelist text files. This will gather all the @@ -230,7 +231,7 @@ public static Whitelist loadFromResourceFiles(Class resource, String... filep parseType = null; // Handle static definition types. - // Expects the following format: ID ID '(' ( ID ( ',' ID )* )? ')' 'bound_to' ID '\n' + // Expects the following format: ID ID '(' ( ID ( ',' ID )* )? ')' ( 'from' | 'bound_to' ) ID '\n' } else if ("static".equals(parseType)) { // Mark the origin of this parsable object. String origin = "[" + filepath + "]:[" + number + "]"; @@ -286,15 +287,18 @@ public static Whitelist loadFromResourceFiles(Class resource, String... filep throw new IllegalArgumentException("invalid static definition: unexpected format [" + line + "]"); } - // Check the static type is valid. - if ("bound_to".equals(staticType) == false) { + // Add a static method or binding depending on the static type. + if ("from".equals(staticType)) { + whitelistStatics.add(new WhitelistMethod(origin, targetJavaClassName, + methodName, returnCanonicalTypeName, Arrays.asList(canonicalTypeNameParameters))); + } else if ("bound_to".equals(staticType)) { + whitelistBindings.add(new WhitelistBinding(origin, targetJavaClassName, + methodName, returnCanonicalTypeName, Arrays.asList(canonicalTypeNameParameters))); + } else { throw new IllegalArgumentException( "invalid static definition: unexpected static type [" + staticType + "] [" + line + "]"); } - whitelistBindings.add(new WhitelistBinding(origin, targetJavaClassName, - methodName, returnCanonicalTypeName, Arrays.asList(canonicalTypeNameParameters))); - // Handle class definition types. } else if ("class".equals(parseType)) { // Mark the origin of this parsable object. @@ -393,7 +397,7 @@ public static Whitelist loadFromResourceFiles(Class resource, String... filep ClassLoader loader = AccessController.doPrivileged((PrivilegedAction)resource::getClassLoader); - return new Whitelist(loader, whitelistClasses, whitelistBindings); + return new Whitelist(loader, whitelistClasses, whitelistStatics, whitelistBindings); } private WhitelistLoader() {} diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/FeatureTest.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/FeatureTest.java index 1e94c19f6d90e..28cbb4aee19a6 100644 --- a/modules/lang-painless/src/main/java/org/elasticsearch/painless/FeatureTest.java +++ b/modules/lang-painless/src/main/java/org/elasticsearch/painless/FeatureTest.java @@ -24,6 +24,21 @@ /** Currently just a dummy class for testing a few features not yet exposed by whitelist! */ public class FeatureTest { + /** static method that returns true */ + public static boolean overloadedStatic() { + return true; + } + + /** static method that returns what you ask it */ + public static boolean overloadedStatic(boolean whatToReturn) { + return whatToReturn; + } + + /** static method only whitelisted as a static */ + public static float staticAddFloatsTest(float x, float y) { + return x + y; + } + private int x; private int y; public int z; @@ -58,21 +73,12 @@ public void setY(int y) { this.y = y; } - /** static method that returns true */ - public static boolean overloadedStatic() { - return true; - } - - /** static method that returns what you ask it */ - public static boolean overloadedStatic(boolean whatToReturn) { - return whatToReturn; - } - /** method taking two functions! */ public Object twoFunctionsOfX(Function f, Function g) { return f.apply(g.apply(x)); } + /** method to take in a list */ public void listInput(List list) { } diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/lookup/PainlessLookup.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/lookup/PainlessLookup.java index 2d6ed3e361dc3..27636d8220a33 100644 --- a/modules/lang-painless/src/main/java/org/elasticsearch/painless/lookup/PainlessLookup.java +++ b/modules/lang-painless/src/main/java/org/elasticsearch/painless/lookup/PainlessLookup.java @@ -37,16 +37,23 @@ public final class PainlessLookup { private final Map> canonicalClassNamesToClasses; private final Map, PainlessClass> classesToPainlessClasses; + private final Map painlessMethodKeysToPainlessStatics; private final Map painlessMethodKeysToPainlessBindings; PainlessLookup(Map> canonicalClassNamesToClasses, Map, PainlessClass> classesToPainlessClasses, + Map painlessMethodKeysToPainlessStatics, Map painlessMethodKeysToPainlessBindings) { + Objects.requireNonNull(canonicalClassNamesToClasses); Objects.requireNonNull(classesToPainlessClasses); + Objects.requireNonNull(painlessMethodKeysToPainlessStatics); + Objects.requireNonNull(painlessMethodKeysToPainlessBindings); + this.canonicalClassNamesToClasses = Collections.unmodifiableMap(canonicalClassNamesToClasses); this.classesToPainlessClasses = Collections.unmodifiableMap(classesToPainlessClasses); + this.painlessMethodKeysToPainlessStatics = Collections.unmodifiableMap(painlessMethodKeysToPainlessStatics); this.painlessMethodKeysToPainlessBindings = Collections.unmodifiableMap(painlessMethodKeysToPainlessBindings); } @@ -167,6 +174,14 @@ public PainlessField lookupPainlessField(Class targetClass, boolean isStatic, return painlessField; } + public PainlessMethod lookupPainlessStatic(String methodName, int arity) { + Objects.requireNonNull(methodName); + + String painlessMethodKey = buildPainlessMethodKey(methodName, arity); + + return painlessMethodKeysToPainlessStatics.get(painlessMethodKey); + } + public PainlessBinding lookupPainlessBinding(String methodName, int arity) { Objects.requireNonNull(methodName); diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/lookup/PainlessLookupBuilder.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/lookup/PainlessLookupBuilder.java index a64814f866113..6c94802121d95 100644 --- a/modules/lang-painless/src/main/java/org/elasticsearch/painless/lookup/PainlessLookupBuilder.java +++ b/modules/lang-painless/src/main/java/org/elasticsearch/painless/lookup/PainlessLookupBuilder.java @@ -243,6 +243,14 @@ public static PainlessLookup buildFromWhitelists(List whitelists) { } } + for (WhitelistMethod whitelistStatic : whitelist.whitelistStatics) { + origin = whitelistStatic.origin; + painlessLookupBuilder.addPainlessStatic( + whitelist.classLoader, whitelistStatic.augmentedCanonicalClassName, + whitelistStatic.methodName, whitelistStatic.returnCanonicalTypeName, + whitelistStatic.canonicalTypeNameParameters); + } + for (WhitelistBinding whitelistBinding : whitelist.whitelistBindings) { origin = whitelistBinding.origin; painlessLookupBuilder.addPainlessBinding( @@ -261,12 +269,14 @@ public static PainlessLookup buildFromWhitelists(List whitelists) { private final Map> canonicalClassNamesToClasses; private final Map, PainlessClassBuilder> classesToPainlessClassBuilders; + private final Map painlessMethodKeysToPainlessStatics; private final Map painlessMethodKeysToPainlessBindings; public PainlessLookupBuilder() { canonicalClassNamesToClasses = new HashMap<>(); classesToPainlessClassBuilders = new HashMap<>(); + painlessMethodKeysToPainlessStatics = new HashMap<>(); painlessMethodKeysToPainlessBindings = new HashMap<>(); } @@ -513,8 +523,9 @@ public void addPainlessMethod(ClassLoader classLoader, String targetCanonicalCla addPainlessMethod(targetClass, augmentedClass, methodName, returnType, typeParameters); } - public void addPainlessMethod(Class targetClass, Class augmentedClass, String methodName, - Class returnType, List> typeParameters) { + public void addPainlessMethod(Class targetClass, Class augmentedClass, + String methodName, Class returnType, List> typeParameters) { + Objects.requireNonNull(targetClass); Objects.requireNonNull(methodName); Objects.requireNonNull(returnType); @@ -788,6 +799,147 @@ public void addPainlessField(Class targetClass, String fieldName, Class ty } } + public void addPainlessStatic(ClassLoader classLoader, String targetCanonicalClassName, + String methodName, String returnCanonicalTypeName, List canonicalTypeNameParameters) { + + Objects.requireNonNull(classLoader); + Objects.requireNonNull(targetCanonicalClassName); + Objects.requireNonNull(methodName); + Objects.requireNonNull(returnCanonicalTypeName); + Objects.requireNonNull(canonicalTypeNameParameters); + + Class targetClass = canonicalClassNamesToClasses.get(targetCanonicalClassName); + + if (targetClass == null) { + throw new IllegalArgumentException("target class [" + targetCanonicalClassName + "] not found for static " + + "[[" + targetCanonicalClassName + "], [" + methodName + "], " + canonicalTypeNameParameters + "]"); + } + + List> typeParameters = new ArrayList<>(canonicalTypeNameParameters.size()); + + for (String canonicalTypeNameParameter : canonicalTypeNameParameters) { + Class typeParameter = canonicalTypeNameToType(canonicalTypeNameParameter); + + if (typeParameter == null) { + throw new IllegalArgumentException("type parameter [" + canonicalTypeNameParameter + "] not found for static " + + "[[" + targetCanonicalClassName + "], [" + methodName + "], " + canonicalTypeNameParameters + "]"); + } + + typeParameters.add(typeParameter); + } + + Class returnType = canonicalTypeNameToType(returnCanonicalTypeName); + + if (returnType == null) { + throw new IllegalArgumentException("return type [" + returnCanonicalTypeName + "] not found for static " + + "[[" + targetCanonicalClassName + "], [" + methodName + "], " + canonicalTypeNameParameters + "]"); + } + + addPainlessStatic(targetClass, methodName, returnType, typeParameters); + } + + public void addPainlessStatic(Class targetClass, String methodName, Class returnType, List> typeParameters) { + Objects.requireNonNull(targetClass); + Objects.requireNonNull(methodName); + Objects.requireNonNull(returnType); + Objects.requireNonNull(typeParameters); + + if (targetClass == def.class) { + throw new IllegalArgumentException("cannot add static from reserved class [" + DEF_CLASS_NAME + "]"); + } + + String targetCanonicalClassName = typeToCanonicalTypeName(targetClass); + + if (METHOD_NAME_PATTERN.matcher(methodName).matches() == false) { + throw new IllegalArgumentException( + "invalid method name [" + methodName + "] for static target class [" + targetCanonicalClassName + "]."); + } + + PainlessClassBuilder painlessClassBuilder = classesToPainlessClassBuilders.get(targetClass); + + if (painlessClassBuilder == null) { + throw new IllegalArgumentException("target class [" + targetCanonicalClassName + "] not found for static " + + "[[" + targetCanonicalClassName + "], [" + methodName + "], " + typesToCanonicalTypeNames(typeParameters) + "]"); + } + + int typeParametersSize = typeParameters.size(); + List> javaTypeParameters = new ArrayList<>(typeParametersSize); + + for (Class typeParameter : typeParameters) { + if (isValidType(typeParameter) == false) { + throw new IllegalArgumentException("type parameter [" + typeToCanonicalTypeName(typeParameter) + "] " + + "not found for static [[" + targetCanonicalClassName + "], [" + methodName + "], " + + typesToCanonicalTypeNames(typeParameters) + "]"); + } + + javaTypeParameters.add(typeToJavaType(typeParameter)); + } + + if (isValidType(returnType) == false) { + throw new IllegalArgumentException("return type [" + typeToCanonicalTypeName(returnType) + "] not found for static " + + "[[" + targetCanonicalClassName + "], [" + methodName + "], " + typesToCanonicalTypeNames(typeParameters) + "]"); + } + + Method javaMethod; + + try { + javaMethod = targetClass.getMethod(methodName, javaTypeParameters.toArray(new Class[typeParametersSize])); + } catch (NoSuchMethodException nsme) { + throw new IllegalArgumentException("static method reflection object [[" + targetCanonicalClassName + "], " + + "[" + methodName + "], " + typesToCanonicalTypeNames(typeParameters) + "] not found", nsme); + } + + if (javaMethod.getReturnType() != typeToJavaType(returnType)) { + throw new IllegalArgumentException("return type [" + typeToCanonicalTypeName(javaMethod.getReturnType()) + "] " + + "does not match the specified returned type [" + typeToCanonicalTypeName(returnType) + "] " + + "for static [[" + targetClass.getCanonicalName() + "], [" + methodName + "], " + + typesToCanonicalTypeNames(typeParameters) + "]"); + } + + if (Modifier.isStatic(javaMethod.getModifiers()) == false) { + throw new IllegalArgumentException("return type [" + typeToCanonicalTypeName(javaMethod.getReturnType()) + "] " + + "does not match the specified returned type [" + typeToCanonicalTypeName(returnType) + "] " + + "for static [[" + targetClass.getCanonicalName() + "], [" + methodName + "], " + + typesToCanonicalTypeNames(typeParameters) + "]"); + } + + String painlessMethodKey = buildPainlessMethodKey(methodName, typeParametersSize); + + if (painlessMethodKeysToPainlessBindings.containsKey(painlessMethodKey)) { + throw new IllegalArgumentException("static and binding cannot have the same name [" + methodName + "]"); + } + + PainlessMethod painlessStatic = painlessMethodKeysToPainlessStatics.get(painlessMethodKey); + + if (painlessStatic == null) { + MethodHandle methodHandle; + + try { + methodHandle = MethodHandles.publicLookup().in(targetClass).unreflect(javaMethod); + } catch (IllegalAccessException iae) { + throw new IllegalArgumentException("static method handle [[" + targetClass.getCanonicalName() + "], " + + "[" + methodName + "], " + typesToCanonicalTypeNames(typeParameters) + "] not found", iae); + } + + MethodType methodType = methodHandle.type(); + + painlessStatic = painlessMethodCache.computeIfAbsent( + new PainlessMethodCacheKey(targetClass, methodName, returnType, typeParameters), + key -> new PainlessMethod(javaMethod, targetClass, returnType, typeParameters, methodHandle, methodType)); + + painlessMethodKeysToPainlessStatics.put(painlessMethodKey, painlessStatic); + } else if (painlessStatic.returnType == returnType && painlessStatic.typeParameters.equals(typeParameters) == false) { + throw new IllegalArgumentException("cannot have statics " + + "[[" + targetCanonicalClassName + "], [" + methodName + "], " + + "[" + typeToCanonicalTypeName(returnType) + "], " + + typesToCanonicalTypeNames(typeParameters) + "] and " + + "[[" + targetCanonicalClassName + "], [" + methodName + "], " + + "[" + typeToCanonicalTypeName(painlessStatic.returnType) + "], " + + typesToCanonicalTypeNames(painlessStatic.typeParameters) + "] " + + "with the same arity and different return type or type parameters"); + } + } + public void addPainlessBinding(ClassLoader classLoader, String targetJavaClassName, String methodName, String returnCanonicalTypeName, List canonicalTypeNameParameters) { @@ -937,6 +1089,11 @@ public void addPainlessBinding(Class targetClass, String methodName, Class } String painlessMethodKey = buildPainlessMethodKey(methodName, constructorTypeParametersSize + methodTypeParametersSize); + + if (painlessMethodKeysToPainlessStatics.containsKey(painlessMethodKey)) { + throw new IllegalArgumentException("binding and static cannot have the same name [" + methodName + "]"); + } + PainlessBinding painlessBinding = painlessMethodKeysToPainlessBindings.get(painlessMethodKey); if (painlessBinding == null) { @@ -976,7 +1133,8 @@ public PainlessLookup build() { classesToPainlessClasses.put(painlessClassBuilderEntry.getKey(), painlessClassBuilderEntry.getValue().build()); } - return new PainlessLookup(canonicalClassNamesToClasses, classesToPainlessClasses, painlessMethodKeysToPainlessBindings); + return new PainlessLookup(canonicalClassNamesToClasses, classesToPainlessClasses, + painlessMethodKeysToPainlessStatics, painlessMethodKeysToPainlessBindings); } private void copyPainlessClassMembers() { diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/ECallLocal.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/ECallLocal.java index 8ae6ad9723da4..38f318bbdf394 100644 --- a/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/ECallLocal.java +++ b/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/ECallLocal.java @@ -25,6 +25,7 @@ import org.elasticsearch.painless.Location; import org.elasticsearch.painless.MethodWriter; import org.elasticsearch.painless.lookup.PainlessBinding; +import org.elasticsearch.painless.lookup.PainlessMethod; import org.objectweb.asm.Label; import org.objectweb.asm.Type; import org.objectweb.asm.commons.Method; @@ -45,6 +46,7 @@ public final class ECallLocal extends AExpression { private final List arguments; private LocalMethod method = null; + private PainlessMethod statik = null; private PainlessBinding binding = null; public ECallLocal(Location location, String name, List arguments) { @@ -65,16 +67,33 @@ void extractVariables(Set variables) { void analyze(Locals locals) { method = locals.getMethod(name, arguments.size()); - if (method == null) { - binding = locals.getPainlessLookup().lookupPainlessBinding(name, arguments.size()); + statik = locals.getPainlessLookup().lookupPainlessStatic(name, arguments.size()); + + if (statik == null) { + binding = locals.getPainlessLookup().lookupPainlessBinding(name, arguments.size()); - if (binding == null) { - throw createError(new IllegalArgumentException("Unknown call [" + name + "] with [" + arguments.size() + "] arguments.")); + if (binding == null) { + throw createError( + new IllegalArgumentException("Unknown call [" + name + "] with [" + arguments.size() + "] arguments.")); + } } } - List> typeParameters = new ArrayList<>(method == null ? binding.typeParameters : method.typeParameters); + List> typeParameters; + + if (method != null) { + typeParameters = new ArrayList<>(method.typeParameters); + actual = method.returnType; + } else if (statik != null) { + typeParameters = new ArrayList<>(statik.typeParameters); + actual = statik.returnType; + } else if (binding != null) { + typeParameters = new ArrayList<>(binding.typeParameters); + actual = binding.returnType; + } else { + throw new IllegalStateException("Illegal tree structure."); + } for (int argument = 0; argument < arguments.size(); ++argument) { AExpression expression = arguments.get(argument); @@ -86,14 +105,26 @@ void analyze(Locals locals) { } statement = true; - actual = method == null ? binding.returnType : method.returnType; } @Override void write(MethodWriter writer, Globals globals) { writer.writeDebugInfo(location); - if (method == null) { + if (method != null) { + for (AExpression argument : arguments) { + argument.write(writer, globals); + } + + writer.invokeStatic(CLASS_TYPE, new Method(method.name, method.methodType.toMethodDescriptorString())); + } else if (statik != null) { + for (AExpression argument : arguments) { + argument.write(writer, globals); + } + + writer.invokeStatic(Type.getType(statik.targetClass), + new Method(statik.javaMethod.getName(), statik.methodType.toMethodDescriptorString())); + } else if (binding != null) { String name = globals.addBinding(binding.javaConstructor.getDeclaringClass()); Type type = Type.getType(binding.javaConstructor.getDeclaringClass()); int javaConstructorParameterCount = binding.javaConstructor.getParameterCount(); @@ -124,11 +155,7 @@ void write(MethodWriter writer, Globals globals) { writer.invokeVirtual(type, Method.getMethod(binding.javaMethod)); } else { - for (AExpression argument : arguments) { - argument.write(writer, globals); - } - - writer.invokeStatic(CLASS_TYPE, new Method(method.name, method.methodType.toMethodDescriptorString())); + throw new IllegalStateException("Illegal tree structure."); } } diff --git a/modules/lang-painless/src/main/resources/org/elasticsearch/painless/spi/org.elasticsearch.txt b/modules/lang-painless/src/main/resources/org/elasticsearch/painless/spi/org.elasticsearch.txt index 444234384c6d3..e6cf82b0d9159 100644 --- a/modules/lang-painless/src/main/resources/org/elasticsearch/painless/spi/org.elasticsearch.txt +++ b/modules/lang-painless/src/main/resources/org/elasticsearch/painless/spi/org.elasticsearch.txt @@ -177,5 +177,6 @@ class org.elasticsearch.painless.FeatureTest no_import { # for testing static { + float staticAddFloatsTest(float, float) from org.elasticsearch.painless.FeatureTest int testAddWithState(int, int, int, double) bound_to org.elasticsearch.painless.BindingTest } \ No newline at end of file diff --git a/modules/lang-painless/src/test/java/org/elasticsearch/painless/BasicAPITests.java b/modules/lang-painless/src/test/java/org/elasticsearch/painless/BasicAPITests.java index 25866c8d668a3..9863db0b21eac 100644 --- a/modules/lang-painless/src/test/java/org/elasticsearch/painless/BasicAPITests.java +++ b/modules/lang-painless/src/test/java/org/elasticsearch/painless/BasicAPITests.java @@ -133,4 +133,8 @@ public void testPublicMemberAccess() { public void testNoSemicolon() { assertEquals(true, exec("def x = true; if (x) return x")); } + + public void testStatic() { + assertEquals(15.5f, exec("staticAddFloatsTest(6.5f, 9.0f)")); + } } diff --git a/modules/lang-painless/src/test/java/org/elasticsearch/painless/ScriptTestCase.java b/modules/lang-painless/src/test/java/org/elasticsearch/painless/ScriptTestCase.java index 96cc296a1af52..2612ce2aa4861 100644 --- a/modules/lang-painless/src/test/java/org/elasticsearch/painless/ScriptTestCase.java +++ b/modules/lang-painless/src/test/java/org/elasticsearch/painless/ScriptTestCase.java @@ -47,6 +47,8 @@ * Typically just asserts the output of {@code exec()} */ public abstract class ScriptTestCase extends ESTestCase { + private static PainlessLookup PAINLESS_LOOKUP = PainlessLookupBuilder.buildFromWhitelists(Whitelist.BASE_WHITELISTS); + protected PainlessScriptEngine scriptEngine; @Before @@ -92,12 +94,12 @@ public Object exec(String script, Map vars, boolean picky) { public Object exec(String script, Map vars, Map compileParams, Scorer scorer, boolean picky) { // test for ambiguity errors before running the actual script if picky is true if (picky) { - PainlessLookup painlessLookup = PainlessLookupBuilder.buildFromWhitelists(Whitelist.BASE_WHITELISTS); - ScriptClassInfo scriptClassInfo = new ScriptClassInfo(painlessLookup, GenericElasticsearchScript.class); + ScriptClassInfo scriptClassInfo = new ScriptClassInfo(PAINLESS_LOOKUP, GenericElasticsearchScript.class); CompilerSettings pickySettings = new CompilerSettings(); pickySettings.setPicky(true); pickySettings.setRegexesEnabled(CompilerSettings.REGEX_ENABLED.get(scriptEngineSettings())); - Walker.buildPainlessTree(scriptClassInfo, new MainMethodReserved(), getTestName(), script, pickySettings, painlessLookup, null); + Walker.buildPainlessTree( + scriptClassInfo, new MainMethodReserved(), getTestName(), script, pickySettings, PAINLESS_LOOKUP, null); } // test actual script execution ExecutableScript.Factory factory = scriptEngine.compile(null, script, ExecutableScript.CONTEXT, compileParams); From d69b10faa069f41cb0f95d688ab5e167a313da60 Mon Sep 17 00:00:00 2001 From: Jack Conradson Date: Wed, 5 Sep 2018 13:41:46 -0700 Subject: [PATCH 2/3] Make static final. --- .../test/java/org/elasticsearch/painless/ScriptTestCase.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/lang-painless/src/test/java/org/elasticsearch/painless/ScriptTestCase.java b/modules/lang-painless/src/test/java/org/elasticsearch/painless/ScriptTestCase.java index 2612ce2aa4861..963a433f172e8 100644 --- a/modules/lang-painless/src/test/java/org/elasticsearch/painless/ScriptTestCase.java +++ b/modules/lang-painless/src/test/java/org/elasticsearch/painless/ScriptTestCase.java @@ -47,7 +47,7 @@ * Typically just asserts the output of {@code exec()} */ public abstract class ScriptTestCase extends ESTestCase { - private static PainlessLookup PAINLESS_LOOKUP = PainlessLookupBuilder.buildFromWhitelists(Whitelist.BASE_WHITELISTS); + private static final PainlessLookup PAINLESS_LOOKUP = PainlessLookupBuilder.buildFromWhitelists(Whitelist.BASE_WHITELISTS); protected PainlessScriptEngine scriptEngine; From f4b50728e266b65784190e1ab09257e6cdcddefb Mon Sep 17 00:00:00 2001 From: Jack Conradson Date: Fri, 7 Sep 2018 15:53:05 -0700 Subject: [PATCH 3/3] Reponse to PR comments. --- .../elasticsearch/painless/spi/Whitelist.java | 8 +- .../painless/spi/WhitelistLoader.java | 41 +++++----- .../painless/lookup/PainlessLookup.java | 12 +-- .../lookup/PainlessLookupBuilder.java | 75 ++++++++++--------- .../painless/node/ECallLocal.java | 18 ++--- .../painless/spi/org.elasticsearch.txt | 4 +- 6 files changed, 79 insertions(+), 79 deletions(-) diff --git a/modules/lang-painless/spi/src/main/java/org/elasticsearch/painless/spi/Whitelist.java b/modules/lang-painless/spi/src/main/java/org/elasticsearch/painless/spi/Whitelist.java index 428935b7e895b..31a9e595d0b6d 100644 --- a/modules/lang-painless/spi/src/main/java/org/elasticsearch/painless/spi/Whitelist.java +++ b/modules/lang-painless/spi/src/main/java/org/elasticsearch/painless/spi/Whitelist.java @@ -62,18 +62,18 @@ public final class Whitelist { public final List whitelistClasses; /** The {@link List} of all the whitelisted static Painless methods. */ - public final List whitelistStatics; + public final List whitelistImportedMethods; /** The {@link List} of all the whitelisted Painless bindings. */ public final List whitelistBindings; - /** Standard constructor. All values must be not {@code null}. */ + /** Standard constructor. All values must be not {@code null}. */ public Whitelist(ClassLoader classLoader, List whitelistClasses, - List whitelistStatics, List whitelistBindings) { + List whitelistImportedMethods, List whitelistBindings) { this.classLoader = Objects.requireNonNull(classLoader); this.whitelistClasses = Collections.unmodifiableList(Objects.requireNonNull(whitelistClasses)); - this.whitelistStatics = Collections.unmodifiableList(Objects.requireNonNull(whitelistStatics)); + this.whitelistImportedMethods = Collections.unmodifiableList(Objects.requireNonNull(whitelistImportedMethods)); this.whitelistBindings = Collections.unmodifiableList(Objects.requireNonNull(whitelistBindings)); } } diff --git a/modules/lang-painless/spi/src/main/java/org/elasticsearch/painless/spi/WhitelistLoader.java b/modules/lang-painless/spi/src/main/java/org/elasticsearch/painless/spi/WhitelistLoader.java index 1c9c9134b38db..2f5dec769fc2f 100644 --- a/modules/lang-painless/spi/src/main/java/org/elasticsearch/painless/spi/WhitelistLoader.java +++ b/modules/lang-painless/spi/src/main/java/org/elasticsearch/painless/spi/WhitelistLoader.java @@ -193,18 +193,18 @@ public static Whitelist loadFromResourceFiles(Class resource, String... filep whitelistConstructors = new ArrayList<>(); whitelistMethods = new ArrayList<>(); whitelistFields = new ArrayList<>(); - } else if (line.startsWith("static ")) { + } else if (line.startsWith("static_import ")) { // Ensure the final token of the line is '{'. if (line.endsWith("{") == false) { throw new IllegalArgumentException( - "invalid static definition: failed to parse static opening bracket [" + line + "]"); + "invalid static import definition: failed to parse static import opening bracket [" + line + "]"); } if (parseType != null) { - throw new IllegalArgumentException("invalid definition: cannot embed static definition [" + line + "]"); + throw new IllegalArgumentException("invalid definition: cannot embed static import definition [" + line + "]"); } - parseType = "static"; + parseType = "static_import"; // Handle the end of a definition and reset all previously gathered values. // Expects the following format: '}' '\n' @@ -230,9 +230,9 @@ public static Whitelist loadFromResourceFiles(Class resource, String... filep // Reset the parseType. parseType = null; - // Handle static definition types. - // Expects the following format: ID ID '(' ( ID ( ',' ID )* )? ')' ( 'from' | 'bound_to' ) ID '\n' - } else if ("static".equals(parseType)) { + // Handle static import definition types. + // Expects the following format: ID ID '(' ( ID ( ',' ID )* )? ')' ( 'from_class' | 'bound_to' ) ID '\n' + } else if ("static_import".equals(parseType)) { // Mark the origin of this parsable object. String origin = "[" + filepath + "]:[" + number + "]"; @@ -241,7 +241,7 @@ public static Whitelist loadFromResourceFiles(Class resource, String... filep if (parameterStartIndex == -1) { throw new IllegalArgumentException( - "illegal static definition: start of method parameters not found [" + line + "]"); + "illegal static import definition: start of method parameters not found [" + line + "]"); } String[] tokens = line.substring(0, parameterStartIndex).trim().split("\\s+"); @@ -262,7 +262,7 @@ public static Whitelist loadFromResourceFiles(Class resource, String... filep if (parameterEndIndex == -1) { throw new IllegalArgumentException( - "illegal static definition: end of method parameters not found [" + line + "]"); + "illegal static import definition: end of method parameters not found [" + line + "]"); } String[] canonicalTypeNameParameters = @@ -273,30 +273,30 @@ public static Whitelist loadFromResourceFiles(Class resource, String... filep canonicalTypeNameParameters = new String[0]; } - // Parse the static type and class. + // Parse the static import type and class. tokens = line.substring(parameterEndIndex + 1).trim().split("\\s+"); - String staticType; + String staticImportType; String targetJavaClassName; // Based on the number of tokens, look up the type and class. if (tokens.length == 2) { - staticType = tokens[0]; + staticImportType = tokens[0]; targetJavaClassName = tokens[1]; } else { - throw new IllegalArgumentException("invalid static definition: unexpected format [" + line + "]"); + throw new IllegalArgumentException("invalid static import definition: unexpected format [" + line + "]"); } - // Add a static method or binding depending on the static type. - if ("from".equals(staticType)) { + // Add a static import method or binding depending on the static import type. + if ("from_class".equals(staticImportType)) { whitelistStatics.add(new WhitelistMethod(origin, targetJavaClassName, methodName, returnCanonicalTypeName, Arrays.asList(canonicalTypeNameParameters))); - } else if ("bound_to".equals(staticType)) { + } else if ("bound_to".equals(staticImportType)) { whitelistBindings.add(new WhitelistBinding(origin, targetJavaClassName, methodName, returnCanonicalTypeName, Arrays.asList(canonicalTypeNameParameters))); } else { - throw new IllegalArgumentException( - "invalid static definition: unexpected static type [" + staticType + "] [" + line + "]"); + throw new IllegalArgumentException("invalid static import definition: " + + "unexpected static import type [" + staticImportType + "] [" + line + "]"); } // Handle class definition types. @@ -304,11 +304,6 @@ public static Whitelist loadFromResourceFiles(Class resource, String... filep // Mark the origin of this parsable object. String origin = "[" + filepath + "]:[" + number + "]"; - // Ensure we have a defined class before adding any constructors, methods, augmented methods, or fields. - if (parseType == null) { - throw new IllegalArgumentException("invalid definition: expected one of ['class', 'static'] [" + line + "]"); - } - // Handle the case for a constructor definition. // Expects the following format: '(' ( ID ( ',' ID )* )? ')' '\n' if (line.startsWith("(")) { diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/lookup/PainlessLookup.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/lookup/PainlessLookup.java index 27636d8220a33..7be659d11a124 100644 --- a/modules/lang-painless/src/main/java/org/elasticsearch/painless/lookup/PainlessLookup.java +++ b/modules/lang-painless/src/main/java/org/elasticsearch/painless/lookup/PainlessLookup.java @@ -37,23 +37,23 @@ public final class PainlessLookup { private final Map> canonicalClassNamesToClasses; private final Map, PainlessClass> classesToPainlessClasses; - private final Map painlessMethodKeysToPainlessStatics; + private final Map painlessMethodKeysToImportedPainlessMethods; private final Map painlessMethodKeysToPainlessBindings; PainlessLookup(Map> canonicalClassNamesToClasses, Map, PainlessClass> classesToPainlessClasses, - Map painlessMethodKeysToPainlessStatics, + Map painlessMethodKeysToImportedPainlessMethods, Map painlessMethodKeysToPainlessBindings) { Objects.requireNonNull(canonicalClassNamesToClasses); Objects.requireNonNull(classesToPainlessClasses); - Objects.requireNonNull(painlessMethodKeysToPainlessStatics); + Objects.requireNonNull(painlessMethodKeysToImportedPainlessMethods); Objects.requireNonNull(painlessMethodKeysToPainlessBindings); this.canonicalClassNamesToClasses = Collections.unmodifiableMap(canonicalClassNamesToClasses); this.classesToPainlessClasses = Collections.unmodifiableMap(classesToPainlessClasses); - this.painlessMethodKeysToPainlessStatics = Collections.unmodifiableMap(painlessMethodKeysToPainlessStatics); + this.painlessMethodKeysToImportedPainlessMethods = Collections.unmodifiableMap(painlessMethodKeysToImportedPainlessMethods); this.painlessMethodKeysToPainlessBindings = Collections.unmodifiableMap(painlessMethodKeysToPainlessBindings); } @@ -174,12 +174,12 @@ public PainlessField lookupPainlessField(Class targetClass, boolean isStatic, return painlessField; } - public PainlessMethod lookupPainlessStatic(String methodName, int arity) { + public PainlessMethod lookupImportedPainlessMethod(String methodName, int arity) { Objects.requireNonNull(methodName); String painlessMethodKey = buildPainlessMethodKey(methodName, arity); - return painlessMethodKeysToPainlessStatics.get(painlessMethodKey); + return painlessMethodKeysToImportedPainlessMethods.get(painlessMethodKey); } public PainlessBinding lookupPainlessBinding(String methodName, int arity) { diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/lookup/PainlessLookupBuilder.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/lookup/PainlessLookupBuilder.java index 6c94802121d95..b822bd47c7a48 100644 --- a/modules/lang-painless/src/main/java/org/elasticsearch/painless/lookup/PainlessLookupBuilder.java +++ b/modules/lang-painless/src/main/java/org/elasticsearch/painless/lookup/PainlessLookupBuilder.java @@ -243,9 +243,9 @@ public static PainlessLookup buildFromWhitelists(List whitelists) { } } - for (WhitelistMethod whitelistStatic : whitelist.whitelistStatics) { + for (WhitelistMethod whitelistStatic : whitelist.whitelistImportedMethods) { origin = whitelistStatic.origin; - painlessLookupBuilder.addPainlessStatic( + painlessLookupBuilder.addImportedPainlessMethod( whitelist.classLoader, whitelistStatic.augmentedCanonicalClassName, whitelistStatic.methodName, whitelistStatic.returnCanonicalTypeName, whitelistStatic.canonicalTypeNameParameters); @@ -269,14 +269,14 @@ public static PainlessLookup buildFromWhitelists(List whitelists) { private final Map> canonicalClassNamesToClasses; private final Map, PainlessClassBuilder> classesToPainlessClassBuilders; - private final Map painlessMethodKeysToPainlessStatics; + private final Map painlessMethodKeysToImportedPainlessMethods; private final Map painlessMethodKeysToPainlessBindings; public PainlessLookupBuilder() { canonicalClassNamesToClasses = new HashMap<>(); classesToPainlessClassBuilders = new HashMap<>(); - painlessMethodKeysToPainlessStatics = new HashMap<>(); + painlessMethodKeysToImportedPainlessMethods = new HashMap<>(); painlessMethodKeysToPainlessBindings = new HashMap<>(); } @@ -584,6 +584,12 @@ public void addPainlessMethod(Class targetClass, Class augmentedClass, } else { try { javaMethod = augmentedClass.getMethod(methodName, javaTypeParameters.toArray(new Class[typeParametersSize])); + + if (Modifier.isStatic(javaMethod.getModifiers()) == false) { + throw new IllegalArgumentException("method [[" + targetCanonicalClassName + "], [" + methodName + "], " + + typesToCanonicalTypeNames(typeParameters) + "] with augmented class " + + "[" + typeToCanonicalTypeName(augmentedClass) + "] must be static"); + } } catch (NoSuchMethodException nsme) { throw new IllegalArgumentException("method reflection object [[" + targetCanonicalClassName + "], " + "[" + methodName + "], " + typesToCanonicalTypeNames(typeParameters) + "] not found " + @@ -631,7 +637,7 @@ public void addPainlessMethod(Class targetClass, Class augmentedClass, "with the same arity and different return type or type parameters"); } } else { - PainlessMethod painlessMethod = painlessClassBuilder.staticMethods.get(painlessMethodKey); + PainlessMethod painlessMethod = painlessClassBuilder.methods.get(painlessMethodKey); if (painlessMethod == null) { MethodHandle methodHandle; @@ -799,7 +805,7 @@ public void addPainlessField(Class targetClass, String fieldName, Class ty } } - public void addPainlessStatic(ClassLoader classLoader, String targetCanonicalClassName, + public void addImportedPainlessMethod(ClassLoader classLoader, String targetCanonicalClassName, String methodName, String returnCanonicalTypeName, List canonicalTypeNameParameters) { Objects.requireNonNull(classLoader); @@ -811,7 +817,7 @@ public void addPainlessStatic(ClassLoader classLoader, String targetCanonicalCla Class targetClass = canonicalClassNamesToClasses.get(targetCanonicalClassName); if (targetClass == null) { - throw new IllegalArgumentException("target class [" + targetCanonicalClassName + "] not found for static " + + throw new IllegalArgumentException("target class [" + targetCanonicalClassName + "] not found for imported method " + "[[" + targetCanonicalClassName + "], [" + methodName + "], " + canonicalTypeNameParameters + "]"); } @@ -821,7 +827,7 @@ public void addPainlessStatic(ClassLoader classLoader, String targetCanonicalCla Class typeParameter = canonicalTypeNameToType(canonicalTypeNameParameter); if (typeParameter == null) { - throw new IllegalArgumentException("type parameter [" + canonicalTypeNameParameter + "] not found for static " + + throw new IllegalArgumentException("type parameter [" + canonicalTypeNameParameter + "] not found for imported method " + "[[" + targetCanonicalClassName + "], [" + methodName + "], " + canonicalTypeNameParameters + "]"); } @@ -831,34 +837,34 @@ public void addPainlessStatic(ClassLoader classLoader, String targetCanonicalCla Class returnType = canonicalTypeNameToType(returnCanonicalTypeName); if (returnType == null) { - throw new IllegalArgumentException("return type [" + returnCanonicalTypeName + "] not found for static " + + throw new IllegalArgumentException("return type [" + returnCanonicalTypeName + "] not found for imported method " + "[[" + targetCanonicalClassName + "], [" + methodName + "], " + canonicalTypeNameParameters + "]"); } - addPainlessStatic(targetClass, methodName, returnType, typeParameters); + addImportedPainlessMethod(targetClass, methodName, returnType, typeParameters); } - public void addPainlessStatic(Class targetClass, String methodName, Class returnType, List> typeParameters) { + public void addImportedPainlessMethod(Class targetClass, String methodName, Class returnType, List> typeParameters) { Objects.requireNonNull(targetClass); Objects.requireNonNull(methodName); Objects.requireNonNull(returnType); Objects.requireNonNull(typeParameters); if (targetClass == def.class) { - throw new IllegalArgumentException("cannot add static from reserved class [" + DEF_CLASS_NAME + "]"); + throw new IllegalArgumentException("cannot add imported method from reserved class [" + DEF_CLASS_NAME + "]"); } String targetCanonicalClassName = typeToCanonicalTypeName(targetClass); if (METHOD_NAME_PATTERN.matcher(methodName).matches() == false) { throw new IllegalArgumentException( - "invalid method name [" + methodName + "] for static target class [" + targetCanonicalClassName + "]."); + "invalid imported method name [" + methodName + "] for target class [" + targetCanonicalClassName + "]."); } PainlessClassBuilder painlessClassBuilder = classesToPainlessClassBuilders.get(targetClass); if (painlessClassBuilder == null) { - throw new IllegalArgumentException("target class [" + targetCanonicalClassName + "] not found for static " + + throw new IllegalArgumentException("target class [" + targetCanonicalClassName + "] not found for imported method " + "[[" + targetCanonicalClassName + "], [" + methodName + "], " + typesToCanonicalTypeNames(typeParameters) + "]"); } @@ -868,7 +874,7 @@ public void addPainlessStatic(Class targetClass, String methodName, Class for (Class typeParameter : typeParameters) { if (isValidType(typeParameter) == false) { throw new IllegalArgumentException("type parameter [" + typeToCanonicalTypeName(typeParameter) + "] " + - "not found for static [[" + targetCanonicalClassName + "], [" + methodName + "], " + + "not found for imported method [[" + targetCanonicalClassName + "], [" + methodName + "], " + typesToCanonicalTypeNames(typeParameters) + "]"); } @@ -876,7 +882,7 @@ public void addPainlessStatic(Class targetClass, String methodName, Class } if (isValidType(returnType) == false) { - throw new IllegalArgumentException("return type [" + typeToCanonicalTypeName(returnType) + "] not found for static " + + throw new IllegalArgumentException("return type [" + typeToCanonicalTypeName(returnType) + "] not found for imported method " + "[[" + targetCanonicalClassName + "], [" + methodName + "], " + typesToCanonicalTypeNames(typeParameters) + "]"); } @@ -885,57 +891,56 @@ public void addPainlessStatic(Class targetClass, String methodName, Class try { javaMethod = targetClass.getMethod(methodName, javaTypeParameters.toArray(new Class[typeParametersSize])); } catch (NoSuchMethodException nsme) { - throw new IllegalArgumentException("static method reflection object [[" + targetCanonicalClassName + "], " + + throw new IllegalArgumentException("imported method reflection object [[" + targetCanonicalClassName + "], " + "[" + methodName + "], " + typesToCanonicalTypeNames(typeParameters) + "] not found", nsme); } if (javaMethod.getReturnType() != typeToJavaType(returnType)) { throw new IllegalArgumentException("return type [" + typeToCanonicalTypeName(javaMethod.getReturnType()) + "] " + "does not match the specified returned type [" + typeToCanonicalTypeName(returnType) + "] " + - "for static [[" + targetClass.getCanonicalName() + "], [" + methodName + "], " + + "for imported method [[" + targetClass.getCanonicalName() + "], [" + methodName + "], " + typesToCanonicalTypeNames(typeParameters) + "]"); } if (Modifier.isStatic(javaMethod.getModifiers()) == false) { - throw new IllegalArgumentException("return type [" + typeToCanonicalTypeName(javaMethod.getReturnType()) + "] " + - "does not match the specified returned type [" + typeToCanonicalTypeName(returnType) + "] " + - "for static [[" + targetClass.getCanonicalName() + "], [" + methodName + "], " + - typesToCanonicalTypeNames(typeParameters) + "]"); + throw new IllegalArgumentException("imported method [[" + targetClass.getCanonicalName() + "], [" + methodName + "], " + + typesToCanonicalTypeNames(typeParameters) + "] must be static"); } String painlessMethodKey = buildPainlessMethodKey(methodName, typeParametersSize); if (painlessMethodKeysToPainlessBindings.containsKey(painlessMethodKey)) { - throw new IllegalArgumentException("static and binding cannot have the same name [" + methodName + "]"); + throw new IllegalArgumentException("imported method and binding cannot have the same name [" + methodName + "]"); } - PainlessMethod painlessStatic = painlessMethodKeysToPainlessStatics.get(painlessMethodKey); + PainlessMethod importedPainlessMethod = painlessMethodKeysToImportedPainlessMethods.get(painlessMethodKey); - if (painlessStatic == null) { + if (importedPainlessMethod == null) { MethodHandle methodHandle; try { methodHandle = MethodHandles.publicLookup().in(targetClass).unreflect(javaMethod); } catch (IllegalAccessException iae) { - throw new IllegalArgumentException("static method handle [[" + targetClass.getCanonicalName() + "], " + + throw new IllegalArgumentException("imported method handle [[" + targetClass.getCanonicalName() + "], " + "[" + methodName + "], " + typesToCanonicalTypeNames(typeParameters) + "] not found", iae); } MethodType methodType = methodHandle.type(); - painlessStatic = painlessMethodCache.computeIfAbsent( + importedPainlessMethod = painlessMethodCache.computeIfAbsent( new PainlessMethodCacheKey(targetClass, methodName, returnType, typeParameters), key -> new PainlessMethod(javaMethod, targetClass, returnType, typeParameters, methodHandle, methodType)); - painlessMethodKeysToPainlessStatics.put(painlessMethodKey, painlessStatic); - } else if (painlessStatic.returnType == returnType && painlessStatic.typeParameters.equals(typeParameters) == false) { - throw new IllegalArgumentException("cannot have statics " + + painlessMethodKeysToImportedPainlessMethods.put(painlessMethodKey, importedPainlessMethod); + } else if (importedPainlessMethod.returnType == returnType && + importedPainlessMethod.typeParameters.equals(typeParameters) == false) { + throw new IllegalArgumentException("cannot have imported methods " + "[[" + targetCanonicalClassName + "], [" + methodName + "], " + "[" + typeToCanonicalTypeName(returnType) + "], " + typesToCanonicalTypeNames(typeParameters) + "] and " + "[[" + targetCanonicalClassName + "], [" + methodName + "], " + - "[" + typeToCanonicalTypeName(painlessStatic.returnType) + "], " + - typesToCanonicalTypeNames(painlessStatic.typeParameters) + "] " + + "[" + typeToCanonicalTypeName(importedPainlessMethod.returnType) + "], " + + typesToCanonicalTypeNames(importedPainlessMethod.typeParameters) + "] " + "with the same arity and different return type or type parameters"); } } @@ -1090,8 +1095,8 @@ public void addPainlessBinding(Class targetClass, String methodName, Class String painlessMethodKey = buildPainlessMethodKey(methodName, constructorTypeParametersSize + methodTypeParametersSize); - if (painlessMethodKeysToPainlessStatics.containsKey(painlessMethodKey)) { - throw new IllegalArgumentException("binding and static cannot have the same name [" + methodName + "]"); + if (painlessMethodKeysToImportedPainlessMethods.containsKey(painlessMethodKey)) { + throw new IllegalArgumentException("binding and imported method cannot have the same name [" + methodName + "]"); } PainlessBinding painlessBinding = painlessMethodKeysToPainlessBindings.get(painlessMethodKey); @@ -1134,7 +1139,7 @@ public PainlessLookup build() { } return new PainlessLookup(canonicalClassNamesToClasses, classesToPainlessClasses, - painlessMethodKeysToPainlessStatics, painlessMethodKeysToPainlessBindings); + painlessMethodKeysToImportedPainlessMethods, painlessMethodKeysToPainlessBindings); } private void copyPainlessClassMembers() { diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/ECallLocal.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/ECallLocal.java index 38f318bbdf394..d161296d90a56 100644 --- a/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/ECallLocal.java +++ b/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/ECallLocal.java @@ -46,7 +46,7 @@ public final class ECallLocal extends AExpression { private final List arguments; private LocalMethod method = null; - private PainlessMethod statik = null; + private PainlessMethod imported = null; private PainlessBinding binding = null; public ECallLocal(Location location, String name, List arguments) { @@ -68,9 +68,9 @@ void analyze(Locals locals) { method = locals.getMethod(name, arguments.size()); if (method == null) { - statik = locals.getPainlessLookup().lookupPainlessStatic(name, arguments.size()); + imported = locals.getPainlessLookup().lookupImportedPainlessMethod(name, arguments.size()); - if (statik == null) { + if (imported == null) { binding = locals.getPainlessLookup().lookupPainlessBinding(name, arguments.size()); if (binding == null) { @@ -85,9 +85,9 @@ void analyze(Locals locals) { if (method != null) { typeParameters = new ArrayList<>(method.typeParameters); actual = method.returnType; - } else if (statik != null) { - typeParameters = new ArrayList<>(statik.typeParameters); - actual = statik.returnType; + } else if (imported != null) { + typeParameters = new ArrayList<>(imported.typeParameters); + actual = imported.returnType; } else if (binding != null) { typeParameters = new ArrayList<>(binding.typeParameters); actual = binding.returnType; @@ -117,13 +117,13 @@ void write(MethodWriter writer, Globals globals) { } writer.invokeStatic(CLASS_TYPE, new Method(method.name, method.methodType.toMethodDescriptorString())); - } else if (statik != null) { + } else if (imported != null) { for (AExpression argument : arguments) { argument.write(writer, globals); } - writer.invokeStatic(Type.getType(statik.targetClass), - new Method(statik.javaMethod.getName(), statik.methodType.toMethodDescriptorString())); + writer.invokeStatic(Type.getType(imported.targetClass), + new Method(imported.javaMethod.getName(), imported.methodType.toMethodDescriptorString())); } else if (binding != null) { String name = globals.addBinding(binding.javaConstructor.getDeclaringClass()); Type type = Type.getType(binding.javaConstructor.getDeclaringClass()); diff --git a/modules/lang-painless/src/main/resources/org/elasticsearch/painless/spi/org.elasticsearch.txt b/modules/lang-painless/src/main/resources/org/elasticsearch/painless/spi/org.elasticsearch.txt index e6cf82b0d9159..81009de9979eb 100644 --- a/modules/lang-painless/src/main/resources/org/elasticsearch/painless/spi/org.elasticsearch.txt +++ b/modules/lang-painless/src/main/resources/org/elasticsearch/painless/spi/org.elasticsearch.txt @@ -176,7 +176,7 @@ class org.elasticsearch.painless.FeatureTest no_import { } # for testing -static { - float staticAddFloatsTest(float, float) from org.elasticsearch.painless.FeatureTest +static_import { + float staticAddFloatsTest(float, float) from_class org.elasticsearch.painless.FeatureTest int testAddWithState(int, int, int, double) bound_to org.elasticsearch.painless.BindingTest } \ No newline at end of file