Skip to content

Commit fed37da

Browse files
jdconradjdconrad
committed
Painless: Clean Up PainlessField (#32525)
Updates PainlessField variable names to current naming scheme and removes extraneous variables.
1 parent 887f284 commit fed37da

File tree

4 files changed

+66
-65
lines changed

4 files changed

+66
-65
lines changed

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

Lines changed: 12 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -20,24 +20,20 @@
2020
package org.elasticsearch.painless.lookup;
2121

2222
import java.lang.invoke.MethodHandle;
23+
import java.lang.reflect.Field;
2324

2425
public final class PainlessField {
25-
public final String name;
26-
public final Class<?> target;
27-
public final Class<?> clazz;
28-
public final String javaName;
29-
public final int modifiers;
30-
public final MethodHandle getter;
31-
public final MethodHandle setter;
26+
public final Field javaField;
27+
public final Class<?> typeParameter;
3228

33-
PainlessField(String name, String javaName, Class<?> target, Class<?> clazz, int modifiers,
34-
MethodHandle getter, MethodHandle setter) {
35-
this.name = name;
36-
this.javaName = javaName;
37-
this.target = target;
38-
this.clazz = clazz;
39-
this.modifiers = modifiers;
40-
this.getter = getter;
41-
this.setter = setter;
29+
public final MethodHandle getterMethodHandle;
30+
public final MethodHandle setterMethodHandle;
31+
32+
PainlessField(Field javaField, Class<?> typeParameter, MethodHandle getterMethodHandle, MethodHandle setterMethodHandle) {
33+
this.javaField = javaField;
34+
this.typeParameter = typeParameter;
35+
36+
this.getterMethodHandle = getterMethodHandle;
37+
this.setterMethodHandle = setterMethodHandle;
4238
}
4339
}

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

Lines changed: 25 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -679,40 +679,39 @@ public void addPainlessField(Class<?> targetClass, String fieldName, Class<?> ty
679679
"for field [[" + targetCanonicalClassName + "], [" + fieldName + "]");
680680
}
681681

682+
MethodHandle methodHandleGetter;
683+
684+
try {
685+
methodHandleGetter = MethodHandles.publicLookup().unreflectGetter(javaField);
686+
} catch (IllegalAccessException iae) {
687+
throw new IllegalArgumentException(
688+
"getter method handle not found for field [[" + targetCanonicalClassName + "], [" + fieldName + "]]");
689+
}
690+
682691
String painlessFieldKey = buildPainlessFieldKey(fieldName);
683692

684693
if (Modifier.isStatic(javaField.getModifiers())) {
685694
if (Modifier.isFinal(javaField.getModifiers()) == false) {
686-
throw new IllegalArgumentException("static field [[" + targetCanonicalClassName + "]. [" + fieldName + "]] must be final");
695+
throw new IllegalArgumentException("static field [[" + targetCanonicalClassName + "], [" + fieldName + "]] must be final");
687696
}
688697

689698
PainlessField painlessField = painlessClassBuilder.staticFields.get(painlessFieldKey);
690699

691700
if (painlessField == null) {
692701
painlessField = painlessFieldCache.computeIfAbsent(
693702
new PainlessFieldCacheKey(targetClass, fieldName, typeParameter),
694-
key -> new PainlessField(fieldName, javaField.getName(), targetClass,
695-
typeParameter, javaField.getModifiers(), null, null));
703+
key -> new PainlessField(javaField, typeParameter, methodHandleGetter, null));
696704

697705
painlessClassBuilder.staticFields.put(painlessFieldKey, painlessField);
698-
} else if (painlessField.clazz != typeParameter) {
706+
} else if (painlessField.typeParameter != typeParameter) {
699707
throw new IllegalArgumentException("cannot have static fields " +
700708
"[[" + targetCanonicalClassName + "], [" + fieldName + "], [" +
701709
typeToCanonicalTypeName(typeParameter) + "] and " +
702-
"[[" + targetCanonicalClassName + "], [" + painlessField.name + "], " +
703-
typeToCanonicalTypeName(painlessField.clazz) + "] " +
704-
"with the same and different type parameters");
710+
"[[" + targetCanonicalClassName + "], [" + painlessField.javaField.getName() + "], " +
711+
typeToCanonicalTypeName(painlessField.typeParameter) + "] " +
712+
"with the same name and different type parameters");
705713
}
706714
} else {
707-
MethodHandle methodHandleGetter;
708-
709-
try {
710-
methodHandleGetter = MethodHandles.publicLookup().unreflectGetter(javaField);
711-
} catch (IllegalAccessException iae) {
712-
throw new IllegalArgumentException(
713-
"getter method handle not found for field [[" + targetCanonicalClassName + "], [" + fieldName + "]]");
714-
}
715-
716715
MethodHandle methodHandleSetter;
717716

718717
try {
@@ -727,17 +726,16 @@ public void addPainlessField(Class<?> targetClass, String fieldName, Class<?> ty
727726
if (painlessField == null) {
728727
painlessField = painlessFieldCache.computeIfAbsent(
729728
new PainlessFieldCacheKey(targetClass, painlessFieldKey, typeParameter),
730-
key -> new PainlessField(fieldName, javaField.getName(), targetClass,
731-
typeParameter, javaField.getModifiers(), methodHandleGetter, methodHandleSetter));
729+
key -> new PainlessField(javaField, typeParameter, methodHandleGetter, methodHandleSetter));
732730

733731
painlessClassBuilder.fields.put(fieldName, painlessField);
734-
} else if (painlessField.clazz != typeParameter) {
732+
} else if (painlessField.typeParameter != typeParameter) {
735733
throw new IllegalArgumentException("cannot have fields " +
736734
"[[" + targetCanonicalClassName + "], [" + fieldName + "], [" +
737735
typeToCanonicalTypeName(typeParameter) + "] and " +
738-
"[[" + targetCanonicalClassName + "], [" + painlessField.name + "], " +
739-
typeToCanonicalTypeName(painlessField.clazz) + "] " +
740-
"with the same and different type parameters");
736+
"[[" + targetCanonicalClassName + "], [" + painlessField.javaField.getName() + "], " +
737+
typeToCanonicalTypeName(painlessField.typeParameter) + "] " +
738+
"with the same name and different type parameters");
741739
}
742740
}
743741
}
@@ -812,8 +810,9 @@ private void copyPainlessClassMembers(Class<?> originalClass, Class<?> targetCla
812810
PainlessField newPainlessField = painlessFieldEntry.getValue();
813811
PainlessField existingPainlessField = targetPainlessClassBuilder.fields.get(painlessFieldKey);
814812

815-
if (existingPainlessField == null || existingPainlessField.target != newPainlessField.target &&
816-
existingPainlessField.target.isAssignableFrom(newPainlessField.target)) {
813+
if (existingPainlessField == null ||
814+
existingPainlessField.javaField.getDeclaringClass() != newPainlessField.javaField.getDeclaringClass() &&
815+
existingPainlessField.javaField.getDeclaringClass().isAssignableFrom(newPainlessField.javaField.getDeclaringClass())) {
817816
targetPainlessClassBuilder.fields.put(painlessFieldKey, newPainlessField);
818817
}
819818
}
@@ -846,8 +845,8 @@ private void cacheRuntimeHandles(PainlessClassBuilder painlessClassBuilder) {
846845
}
847846

848847
for (PainlessField painlessField : painlessClassBuilder.fields.values()) {
849-
painlessClassBuilder.getterMethodHandles.put(painlessField.name, painlessField.getter);
850-
painlessClassBuilder.setterMethodHandles.put(painlessField.name, painlessField.setter);
848+
painlessClassBuilder.getterMethodHandles.put(painlessField.javaField.getName(), painlessField.getterMethodHandle);
849+
painlessClassBuilder.setterMethodHandles.put(painlessField.javaField.getName(), painlessField.setterMethodHandle);
851850
}
852851
}
853852

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

Lines changed: 20 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -51,22 +51,24 @@ void extractVariables(Set<String> variables) {
5151

5252
@Override
5353
void analyze(Locals locals) {
54-
if (write && Modifier.isFinal(field.modifiers)) {
55-
throw createError(new IllegalArgumentException("Cannot write to read-only field [" + field.name + "] for type " +
56-
"[" + PainlessLookupUtility.typeToCanonicalTypeName(field.clazz) + "]."));
54+
if (write && Modifier.isFinal(field.javaField.getModifiers())) {
55+
throw createError(new IllegalArgumentException("Cannot write to read-only field [" + field.javaField.getName() + "] " +
56+
"for type [" + PainlessLookupUtility.typeToCanonicalTypeName(field.javaField.getDeclaringClass()) + "]."));
5757
}
5858

59-
actual = field.clazz;
59+
actual = field.typeParameter;
6060
}
6161

6262
@Override
6363
void write(MethodWriter writer, Globals globals) {
6464
writer.writeDebugInfo(location);
6565

66-
if (java.lang.reflect.Modifier.isStatic(field.modifiers)) {
67-
writer.getStatic(Type.getType(field.target), field.javaName, MethodWriter.getType(field.clazz));
66+
if (java.lang.reflect.Modifier.isStatic(field.javaField.getModifiers())) {
67+
writer.getStatic(Type.getType(
68+
field.javaField.getDeclaringClass()), field.javaField.getName(), MethodWriter.getType(field.typeParameter));
6869
} else {
69-
writer.getField(Type.getType(field.target), field.javaName, MethodWriter.getType(field.clazz));
70+
writer.getField(Type.getType(
71+
field.javaField.getDeclaringClass()), field.javaField.getName(), MethodWriter.getType(field.typeParameter));
7072
}
7173
}
7274

@@ -94,26 +96,30 @@ void setup(MethodWriter writer, Globals globals) {
9496
void load(MethodWriter writer, Globals globals) {
9597
writer.writeDebugInfo(location);
9698

97-
if (java.lang.reflect.Modifier.isStatic(field.modifiers)) {
98-
writer.getStatic(Type.getType(field.target), field.javaName, MethodWriter.getType(field.clazz));
99+
if (java.lang.reflect.Modifier.isStatic(field.javaField.getModifiers())) {
100+
writer.getStatic(Type.getType(
101+
field.javaField.getDeclaringClass()), field.javaField.getName(), MethodWriter.getType(field.typeParameter));
99102
} else {
100-
writer.getField(Type.getType(field.target), field.javaName, MethodWriter.getType(field.clazz));
103+
writer.getField(Type.getType(
104+
field.javaField.getDeclaringClass()), field.javaField.getName(), MethodWriter.getType(field.typeParameter));
101105
}
102106
}
103107

104108
@Override
105109
void store(MethodWriter writer, Globals globals) {
106110
writer.writeDebugInfo(location);
107111

108-
if (java.lang.reflect.Modifier.isStatic(field.modifiers)) {
109-
writer.putStatic(Type.getType(field.target), field.javaName, MethodWriter.getType(field.clazz));
112+
if (java.lang.reflect.Modifier.isStatic(field.javaField.getModifiers())) {
113+
writer.putStatic(Type.getType(
114+
field.javaField.getDeclaringClass()), field.javaField.getName(), MethodWriter.getType(field.typeParameter));
110115
} else {
111-
writer.putField(Type.getType(field.target), field.javaName, MethodWriter.getType(field.clazz));
116+
writer.putField(Type.getType(
117+
field.javaField.getDeclaringClass()), field.javaField.getName(), MethodWriter.getType(field.typeParameter));
112118
}
113119
}
114120

115121
@Override
116122
public String toString() {
117-
return singleLineToString(prefix, field.name);
123+
return singleLineToString(prefix, field.javaField.getName());
118124
}
119125
}

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

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ public class PainlessDocGenerator {
5656

5757
private static final PainlessLookup PAINLESS_LOOKUP = PainlessLookupBuilder.buildFromWhitelists(Whitelist.BASE_WHITELISTS);
5858
private static final Logger logger = ESLoggerFactory.getLogger(PainlessDocGenerator.class);
59-
private static final Comparator<PainlessField> FIELD_NAME = comparing(f -> f.name);
59+
private static final Comparator<PainlessField> FIELD_NAME = comparing(f -> f.javaField.getName());
6060
private static final Comparator<PainlessMethod> METHOD_NAME = comparing(m -> m.javaMethod.getName());
6161
private static final Comparator<PainlessMethod> METHOD_NUMBER_OF_PARAMS = comparing(m -> m.typeParameters.size());
6262
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) {
147147
emitAnchor(stream, field);
148148
stream.print("]]");
149149

150-
if (Modifier.isStatic(field.modifiers)) {
150+
if (Modifier.isStatic(field.javaField.getModifiers())) {
151151
stream.print("static ");
152152
}
153153

154-
emitType(stream, field.clazz);
154+
emitType(stream, field.typeParameter);
155155
stream.print(' ');
156156

157157
String javadocRoot = javadocRoot(field);
158158
emitJavadocLink(stream, javadocRoot, field);
159159
stream.print('[');
160-
stream.print(field.name);
160+
stream.print(field.javaField.getName());
161161
stream.print(']');
162162

163163
if (javadocRoot.equals("java8")) {
@@ -280,9 +280,9 @@ private static void emitAnchor(PrintStream stream, PainlessMethod method) {
280280
* Anchor text for a {@link PainlessField}.
281281
*/
282282
private static void emitAnchor(PrintStream stream, PainlessField field) {
283-
emitAnchor(stream, field.target);
283+
emitAnchor(stream, field.javaField.getDeclaringClass());
284284
stream.print('-');
285-
stream.print(field.name);
285+
stream.print(field.javaField.getName());
286286
}
287287

288288
private static String constructorName(PainlessConstructor constructor) {
@@ -391,9 +391,9 @@ private static void emitJavadocLink(PrintStream stream, String root, PainlessFie
391391
stream.print("link:{");
392392
stream.print(root);
393393
stream.print("-javadoc}/");
394-
stream.print(classUrlPath(field.target));
394+
stream.print(classUrlPath(field.javaField.getDeclaringClass()));
395395
stream.print(".html#");
396-
stream.print(field.javaName);
396+
stream.print(field.javaField.getName());
397397
}
398398

399399
/**
@@ -410,7 +410,7 @@ private static String javadocRoot(PainlessMethod method) {
410410
* Pick the javadoc root for a {@link PainlessField}.
411411
*/
412412
private static String javadocRoot(PainlessField field) {
413-
return javadocRoot(field.target);
413+
return javadocRoot(field.javaField.getDeclaringClass());
414414
}
415415

416416
/**

0 commit comments

Comments
 (0)