Skip to content

Commit

Permalink
Painless: Clean Up PainlessField (#32525)
Browse files Browse the repository at this point in the history 
Updates PainlessField variable names to current naming scheme and removes 
extraneous variables.
  • Loading branch information
@jdconrad
jdconrad authored Aug 1, 2018
1 parent 2db420a commit 2985920
Show file tree
Hide file tree
Showing 4 changed files with 66 additions and 65 deletions.
28 changes: 12 additions & 16 deletions modules/lang-painless/src/main/java/org/elasticsearch/painless/lookup/PainlessField.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}
51 changes: 25 additions & 26 deletions .../lang-painless/src/main/java/org/elasticsearch/painless/lookup/PainlessLookupBuilder.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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");
}
}
}
Expand Down Expand Up @@ -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);
}
}
Expand Down Expand Up @@ -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);
}
}

Expand Down
34 changes: 20 additions & 14 deletions modules/lang-painless/src/main/java/org/elasticsearch/painless/node/PSubField.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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));
}
}

Expand Down Expand Up @@ -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());
}
}
18 changes: 9 additions & 9 deletions modules/lang-painless/src/test/java/org/elasticsearch/painless/PainlessDocGenerator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand Down Expand Up @@ -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")) {
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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());
}

/**
Expand All @@ -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());
}

/**
Expand Down

0 comments on commit 2985920

Please sign in to comment.