Painless: Clean Up PainlessField

modules/lang-painless/src/main/java/org/elasticsearch/painless/lookup/PainlessField.java

@@ -20,24 +20,20 @@
 package org.elasticsearch.painless.lookup;
 
 import java.lang.invoke.MethodHandle;
+import java.lang.reflect.Field;
 
 public final class PainlessField {
-    public final String name;
-    public final Class<?> target;
-    public final Class<?> clazz;
-    public final String javaName;
-    public final int modifiers;
-    public final MethodHandle getter;
-    public final MethodHandle setter;
+    public final Field javaField;
+    public final Class<?> typeParameter;
 
-    PainlessField(String name, String javaName, Class<?> target, Class<?> clazz, int modifiers,
-                  MethodHandle getter, MethodHandle setter) {
-        this.name = name;
-        this.javaName = javaName;
-        this.target = target;
-        this.clazz = clazz;
-        this.modifiers = modifiers;
-        this.getter = getter;
-        this.setter = setter;
+    public final MethodHandle getterMethodHandle;
+    public final MethodHandle setterMethodHandle;
+
+    PainlessField(Field javaField, Class<?> typeParameter, MethodHandle getterMethodHandle, MethodHandle setterMethodHandle) {
+        this.javaField = javaField;
+        this.typeParameter = typeParameter;
+
+        this.getterMethodHandle = getterMethodHandle;
+        this.setterMethodHandle = setterMethodHandle;
     }
 }

modules/lang-painless/src/main/java/org/elasticsearch/painless/lookup/PainlessLookupBuilder.java

@@ -679,40 +679,39 @@ public void addPainlessField(Class<?> targetClass, String fieldName, Class<?> ty
                     "for field [[" + targetCanonicalClassName + "], [" + fieldName + "]");
         }
 
+        MethodHandle methodHandleGetter;
+
+        try {
+            methodHandleGetter = MethodHandles.publicLookup().unreflectGetter(javaField);
+        } catch (IllegalAccessException iae) {
+            throw new IllegalArgumentException(
+                    "getter method handle not found for field [[" + targetCanonicalClassName + "], [" + fieldName + "]]");
+        }
+
         String painlessFieldKey = buildPainlessFieldKey(fieldName);
 
         if (Modifier.isStatic(javaField.getModifiers())) {
             if (Modifier.isFinal(javaField.getModifiers()) == false) {
-                throw new IllegalArgumentException("static field [[" + targetCanonicalClassName + "]. [" + fieldName + "]] must be final");
+                throw new IllegalArgumentException("static field [[" + targetCanonicalClassName + "], [" + fieldName + "]] must be final");
             }
 
             PainlessField painlessField = painlessClassBuilder.staticFields.get(painlessFieldKey);
 
             if (painlessField == null) {
                 painlessField = painlessFieldCache.computeIfAbsent(
                         new PainlessFieldCacheKey(targetClass, fieldName, typeParameter),
-                        key -> new PainlessField(fieldName, javaField.getName(), targetClass,
-                                typeParameter, javaField.getModifiers(), null, null));
+                        key -> new PainlessField(javaField, typeParameter, methodHandleGetter, null));
 
                 painlessClassBuilder.staticFields.put(painlessFieldKey, painlessField);
-            } else if (painlessField.clazz != typeParameter) {
+            } else if (painlessField.typeParameter != typeParameter) {
                 throw new IllegalArgumentException("cannot have static fields " +
                         "[[" + targetCanonicalClassName + "], [" + fieldName + "], [" +
                         typeToCanonicalTypeName(typeParameter) + "] and " +
-                        "[[" + targetCanonicalClassName + "], [" + painlessField.name + "], " +
-                        typeToCanonicalTypeName(painlessField.clazz) + "] " +
-                        "with the same and different type parameters");
+                        "[[" + targetCanonicalClassName + "], [" + painlessField.javaField.getName() + "], " +
+                        typeToCanonicalTypeName(painlessField.typeParameter) + "] " +
+                        "with the same name and different type parameters");
             }
         } else {
-            MethodHandle methodHandleGetter;
-
-            try {
-                methodHandleGetter = MethodHandles.publicLookup().unreflectGetter(javaField);
-            } catch (IllegalAccessException iae) {
-                throw new IllegalArgumentException(
-                        "getter method handle not found for field [[" + targetCanonicalClassName + "], [" + fieldName + "]]");
-            }
-
             MethodHandle methodHandleSetter;
 
             try {
@@ -727,17 +726,16 @@ public void addPainlessField(Class<?> targetClass, String fieldName, Class<?> ty
             if (painlessField == null) {
                 painlessField = painlessFieldCache.computeIfAbsent(
                         new PainlessFieldCacheKey(targetClass, painlessFieldKey, typeParameter),
-                        key -> new PainlessField(fieldName, javaField.getName(), targetClass,
-                                typeParameter, javaField.getModifiers(), methodHandleGetter, methodHandleSetter));
+                        key -> new PainlessField(javaField, typeParameter, methodHandleGetter, methodHandleSetter));
 
                 painlessClassBuilder.fields.put(fieldName, painlessField);
-            } else if (painlessField.clazz != typeParameter) {
+            } else if (painlessField.typeParameter != typeParameter) {
                 throw new IllegalArgumentException("cannot have fields " +
                         "[[" + targetCanonicalClassName + "], [" + fieldName + "], [" +
                         typeToCanonicalTypeName(typeParameter) + "] and " +
-                        "[[" + targetCanonicalClassName + "], [" + painlessField.name + "], " +
-                        typeToCanonicalTypeName(painlessField.clazz) + "] " +
-                        "with the same and different type parameters");
+                        "[[" + targetCanonicalClassName + "], [" + painlessField.javaField.getName() + "], " +
+                        typeToCanonicalTypeName(painlessField.typeParameter) + "] " +
+                        "with the same name and different type parameters");
             }
         }
     }
@@ -812,8 +810,9 @@ private void copyPainlessClassMembers(Class<?> originalClass, Class<?> targetCla
             PainlessField newPainlessField = painlessFieldEntry.getValue();
             PainlessField existingPainlessField = targetPainlessClassBuilder.fields.get(painlessFieldKey);
 
-            if (existingPainlessField == null || existingPainlessField.target != newPainlessField.target &&
-                    existingPainlessField.target.isAssignableFrom(newPainlessField.target)) {
+            if (existingPainlessField == null ||
+                    existingPainlessField.javaField.getDeclaringClass() != newPainlessField.javaField.getDeclaringClass() &&
+                    existingPainlessField.javaField.getDeclaringClass().isAssignableFrom(newPainlessField.javaField.getDeclaringClass())) {
                 targetPainlessClassBuilder.fields.put(painlessFieldKey, newPainlessField);
             }
         }
@@ -846,8 +845,8 @@ private void cacheRuntimeHandles(PainlessClassBuilder painlessClassBuilder) {
         }
 
         for (PainlessField painlessField : painlessClassBuilder.fields.values()) {
-            painlessClassBuilder.getterMethodHandles.put(painlessField.name, painlessField.getter);
-            painlessClassBuilder.setterMethodHandles.put(painlessField.name, painlessField.setter);
+            painlessClassBuilder.getterMethodHandles.put(painlessField.javaField.getName(), painlessField.getterMethodHandle);
+            painlessClassBuilder.setterMethodHandles.put(painlessField.javaField.getName(), painlessField.setterMethodHandle);
         }
     }
 

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

@@ -51,22 +51,24 @@ void extractVariables(Set<String> variables) {
 
     @Override
     void analyze(Locals locals) {
-         if (write && Modifier.isFinal(field.modifiers)) {
-             throw createError(new IllegalArgumentException("Cannot write to read-only field [" + field.name + "] for type " +
-                     "[" + PainlessLookupUtility.typeToCanonicalTypeName(field.clazz) + "]."));
+         if (write && Modifier.isFinal(field.javaField.getModifiers())) {
+             throw createError(new IllegalArgumentException("Cannot write to read-only field [" + field.javaField.getName() + "] " +
+                     "for type [" + PainlessLookupUtility.typeToCanonicalTypeName(field.javaField.getDeclaringClass()) + "]."));
          }
 
-        actual = field.clazz;
+        actual = field.typeParameter;
     }
 
     @Override
     void write(MethodWriter writer, Globals globals) {
         writer.writeDebugInfo(location);
 
-        if (java.lang.reflect.Modifier.isStatic(field.modifiers)) {
-            writer.getStatic(Type.getType(field.target), field.javaName, MethodWriter.getType(field.clazz));
+        if (java.lang.reflect.Modifier.isStatic(field.javaField.getModifiers())) {
+            writer.getStatic(Type.getType(
+                    field.javaField.getDeclaringClass()), field.javaField.getName(), MethodWriter.getType(field.typeParameter));
         } else {
-            writer.getField(Type.getType(field.target), field.javaName, MethodWriter.getType(field.clazz));
+            writer.getField(Type.getType(
+                    field.javaField.getDeclaringClass()), field.javaField.getName(), MethodWriter.getType(field.typeParameter));
         }
     }
 
@@ -94,26 +96,30 @@ void setup(MethodWriter writer, Globals globals) {
     void load(MethodWriter writer, Globals globals) {
         writer.writeDebugInfo(location);
 
-        if (java.lang.reflect.Modifier.isStatic(field.modifiers)) {
-            writer.getStatic(Type.getType(field.target), field.javaName, MethodWriter.getType(field.clazz));
+        if (java.lang.reflect.Modifier.isStatic(field.javaField.getModifiers())) {
+            writer.getStatic(Type.getType(
+                    field.javaField.getDeclaringClass()), field.javaField.getName(), MethodWriter.getType(field.typeParameter));
         } else {
-            writer.getField(Type.getType(field.target), field.javaName, MethodWriter.getType(field.clazz));
+            writer.getField(Type.getType(
+                    field.javaField.getDeclaringClass()), field.javaField.getName(), MethodWriter.getType(field.typeParameter));
         }
     }
 
     @Override
     void store(MethodWriter writer, Globals globals) {
         writer.writeDebugInfo(location);
 
-        if (java.lang.reflect.Modifier.isStatic(field.modifiers)) {
-            writer.putStatic(Type.getType(field.target), field.javaName, MethodWriter.getType(field.clazz));
+        if (java.lang.reflect.Modifier.isStatic(field.javaField.getModifiers())) {
+            writer.putStatic(Type.getType(
+                    field.javaField.getDeclaringClass()), field.javaField.getName(), MethodWriter.getType(field.typeParameter));
         } else {
-            writer.putField(Type.getType(field.target), field.javaName, MethodWriter.getType(field.clazz));
+            writer.putField(Type.getType(
+                    field.javaField.getDeclaringClass()), field.javaField.getName(), MethodWriter.getType(field.typeParameter));
         }
     }
 
     @Override
     public String toString() {
-        return singleLineToString(prefix, field.name);
+        return singleLineToString(prefix, field.javaField.getName());
     }
 }

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

@@ -56,7 +56,7 @@ public class PainlessDocGenerator {
 
     private static final PainlessLookup PAINLESS_LOOKUP = PainlessLookupBuilder.buildFromWhitelists(Whitelist.BASE_WHITELISTS);
     private static final Logger logger = ESLoggerFactory.getLogger(PainlessDocGenerator.class);
-    private static final Comparator<PainlessField> FIELD_NAME = comparing(f -> f.name);
+    private static final Comparator<PainlessField> FIELD_NAME = comparing(f -> f.javaField.getName());
     private static final Comparator<PainlessMethod> METHOD_NAME = comparing(m -> m.javaMethod.getName());
     private static final Comparator<PainlessMethod> METHOD_NUMBER_OF_PARAMS = comparing(m -> m.typeParameters.size());
     private static final Comparator<PainlessConstructor> CONSTRUCTOR_NUMBER_OF_PARAMS = comparing(m -> m.typeParameters.size());
@@ -147,17 +147,17 @@ private static void documentField(PrintStream stream, PainlessField field) {
         emitAnchor(stream, field);
         stream.print("]]");
 
-        if (Modifier.isStatic(field.modifiers)) {
+        if (Modifier.isStatic(field.javaField.getModifiers())) {
             stream.print("static ");
         }
 
-        emitType(stream, field.clazz);
+        emitType(stream, field.typeParameter);
         stream.print(' ');
 
         String javadocRoot = javadocRoot(field);
         emitJavadocLink(stream, javadocRoot, field);
         stream.print('[');
-        stream.print(field.name);
+        stream.print(field.javaField.getName());
         stream.print(']');
 
         if (javadocRoot.equals("java8")) {
@@ -280,9 +280,9 @@ private static void emitAnchor(PrintStream stream, PainlessMethod method) {
      * Anchor text for a {@link PainlessField}.
      */
     private static void emitAnchor(PrintStream stream, PainlessField field) {
-        emitAnchor(stream, field.target);
+        emitAnchor(stream, field.javaField.getDeclaringClass());
         stream.print('-');
-        stream.print(field.name);
+        stream.print(field.javaField.getName());
     }
 
     private static String constructorName(PainlessConstructor constructor) {
@@ -391,9 +391,9 @@ private static void emitJavadocLink(PrintStream stream, String root, PainlessFie
         stream.print("link:{");
         stream.print(root);
         stream.print("-javadoc}/");
-        stream.print(classUrlPath(field.target));
+        stream.print(classUrlPath(field.javaField.getDeclaringClass()));
         stream.print(".html#");
-        stream.print(field.javaName);
+        stream.print(field.javaField.getName());
     }
 
     /**
@@ -410,7 +410,7 @@ private static String javadocRoot(PainlessMethod method) {
      * Pick the javadoc root for a {@link PainlessField}.
      */
     private static String javadocRoot(PainlessField field) {
-        return javadocRoot(field.target);
+        return javadocRoot(field.javaField.getDeclaringClass());
     }
 
     /**