From df579f8bce005601d8b6c24f91699e67a92eac19 Mon Sep 17 00:00:00 2001 From: Jack Conradson Date: Thu, 26 Jul 2018 09:02:06 -0700 Subject: [PATCH] Painless: Clean Up PainlessClass Variables (#32380) Removes the variables name, clazz, and type as they are unnecessary. Renames staticMembers -> staticFields, members -> fields, getters -> getterMethodHandles, and setters -> setterMethodHandles. --- .../painless/spi/PainlessExtension.java | 4 +- .../java/org/elasticsearch/painless/Def.java | 8 +-- .../painless/PainlessExplainError.java | 3 +- .../painless/lookup/PainlessClass.java | 35 ++++------ .../painless/lookup/PainlessClassBuilder.java | 35 +++------- .../painless/lookup/PainlessLookup.java | 4 +- .../lookup/PainlessLookupBuilder.java | 69 ++++++++++--------- .../elasticsearch/painless/node/ENewObj.java | 8 ++- .../elasticsearch/painless/node/PBrace.java | 4 +- .../painless/node/PCallInvoke.java | 4 +- .../elasticsearch/painless/node/PField.java | 6 +- .../painless/node/PSubListShortcut.java | 15 ++-- .../painless/node/PSubMapShortcut.java | 15 ++-- .../painless/PainlessDocGenerator.java | 49 +++++++------ .../painless/node/NodeToStringTests.java | 14 ++-- 15 files changed, 131 insertions(+), 142 deletions(-) diff --git a/modules/lang-painless/spi/src/main/java/org/elasticsearch/painless/spi/PainlessExtension.java b/modules/lang-painless/spi/src/main/java/org/elasticsearch/painless/spi/PainlessExtension.java index 9434e6986c0..eb971353437 100644 --- a/modules/lang-painless/spi/src/main/java/org/elasticsearch/painless/spi/PainlessExtension.java +++ b/modules/lang-painless/spi/src/main/java/org/elasticsearch/painless/spi/PainlessExtension.java @@ -19,11 +19,11 @@ package org.elasticsearch.painless.spi; +import org.elasticsearch.script.ScriptContext; + import java.util.List; import java.util.Map; -import org.elasticsearch.script.ScriptContext; - public interface PainlessExtension { Map, List> getContextWhitelists(); diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/Def.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/Def.java index dad8da06e76..8a90f53b4fd 100644 --- a/modules/lang-painless/src/main/java/org/elasticsearch/painless/Def.java +++ b/modules/lang-painless/src/main/java/org/elasticsearch/painless/Def.java @@ -421,7 +421,7 @@ public final class Def { PainlessClass struct = painlessLookup.getPainlessStructFromJavaClass(clazz); if (struct != null) { - MethodHandle handle = struct.getters.get(name); + MethodHandle handle = struct.getterMethodHandles.get(name); if (handle != null) { return handle; } @@ -431,7 +431,7 @@ public final class Def { struct = painlessLookup.getPainlessStructFromJavaClass(iface); if (struct != null) { - MethodHandle handle = struct.getters.get(name); + MethodHandle handle = struct.getterMethodHandles.get(name); if (handle != null) { return handle; } @@ -492,7 +492,7 @@ public final class Def { PainlessClass struct = painlessLookup.getPainlessStructFromJavaClass(clazz); if (struct != null) { - MethodHandle handle = struct.setters.get(name); + MethodHandle handle = struct.setterMethodHandles.get(name); if (handle != null) { return handle; } @@ -502,7 +502,7 @@ public final class Def { struct = painlessLookup.getPainlessStructFromJavaClass(iface); if (struct != null) { - MethodHandle handle = struct.setters.get(name); + MethodHandle handle = struct.setterMethodHandles.get(name); if (handle != null) { return handle; } diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/PainlessExplainError.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/PainlessExplainError.java index 1236c4977e8..7bef028c7d1 100644 --- a/modules/lang-painless/src/main/java/org/elasticsearch/painless/PainlessExplainError.java +++ b/modules/lang-painless/src/main/java/org/elasticsearch/painless/PainlessExplainError.java @@ -22,6 +22,7 @@ package org.elasticsearch.painless; import org.elasticsearch.painless.api.Debug; import org.elasticsearch.painless.lookup.PainlessClass; import org.elasticsearch.painless.lookup.PainlessLookup; +import org.elasticsearch.painless.lookup.PainlessLookupUtility; import org.elasticsearch.script.ScriptException; import java.util.List; @@ -58,7 +59,7 @@ public class PainlessExplainError extends Error { javaClassName = objectToExplain.getClass().getName(); PainlessClass struct = painlessLookup.getPainlessStructFromJavaClass(objectToExplain.getClass()); if (struct != null) { - painlessClassName = struct.name; + painlessClassName = PainlessLookupUtility.typeToCanonicalTypeName(objectToExplain.getClass()); } } diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/lookup/PainlessClass.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/lookup/PainlessClass.java index 57b18bc60da..24dcf0ebdba 100644 --- a/modules/lang-painless/src/main/java/org/elasticsearch/painless/lookup/PainlessClass.java +++ b/modules/lang-painless/src/main/java/org/elasticsearch/painless/lookup/PainlessClass.java @@ -19,47 +19,38 @@ package org.elasticsearch.painless.lookup; -import org.objectweb.asm.Type; - import java.lang.invoke.MethodHandle; import java.util.Collections; import java.util.Map; public final class PainlessClass { - public final String name; - public final Class clazz; - public final Type type; - public final Map constructors; public final Map staticMethods; public final Map methods; - public final Map staticMembers; - public final Map members; + public final Map staticFields; + public final Map fields; - public final Map getters; - public final Map setters; + public final Map getterMethodHandles; + public final Map setterMethodHandles; public final PainlessMethod functionalMethod; - PainlessClass(String name, Class clazz, Type type, - Map constructors, Map staticMethods, Map methods, - Map staticMembers, Map members, - Map getters, Map setters, - PainlessMethod functionalMethod) { - this.name = name; - this.clazz = clazz; - this.type = type; + PainlessClass(Map constructors, + Map staticMethods, Map methods, + Map staticFields, Map fields, + Map getterMethodHandles, Map setterMethodHandles, + PainlessMethod functionalMethod) { this.constructors = Collections.unmodifiableMap(constructors); this.staticMethods = Collections.unmodifiableMap(staticMethods); this.methods = Collections.unmodifiableMap(methods); - this.staticMembers = Collections.unmodifiableMap(staticMembers); - this.members = Collections.unmodifiableMap(members); + this.staticFields = Collections.unmodifiableMap(staticFields); + this.fields = Collections.unmodifiableMap(fields); - this.getters = Collections.unmodifiableMap(getters); - this.setters = Collections.unmodifiableMap(setters); + this.getterMethodHandles = Collections.unmodifiableMap(getterMethodHandles); + this.setterMethodHandles = Collections.unmodifiableMap(setterMethodHandles); this.functionalMethod = functionalMethod; } diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/lookup/PainlessClassBuilder.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/lookup/PainlessClassBuilder.java index 0eda3660f0b..2f41ed5dca8 100644 --- a/modules/lang-painless/src/main/java/org/elasticsearch/painless/lookup/PainlessClassBuilder.java +++ b/modules/lang-painless/src/main/java/org/elasticsearch/painless/lookup/PainlessClassBuilder.java @@ -19,52 +19,39 @@ package org.elasticsearch.painless.lookup; -import org.objectweb.asm.Type; - import java.lang.invoke.MethodHandle; import java.util.HashMap; import java.util.Map; final class PainlessClassBuilder { - final String name; - final Class clazz; - final Type type; - final Map constructors; final Map staticMethods; final Map methods; - final Map staticMembers; - final Map members; + final Map staticFields; + final Map fields; - final Map getters; - final Map setters; + final Map getterMethodHandles; + final Map setterMethodHandles; PainlessMethod functionalMethod; - PainlessClassBuilder(String name, Class clazz, Type type) { - this.name = name; - this.clazz = clazz; - this.type = type; - + PainlessClassBuilder() { constructors = new HashMap<>(); staticMethods = new HashMap<>(); methods = new HashMap<>(); - staticMembers = new HashMap<>(); - members = new HashMap<>(); + staticFields = new HashMap<>(); + fields = new HashMap<>(); - getters = new HashMap<>(); - setters = new HashMap<>(); + getterMethodHandles = new HashMap<>(); + setterMethodHandles = new HashMap<>(); functionalMethod = null; } PainlessClass build() { - return new PainlessClass(name, clazz, type, - constructors, staticMethods, methods, - staticMembers, members, - getters, setters, - functionalMethod); + return new PainlessClass(constructors, staticMethods, methods, staticFields, fields, + getterMethodHandles, setterMethodHandles, functionalMethod); } } 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 bcecd7bbdc7..67c04498a58 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 @@ -29,8 +29,8 @@ import java.util.Map; */ public final class PainlessLookup { - public Collection getStructs() { - return classesToPainlessClasses.values(); + public Collection> getStructs() { + return classesToPainlessClasses.keySet(); } private final Map> canonicalClassNamesToClasses; 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 519227bb901..3675cc7cd0f 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 @@ -179,8 +179,7 @@ public class PainlessLookupBuilder { classesToPainlessClassBuilders = new HashMap<>(); canonicalClassNamesToClasses.put(DEF_CLASS_NAME, def.class); - classesToPainlessClassBuilders.put(def.class, - new PainlessClassBuilder(DEF_CLASS_NAME, Object.class, org.objectweb.asm.Type.getType(Object.class))); + classesToPainlessClassBuilders.put(def.class, new PainlessClassBuilder()); } private Class canonicalTypeNameToType(String canonicalTypeName) { @@ -234,17 +233,21 @@ public class PainlessLookupBuilder { throw new IllegalArgumentException("invalid class name [" + canonicalClassName + "]"); } + + Class existingClass = canonicalClassNamesToClasses.get(typeToCanonicalTypeName(clazz)); + + if (existingClass != null && existingClass != clazz) { + throw new IllegalArgumentException("class [" + canonicalClassName + "] " + + "cannot represent multiple java classes with the same name from different class loaders"); + } + PainlessClassBuilder existingPainlessClassBuilder = classesToPainlessClassBuilders.get(clazz); if (existingPainlessClassBuilder == null) { - PainlessClassBuilder painlessClassBuilder = - new PainlessClassBuilder(canonicalClassName, clazz, org.objectweb.asm.Type.getType(clazz)); + PainlessClassBuilder painlessClassBuilder = new PainlessClassBuilder(); canonicalClassNamesToClasses.put(canonicalClassName, clazz); classesToPainlessClassBuilders.put(clazz, painlessClassBuilder); - } else if (existingPainlessClassBuilder.clazz.equals(clazz) == false) { - throw new IllegalArgumentException("class [" + canonicalClassName + "] " + - "cannot represent multiple java classes with the same name from different class loaders"); } String javaClassName = clazz.getName(); @@ -265,7 +268,7 @@ public class PainlessLookupBuilder { canonicalClassNamesToClasses.put(importedCanonicalClassName, clazz); } - } else if (importedPainlessClass.equals(clazz) == false) { + } else if (importedPainlessClass != clazz) { throw new IllegalArgumentException("imported class [" + importedCanonicalClassName + "] cannot represent multiple " + "classes [" + canonicalClassName + "] and [" + typeToCanonicalTypeName(importedPainlessClass) + "]"); } else if (importClassName == false) { @@ -504,10 +507,10 @@ public class PainlessLookupBuilder { if (painlessMethod == null) { org.objectweb.asm.commons.Method asmMethod = org.objectweb.asm.commons.Method.getMethod(javaMethod); - MethodHandle javaMethodHandle; + MethodHandle methodHandle; try { - javaMethodHandle = MethodHandles.publicLookup().in(targetClass).unreflect(javaMethod); + methodHandle = MethodHandles.publicLookup().in(targetClass).unreflect(javaMethod); } catch (IllegalAccessException iae) { throw new IllegalArgumentException("static method handle [[" + targetClass.getCanonicalName() + "], " + "[" + methodName + "], " + typesToCanonicalTypeNames(typeParameters) + "] not found", iae); @@ -516,7 +519,7 @@ public class PainlessLookupBuilder { painlessMethod = painlessMethodCache.computeIfAbsent( new PainlessMethodCacheKey(targetClass, methodName, typeParameters), key -> new PainlessMethod(methodName, targetClass, null, returnType, - typeParameters, asmMethod, javaMethod.getModifiers(), javaMethodHandle)); + typeParameters, asmMethod, javaMethod.getModifiers(), methodHandle)); painlessClassBuilder.staticMethods.put(painlessMethodKey, painlessMethod); } else if ((painlessMethod.name.equals(methodName) && painlessMethod.rtn == returnType && @@ -535,18 +538,18 @@ public class PainlessLookupBuilder { if (painlessMethod == null) { org.objectweb.asm.commons.Method asmMethod = org.objectweb.asm.commons.Method.getMethod(javaMethod); - MethodHandle javaMethodHandle; + MethodHandle methodHandle; if (augmentedClass == null) { try { - javaMethodHandle = MethodHandles.publicLookup().in(targetClass).unreflect(javaMethod); + methodHandle = MethodHandles.publicLookup().in(targetClass).unreflect(javaMethod); } catch (IllegalAccessException iae) { throw new IllegalArgumentException("method handle [[" + targetClass.getCanonicalName() + "], " + "[" + methodName + "], " + typesToCanonicalTypeNames(typeParameters) + "] not found", iae); } } else { try { - javaMethodHandle = MethodHandles.publicLookup().in(augmentedClass).unreflect(javaMethod); + methodHandle = MethodHandles.publicLookup().in(augmentedClass).unreflect(javaMethod); } catch (IllegalAccessException iae) { throw new IllegalArgumentException("method handle [[" + targetClass.getCanonicalName() + "], " + "[" + methodName + "], " + typesToCanonicalTypeNames(typeParameters) + "] not found " + @@ -557,7 +560,7 @@ public class PainlessLookupBuilder { painlessMethod = painlessMethodCache.computeIfAbsent( new PainlessMethodCacheKey(targetClass, methodName, typeParameters), key -> new PainlessMethod(methodName, targetClass, augmentedClass, returnType, - typeParameters, asmMethod, javaMethod.getModifiers(), javaMethodHandle)); + typeParameters, asmMethod, javaMethod.getModifiers(), methodHandle)); painlessClassBuilder.methods.put(painlessMethodKey, painlessMethod); } else if ((painlessMethod.name.equals(methodName) && painlessMethod.rtn == returnType && @@ -650,7 +653,7 @@ public class PainlessLookupBuilder { throw new IllegalArgumentException("static field [[" + targetCanonicalClassName + "]. [" + fieldName + "]] must be final"); } - PainlessField painlessField = painlessClassBuilder.staticMembers.get(painlessFieldKey); + PainlessField painlessField = painlessClassBuilder.staticFields.get(painlessFieldKey); if (painlessField == null) { painlessField = painlessFieldCache.computeIfAbsent( @@ -658,7 +661,7 @@ public class PainlessLookupBuilder { key -> new PainlessField(fieldName, javaField.getName(), targetClass, typeParameter, javaField.getModifiers(), null, null)); - painlessClassBuilder.staticMembers.put(painlessFieldKey, painlessField); + painlessClassBuilder.staticFields.put(painlessFieldKey, painlessField); } else if (painlessField.clazz != typeParameter) { throw new IllegalArgumentException("cannot have static fields " + "[[" + targetCanonicalClassName + "], [" + fieldName + "], [" + @@ -674,7 +677,7 @@ public class PainlessLookupBuilder { methodHandleGetter = MethodHandles.publicLookup().unreflectGetter(javaField); } catch (IllegalAccessException iae) { throw new IllegalArgumentException( - "method handle getter not found for field [[" + targetCanonicalClassName + "], [" + fieldName + "]]"); + "getter method handle not found for field [[" + targetCanonicalClassName + "], [" + fieldName + "]]"); } MethodHandle methodHandleSetter; @@ -683,10 +686,10 @@ public class PainlessLookupBuilder { methodHandleSetter = MethodHandles.publicLookup().unreflectSetter(javaField); } catch (IllegalAccessException iae) { throw new IllegalArgumentException( - "method handle setter not found for field [[" + targetCanonicalClassName + "], [" + fieldName + "]]"); + "setter method handle not found for field [[" + targetCanonicalClassName + "], [" + fieldName + "]]"); } - PainlessField painlessField = painlessClassBuilder.members.get(painlessFieldKey); + PainlessField painlessField = painlessClassBuilder.fields.get(painlessFieldKey); if (painlessField == null) { painlessField = painlessFieldCache.computeIfAbsent( @@ -694,7 +697,7 @@ public class PainlessLookupBuilder { key -> new PainlessField(fieldName, javaField.getName(), targetClass, typeParameter, javaField.getModifiers(), methodHandleGetter, methodHandleSetter)); - painlessClassBuilder.members.put(fieldName, painlessField); + painlessClassBuilder.fields.put(fieldName, painlessField); } else if (painlessField.clazz != typeParameter) { throw new IllegalArgumentException("cannot have fields " + "[[" + targetCanonicalClassName + "], [" + fieldName + "], [" + @@ -771,14 +774,14 @@ public class PainlessLookupBuilder { } } - for (Map.Entry painlessFieldEntry : originalPainlessClassBuilder.members.entrySet()) { + for (Map.Entry painlessFieldEntry : originalPainlessClassBuilder.fields.entrySet()) { String painlessFieldKey = painlessFieldEntry.getKey(); PainlessField newPainlessField = painlessFieldEntry.getValue(); - PainlessField existingPainlessField = targetPainlessClassBuilder.members.get(painlessFieldKey); + PainlessField existingPainlessField = targetPainlessClassBuilder.fields.get(painlessFieldKey); if (existingPainlessField == null || existingPainlessField.target != newPainlessField.target && existingPainlessField.target.isAssignableFrom(newPainlessField.target)) { - targetPainlessClassBuilder.members.put(painlessFieldKey, newPainlessField); + targetPainlessClassBuilder.fields.put(painlessFieldKey, newPainlessField); } } } @@ -796,34 +799,32 @@ public class PainlessLookupBuilder { if (typeParametersSize == 0 && methodName.startsWith("get") && methodName.length() > 3 && Character.isUpperCase(methodName.charAt(3))) { - painlessClassBuilder.getters.putIfAbsent( + painlessClassBuilder.getterMethodHandles.putIfAbsent( Character.toLowerCase(methodName.charAt(3)) + methodName.substring(4), painlessMethod.handle); } else if (typeParametersSize == 0 && methodName.startsWith("is") && methodName.length() > 2 && Character.isUpperCase(methodName.charAt(2))) { - painlessClassBuilder.getters.putIfAbsent( + painlessClassBuilder.getterMethodHandles.putIfAbsent( Character.toLowerCase(methodName.charAt(2)) + methodName.substring(3), painlessMethod.handle); } else if (typeParametersSize == 1 && methodName.startsWith("set") && methodName.length() > 3 && Character.isUpperCase(methodName.charAt(3))) { - painlessClassBuilder.setters.putIfAbsent( + painlessClassBuilder.setterMethodHandles.putIfAbsent( Character.toLowerCase(methodName.charAt(3)) + methodName.substring(4), painlessMethod.handle); } } - for (PainlessField painlessField : painlessClassBuilder.members.values()) { - painlessClassBuilder.getters.put(painlessField.name, painlessField.getter); - painlessClassBuilder.setters.put(painlessField.name, painlessField.setter); + for (PainlessField painlessField : painlessClassBuilder.fields.values()) { + painlessClassBuilder.getterMethodHandles.put(painlessField.name, painlessField.getter); + painlessClassBuilder.setterMethodHandles.put(painlessField.name, painlessField.setter); } } private void setFunctionalInterfaceMethods() { for (Map.Entry, PainlessClassBuilder> painlessClassBuilderEntry : classesToPainlessClassBuilders.entrySet()) { - setFunctionalInterfaceMethod(painlessClassBuilderEntry.getValue()); + setFunctionalInterfaceMethod(painlessClassBuilderEntry.getKey(), painlessClassBuilderEntry.getValue()); } } - private void setFunctionalInterfaceMethod(PainlessClassBuilder painlessClassBuilder) { - Class targetClass = painlessClassBuilder.clazz; - + private void setFunctionalInterfaceMethod(Class targetClass, PainlessClassBuilder painlessClassBuilder) { if (targetClass.isInterface()) { List javaMethods = new ArrayList<>(); diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/ENewObj.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/ENewObj.java index c0d4433f7fb..f092a17c9fc 100644 --- a/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/ENewObj.java +++ b/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/ENewObj.java @@ -72,8 +72,9 @@ public final class ENewObj extends AExpression { constructor.arguments.toArray(types); if (constructor.arguments.size() != arguments.size()) { - throw createError(new IllegalArgumentException("When calling constructor on type [" + struct.name + "]" + - " expected [" + constructor.arguments.size() + "] arguments, but found [" + arguments.size() + "].")); + throw createError(new IllegalArgumentException( + "When calling constructor on type [" + PainlessLookupUtility.typeToCanonicalTypeName(actual) + "] " + + "expected [" + constructor.arguments.size() + "] arguments, but found [" + arguments.size() + "].")); } for (int argument = 0; argument < arguments.size(); ++argument) { @@ -87,7 +88,8 @@ public final class ENewObj extends AExpression { statement = true; } else { - throw createError(new IllegalArgumentException("Unknown new call on type [" + struct.name + "].")); + throw createError(new IllegalArgumentException( + "Unknown new call on type [" + PainlessLookupUtility.typeToCanonicalTypeName(actual) + "].")); } } diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/PBrace.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/PBrace.java index 7b55cb5a804..26471f67f65 100644 --- a/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/PBrace.java +++ b/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/PBrace.java @@ -63,9 +63,9 @@ public final class PBrace extends AStoreable { } else if (prefix.actual == def.class) { sub = new PSubDefArray(location, index); } else if (Map.class.isAssignableFrom(prefix.actual)) { - sub = new PSubMapShortcut(location, locals.getPainlessLookup().getPainlessStructFromJavaClass(prefix.actual), index); + sub = new PSubMapShortcut(location, prefix.actual, index); } else if (List.class.isAssignableFrom(prefix.actual)) { - sub = new PSubListShortcut(location, locals.getPainlessLookup().getPainlessStructFromJavaClass(prefix.actual), index); + sub = new PSubListShortcut(location, prefix.actual, index); } else { throw createError(new IllegalArgumentException("Illegal array access on type " + "[" + PainlessLookupUtility.typeToCanonicalTypeName(prefix.actual) + "].")); diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/PCallInvoke.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/PCallInvoke.java index 8fc8a612b84..56bc18eadbd 100644 --- a/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/PCallInvoke.java +++ b/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/PCallInvoke.java @@ -84,8 +84,8 @@ public final class PCallInvoke extends AExpression { } else if (prefix.actual == def.class) { sub = new PSubDefCall(location, name, arguments); } else { - throw createError(new IllegalArgumentException( - "Unknown call [" + name + "] with [" + arguments.size() + "] arguments on type [" + struct.name + "].")); + throw createError(new IllegalArgumentException("Unknown call [" + name + "] with [" + arguments.size() + "] arguments " + + "on type [" + PainlessLookupUtility.typeToCanonicalTypeName(prefix.actual) + "].")); } if (nullSafe) { diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/PField.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/PField.java index abf398d0e67..b322d5b1f28 100644 --- a/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/PField.java +++ b/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/PField.java @@ -68,7 +68,7 @@ public final class PField extends AStoreable { sub = new PSubDefField(location, value); } else { PainlessClass struct = locals.getPainlessLookup().getPainlessStructFromJavaClass(prefix.actual); - PainlessField field = prefix instanceof EStatic ? struct.staticMembers.get(value) : struct.members.get(value); + PainlessField field = prefix instanceof EStatic ? struct.staticFields.get(value) : struct.fields.get(value); if (field != null) { sub = new PSubField(location, field); @@ -92,11 +92,11 @@ public final class PField extends AStoreable { index.analyze(locals); if (Map.class.isAssignableFrom(prefix.actual)) { - sub = new PSubMapShortcut(location, struct, index); + sub = new PSubMapShortcut(location, prefix.actual, index); } if (List.class.isAssignableFrom(prefix.actual)) { - sub = new PSubListShortcut(location, struct, index); + sub = new PSubListShortcut(location, prefix.actual, index); } } } diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/PSubListShortcut.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/PSubListShortcut.java index 3841b1fece1..509aad64153 100644 --- a/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/PSubListShortcut.java +++ b/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/PSubListShortcut.java @@ -36,16 +36,16 @@ import java.util.Set; */ final class PSubListShortcut extends AStoreable { - private final PainlessClass struct; + private final Class targetClass; private AExpression index; private PainlessMethod getter; private PainlessMethod setter; - PSubListShortcut(Location location, PainlessClass struct, AExpression index) { + PSubListShortcut(Location location, Class targetClass, AExpression index) { super(location); - this.struct = Objects.requireNonNull(struct); + this.targetClass = Objects.requireNonNull(targetClass); this.index = Objects.requireNonNull(index); } @@ -56,16 +56,19 @@ final class PSubListShortcut extends AStoreable { @Override void analyze(Locals locals) { + PainlessClass struct = locals.getPainlessLookup().getPainlessStructFromJavaClass(targetClass); + String canonicalClassName = PainlessLookupUtility.typeToCanonicalTypeName(targetClass); + getter = struct.methods.get(PainlessLookupUtility.buildPainlessMethodKey("get", 1)); setter = struct.methods.get(PainlessLookupUtility.buildPainlessMethodKey("set", 2)); if (getter != null && (getter.rtn == void.class || getter.arguments.size() != 1 || getter.arguments.get(0) != int.class)) { - throw createError(new IllegalArgumentException("Illegal list get shortcut for type [" + struct.name + "].")); + throw createError(new IllegalArgumentException("Illegal list get shortcut for type [" + canonicalClassName + "].")); } if (setter != null && (setter.arguments.size() != 2 || setter.arguments.get(0) != int.class)) { - throw createError(new IllegalArgumentException("Illegal list set shortcut for type [" + struct.name + "].")); + throw createError(new IllegalArgumentException("Illegal list set shortcut for type [" + canonicalClassName + "].")); } if (getter != null && setter != null && (!getter.arguments.get(0).equals(setter.arguments.get(0)) @@ -80,7 +83,7 @@ final class PSubListShortcut extends AStoreable { actual = setter != null ? setter.arguments.get(1) : getter.rtn; } else { - throw createError(new IllegalArgumentException("Illegal list shortcut for type [" + struct.name + "].")); + throw createError(new IllegalArgumentException("Illegal list shortcut for type [" + canonicalClassName + "].")); } } diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/PSubMapShortcut.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/PSubMapShortcut.java index 13a3b9c9b94..2d7f2250c6c 100644 --- a/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/PSubMapShortcut.java +++ b/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/PSubMapShortcut.java @@ -35,16 +35,16 @@ import java.util.Set; */ final class PSubMapShortcut extends AStoreable { - private final PainlessClass struct; + private final Class targetClass; private AExpression index; private PainlessMethod getter; private PainlessMethod setter; - PSubMapShortcut(Location location, PainlessClass struct, AExpression index) { + PSubMapShortcut(Location location, Class targetClass, AExpression index) { super(location); - this.struct = Objects.requireNonNull(struct); + this.targetClass = Objects.requireNonNull(targetClass); this.index = Objects.requireNonNull(index); } @@ -55,15 +55,18 @@ final class PSubMapShortcut extends AStoreable { @Override void analyze(Locals locals) { + PainlessClass struct = locals.getPainlessLookup().getPainlessStructFromJavaClass(targetClass); + String canonicalClassName = PainlessLookupUtility.typeToCanonicalTypeName(targetClass); + getter = struct.methods.get(PainlessLookupUtility.buildPainlessMethodKey("get", 1)); setter = struct.methods.get(PainlessLookupUtility.buildPainlessMethodKey("put", 2)); if (getter != null && (getter.rtn == void.class || getter.arguments.size() != 1)) { - throw createError(new IllegalArgumentException("Illegal map get shortcut for type [" + struct.name + "].")); + throw createError(new IllegalArgumentException("Illegal map get shortcut for type [" + canonicalClassName + "].")); } if (setter != null && setter.arguments.size() != 2) { - throw createError(new IllegalArgumentException("Illegal map set shortcut for type [" + struct.name + "].")); + throw createError(new IllegalArgumentException("Illegal map set shortcut for type [" + canonicalClassName + "].")); } if (getter != null && setter != null && @@ -78,7 +81,7 @@ final class PSubMapShortcut extends AStoreable { actual = setter != null ? setter.arguments.get(1) : getter.rtn; } else { - throw createError(new IllegalArgumentException("Illegal map shortcut for type [" + struct.name + "].")); + throw createError(new IllegalArgumentException("Illegal map shortcut for type [" + canonicalClassName + "].")); } } diff --git a/modules/lang-painless/src/test/java/org/elasticsearch/painless/PainlessDocGenerator.java b/modules/lang-painless/src/test/java/org/elasticsearch/painless/PainlessDocGenerator.java index b2cc5e48ad8..ff0d4231175 100644 --- a/modules/lang-painless/src/test/java/org/elasticsearch/painless/PainlessDocGenerator.java +++ b/modules/lang-painless/src/test/java/org/elasticsearch/painless/PainlessDocGenerator.java @@ -29,6 +29,7 @@ import org.elasticsearch.painless.lookup.PainlessLookup; import org.elasticsearch.painless.lookup.PainlessLookupBuilder; import org.elasticsearch.painless.lookup.PainlessLookupUtility; import org.elasticsearch.painless.lookup.PainlessMethod; +import org.elasticsearch.painless.lookup.def; import org.elasticsearch.painless.spi.Whitelist; import java.io.IOException; @@ -71,52 +72,54 @@ public class PainlessDocGenerator { Files.newOutputStream(indexPath, StandardOpenOption.CREATE_NEW, StandardOpenOption.WRITE), false, StandardCharsets.UTF_8.name())) { emitGeneratedWarning(indexStream); - List structs = PAINLESS_LOOKUP.getStructs().stream().sorted(comparing(t -> t.name)).collect(toList()); - for (PainlessClass struct : structs) { - if (struct.clazz.isPrimitive()) { + List> classes = PAINLESS_LOOKUP.getStructs().stream().sorted(comparing(Class::getCanonicalName)).collect(toList()); + for (Class clazz : classes) { + PainlessClass struct = PAINLESS_LOOKUP.getPainlessStructFromJavaClass(clazz); + String canonicalClassName = PainlessLookupUtility.typeToCanonicalTypeName(clazz); + + if (clazz.isPrimitive()) { // Primitives don't have methods to reference continue; } - if ("def".equals(struct.name)) { + if (clazz == def.class) { // def is special but doesn't have any methods all of its own. continue; } indexStream.print("include::"); - indexStream.print(struct.name); + indexStream.print(canonicalClassName); indexStream.println(".asciidoc[]"); - Path typePath = apiRootPath.resolve(struct.name + ".asciidoc"); - logger.info("Writing [{}.asciidoc]", struct.name); + Path typePath = apiRootPath.resolve(canonicalClassName + ".asciidoc"); + logger.info("Writing [{}.asciidoc]", canonicalClassName); try (PrintStream typeStream = new PrintStream( Files.newOutputStream(typePath, StandardOpenOption.CREATE_NEW, StandardOpenOption.WRITE), false, StandardCharsets.UTF_8.name())) { emitGeneratedWarning(typeStream); typeStream.print("[["); - emitAnchor(typeStream, struct.clazz); + emitAnchor(typeStream, clazz); typeStream.print("]]++"); - typeStream.print(struct.name); + typeStream.print(canonicalClassName); typeStream.println("++::"); Consumer documentField = field -> PainlessDocGenerator.documentField(typeStream, field); Consumer documentMethod = method -> PainlessDocGenerator.documentMethod(typeStream, method); - struct.staticMembers.values().stream().sorted(FIELD_NAME).forEach(documentField); - struct.members.values().stream().sorted(FIELD_NAME).forEach(documentField); + struct.staticFields.values().stream().sorted(FIELD_NAME).forEach(documentField); + struct.fields.values().stream().sorted(FIELD_NAME).forEach(documentField); struct.staticMethods.values().stream().sorted(METHOD_NAME.thenComparing(NUMBER_OF_ARGS)).forEach(documentMethod); struct.constructors.values().stream().sorted(NUMBER_OF_ARGS).forEach(documentMethod); - Map inherited = new TreeMap<>(); + Map> inherited = new TreeMap<>(); struct.methods.values().stream().sorted(METHOD_NAME.thenComparing(NUMBER_OF_ARGS)).forEach(method -> { - if (method.target == struct.clazz) { + if (method.target == clazz) { documentMethod(typeStream, method); } else { - PainlessClass painlessClass = PAINLESS_LOOKUP.getPainlessStructFromJavaClass(method.target); - inherited.put(painlessClass.name, painlessClass); + inherited.put(canonicalClassName, method.target); } }); if (false == inherited.isEmpty()) { typeStream.print("* Inherits methods from "); boolean first = true; - for (PainlessClass inheritsFrom : inherited.values()) { + for (Class inheritsFrom : inherited.values()) { if (first) { first = false; } else { @@ -242,7 +245,7 @@ public class PainlessDocGenerator { an internal link with the text. */ private static void emitType(PrintStream stream, Class clazz) { - emitStruct(stream, PAINLESS_LOOKUP.getPainlessStructFromJavaClass(clazz)); + emitStruct(stream, clazz); while ((clazz = clazz.getComponentType()) != null) { stream.print("[]"); } @@ -252,15 +255,17 @@ public class PainlessDocGenerator { * Emit a {@link PainlessClass}. If the {@linkplain PainlessClass} is primitive or def this just emits the name of the struct. * Otherwise this emits an internal link with the name. */ - private static void emitStruct(PrintStream stream, PainlessClass struct) { - if (false == struct.clazz.isPrimitive() && false == struct.name.equals("def")) { + private static void emitStruct(PrintStream stream, Class clazz) { + String canonicalClassName = PainlessLookupUtility.typeToCanonicalTypeName(clazz); + + if (false == clazz.isPrimitive() && clazz != def.class) { stream.print("<<"); - emitAnchor(stream, struct.clazz); + emitAnchor(stream, clazz); stream.print(','); - stream.print(struct.name); + stream.print(canonicalClassName); stream.print(">>"); } else { - stream.print(struct.name); + stream.print(canonicalClassName); } } diff --git a/modules/lang-painless/src/test/java/org/elasticsearch/painless/node/NodeToStringTests.java b/modules/lang-painless/src/test/java/org/elasticsearch/painless/node/NodeToStringTests.java index 84452b4843d..c64014d81a5 100644 --- a/modules/lang-painless/src/test/java/org/elasticsearch/painless/node/NodeToStringTests.java +++ b/modules/lang-painless/src/test/java/org/elasticsearch/painless/node/NodeToStringTests.java @@ -460,7 +460,7 @@ public class NodeToStringTests extends ESTestCase { public void testPSubField() { Location l = new Location(getTestName(), 0); PainlessClass s = painlessLookup.getPainlessStructFromJavaClass(Boolean.class); - PainlessField f = s.staticMembers.get("TRUE"); + PainlessField f = s.staticFields.get("TRUE"); PSubField node = new PSubField(l, f); node.prefix = new EStatic(l, "Boolean"); assertEquals("(PSubField (EStatic Boolean) TRUE)", node.toString()); @@ -469,32 +469,28 @@ public class NodeToStringTests extends ESTestCase { public void testPSubListShortcut() { Location l = new Location(getTestName(), 0); - PainlessClass s = painlessLookup.getPainlessStructFromJavaClass(List.class); - PSubListShortcut node = new PSubListShortcut(l, s, new EConstant(l, 1)); + PSubListShortcut node = new PSubListShortcut(l, List.class, new EConstant(l, 1)); node.prefix = new EVariable(l, "a"); assertEquals("(PSubListShortcut (EVariable a) (EConstant Integer 1))", node.toString()); assertEquals("(PSubNullSafeCallInvoke (PSubListShortcut (EVariable a) (EConstant Integer 1)))", new PSubNullSafeCallInvoke(l, node).toString()); l = new Location(getTestName(), 0); - s = painlessLookup.getPainlessStructFromJavaClass(List.class); - node = new PSubListShortcut(l, s, new EBinary(l, Operation.ADD, new EConstant(l, 1), new EConstant(l, 4))); + node = new PSubListShortcut(l, List.class, new EBinary(l, Operation.ADD, new EConstant(l, 1), new EConstant(l, 4))); node.prefix = new EVariable(l, "a"); assertEquals("(PSubListShortcut (EVariable a) (EBinary (EConstant Integer 1) + (EConstant Integer 4)))", node.toString()); } public void testPSubMapShortcut() { Location l = new Location(getTestName(), 0); - PainlessClass s = painlessLookup.getPainlessStructFromJavaClass(Map.class); - PSubMapShortcut node = new PSubMapShortcut(l, s, new EConstant(l, "cat")); + PSubMapShortcut node = new PSubMapShortcut(l, Map.class, new EConstant(l, "cat")); node.prefix = new EVariable(l, "a"); assertEquals("(PSubMapShortcut (EVariable a) (EConstant String 'cat'))", node.toString()); assertEquals("(PSubNullSafeCallInvoke (PSubMapShortcut (EVariable a) (EConstant String 'cat')))", new PSubNullSafeCallInvoke(l, node).toString()); l = new Location(getTestName(), 1); - s = painlessLookup.getPainlessStructFromJavaClass(Map.class); - node = new PSubMapShortcut(l, s, new EBinary(l, Operation.ADD, new EConstant(l, 1), new EConstant(l, 4))); + node = new PSubMapShortcut(l, Map.class, new EBinary(l, Operation.ADD, new EConstant(l, 1), new EConstant(l, 4))); node.prefix = new EVariable(l, "a"); assertEquals("(PSubMapShortcut (EVariable a) (EBinary (EConstant Integer 1) + (EConstant Integer 4)))", node.toString()); }