From 298592013494f311ac1f1358bcd816322feb2122 Mon Sep 17 00:00:00 2001 From: Jack Conradson Date: Wed, 1 Aug 2018 09:28:18 -0700 Subject: [PATCH] Painless: Clean Up PainlessField (#32525) Updates PainlessField variable names to current naming scheme and removes extraneous variables. --- .../painless/lookup/PainlessField.java | 28 +++++----- .../lookup/PainlessLookupBuilder.java | 51 +++++++++---------- .../painless/node/PSubField.java | 34 ++++++++----- .../painless/PainlessDocGenerator.java | 18 +++---- 4 files changed, 66 insertions(+), 65 deletions(-) diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/lookup/PainlessField.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/lookup/PainlessField.java index f316e1438ec..a55d6c3730e 100644 --- a/modules/lang-painless/src/main/java/org/elasticsearch/painless/lookup/PainlessField.java +++ b/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; } } 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 a4dbe1006d6..a0cab7f1a5b 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 @@ -679,11 +679,20 @@ public final class PainlessLookupBuilder { "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); @@ -691,28 +700,18 @@ public final class PainlessLookupBuilder { 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 final class PainlessLookupBuilder { 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 @@ public final class PainlessLookupBuilder { 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 @@ public final class PainlessLookupBuilder { } 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); } } diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/PSubField.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/PSubField.java index 007a599e9f8..9e09f810250 100644 --- a/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/PSubField.java +++ b/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/PSubField.java @@ -51,22 +51,24 @@ final class PSubField extends AStoreable { @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,10 +96,12 @@ final class PSubField extends AStoreable { 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)); } } @@ -105,15 +109,17 @@ final class PSubField extends AStoreable { 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()); } } 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 1f8410092a0..ad29d702177 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 @@ -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 FIELD_NAME = comparing(f -> f.name); + private static final Comparator FIELD_NAME = comparing(f -> f.javaField.getName()); private static final Comparator METHOD_NAME = comparing(m -> m.javaMethod.getName()); private static final Comparator METHOD_NUMBER_OF_PARAMS = comparing(m -> m.typeParameters.size()); private static final Comparator CONSTRUCTOR_NUMBER_OF_PARAMS = comparing(m -> m.typeParameters.size()); @@ -147,17 +147,17 @@ public class PainlessDocGenerator { 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 @@ public class PainlessDocGenerator { * 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 @@ public class PainlessDocGenerator { 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 @@ public class PainlessDocGenerator { * Pick the javadoc root for a {@link PainlessField}. */ private static String javadocRoot(PainlessField field) { - return javadocRoot(field.target); + return javadocRoot(field.javaField.getDeclaringClass()); } /**