Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

GROOVY-8283: field hides getter of super class (not interface) #1767

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from
Draft

GROOVY-8283: field hides getter of super class (not interface) #1767

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
c9b6ec2
GROOVY-8283: field hides getter of super class (not interface)
eric-milles Aug 19, 2022
8324f8b
GROOVY-8283: field hides setter of super class (not interface)
eric-milles Apr 27, 2024
c880517
propagate sender class for `getProperty` and `setProperty`
eric-milles Apr 28, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
84 changes: 49 additions & 35 deletions src/main/java/groovy/lang/MetaClassImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1820,20 +1820,20 @@ public Object getProperty(final Class sender, final Object object, final String
//----------------------------------------------------------------------
// getter
//----------------------------------------------------------------------
Tuple2<MetaMethod, MetaProperty> methodAndProperty = createMetaMethodAndMetaProperty(sender, sender, name, useSuper, isStatic);
Tuple2<MetaMethod, MetaProperty> methodAndProperty = createMetaMethodAndMetaProperty(sender, name, useSuper, isStatic);
MetaMethod method = methodAndProperty.getV1();
MetaProperty prop = methodAndProperty.getV2();

if (method == null || isSpecialProperty(name)) {
if (method == null || isSpecialProperty(name) || isVisibleProperty(prop, method, sender)) {
//------------------------------------------------------------------
// public field
//------------------------------------------------------------------
MetaProperty mp = methodAndProperty.getV2();
if (mp != null && mp.isPublic()) {
if (prop != null && prop.isPublic()) {
try {
return mp.getProperty(object);
return prop.getProperty(object);
} catch (GroovyRuntimeException e) {
// can't access the field directly but there may be a getter
mp = null;
prop = null;
}
}

Expand All @@ -1847,9 +1847,9 @@ public Object getProperty(final Class sender, final Object object, final String
//------------------------------------------------------------------
// non-public field
//------------------------------------------------------------------
if (mp != null) {
if (prop != null) {
try {
return mp.getProperty(object);
return prop.getProperty(object);
} catch (GroovyRuntimeException e) {
}
}
Expand All @@ -1861,10 +1861,10 @@ public Object getProperty(final Class sender, final Object object, final String
Object[] arguments = EMPTY_ARGUMENTS;
if (method == null && !useSuper && !isStatic && GroovyCategorySupport.hasCategoryInCurrentThread()) {
// check for propertyMissing provided through a category; TODO:should this have lower precedence?
method = getCategoryMethodGetter(sender, PROPERTY_MISSING, true);
method = getCategoryMethodGetter(theClass, PROPERTY_MISSING, true);
if (method == null) {
// check for a generic get method provided through a category
method = getCategoryMethodGetter(sender, "get", true);
method = getCategoryMethodGetter(theClass, "get", true);
}
if (method != null) arguments = new Object[]{name};
}
Expand Down Expand Up @@ -1935,21 +1935,21 @@ public void setProperty(Object object, Object newValue) {
//----------------------------------------------------------------------
// getter
//----------------------------------------------------------------------
Tuple2<MetaMethod, MetaProperty> methodAndProperty = createMetaMethodAndMetaProperty(sender, theClass, name, useSuper, isStatic);
Tuple2<MetaMethod, MetaProperty> methodAndProperty = createMetaMethodAndMetaProperty(sender, name, useSuper, isStatic);
MetaMethod method = methodAndProperty.getV1();
MetaProperty prop = methodAndProperty.getV2();

if (method == null || isSpecialProperty(name)) {
if (method == null || isSpecialProperty(name) || isVisibleProperty(prop, method, sender)) {
//------------------------------------------------------------------
// public field
//------------------------------------------------------------------
MetaProperty mp = methodAndProperty.getV2();
if (mp != null && mp.isPublic()) {
return mp;
if (prop != null && prop.isPublic()) {
return prop;
}

//----------------------------------------------------------------------
//------------------------------------------------------------------
// java.util.Map get method
//----------------------------------------------------------------------
//------------------------------------------------------------------
if (isMap && !isStatic) {
return new MetaProperty(name, Object.class) {
@Override
Expand All @@ -1967,8 +1967,8 @@ public void setProperty(Object object, Object newValue) {
//------------------------------------------------------------------
// non-public field
//------------------------------------------------------------------
if (mp != null) {
return mp;
if (prop != null) {
return prop;
}
}

Expand All @@ -1980,9 +1980,9 @@ public void setProperty(Object object, Object newValue) {
// generic get method
//----------------------------------------------------------------------
if (!useSuper && !isStatic && GroovyCategorySupport.hasCategoryInCurrentThread()) {
method = getCategoryMethodGetter(sender, "get", true);
method = getCategoryMethodGetter(theClass, "get", true);
if (null == method) {
method = getCategoryMethodGetter(sender, PROPERTY_MISSING, true);
method = getCategoryMethodGetter(theClass, PROPERTY_MISSING, true);
}
if (method != null) {
return new GetMethodMetaProperty(name, VM_PLUGIN.transformMetaMethod(this, method));
Expand Down Expand Up @@ -2086,13 +2086,28 @@ private boolean isSpecialProperty(final String name) {
return "class".equals(name) || (isMap && ("empty".equals(name) || "metaClass".equals(name)));
}

private Tuple2<MetaMethod, MetaProperty> createMetaMethodAndMetaProperty(final Class senderForMP, final Class senderForCMG, final String name, final boolean useSuper, final boolean isStatic) {
private boolean isVisibleProperty(final MetaProperty field, final MetaMethod method, final Class<?> sender) {
if (!(field instanceof CachedField)) return false;

if (field.isPrivate()) return false;

Class<?> owner = ((CachedField) field).getDeclaringClass();
// ensure access originates within the type hierarchy of the field owner
if (owner.equals(sender) || !owner.isAssignableFrom(sender)) return false;

if (!field.isPublic() && !field.isProtected() && !inSamePackage(owner, sender)) return false;

// GROOVY-8283: non-private field that hides class access method
return !owner.isAssignableFrom(method.getDeclaringClass().getTheClass()) && !method.getDeclaringClass().isInterface();
}

private Tuple2<MetaMethod, MetaProperty> createMetaMethodAndMetaProperty(final Class sender, final String name, final boolean useSuper, final boolean isStatic) {
MetaMethod method = null;
MetaProperty mp = getMetaProperty(senderForMP, name, useSuper, isStatic);
MetaProperty mp = getMetaProperty(sender, name, useSuper, isStatic);
if ((mp == null || mp instanceof CachedField) && !name.isEmpty() && isUpperCase(name.charAt(0)) && (name.length() < 2 || !isUpperCase(name.charAt(1))) && !"Class".equals(name) && !"MetaClass".equals(name)) {
// GROOVY-9618 adjust because capitalised properties aren't stored as meta bean props
MetaProperty saved = mp;
mp = getMetaProperty(senderForMP, BeanUtils.decapitalize(name), useSuper, isStatic);
mp = getMetaProperty(sender, BeanUtils.decapitalize(name), useSuper, isStatic);
if (mp == null || (saved != null && mp instanceof CachedField)) {
// restore if we didn't find something better
mp = saved;
Expand All @@ -2109,7 +2124,7 @@ private Tuple2<MetaMethod, MetaProperty> createMetaMethodAndMetaProperty(final C
if (!useSuper && !isStatic && GroovyCategorySupport.hasCategoryInCurrentThread()) {
String getterName = GroovyCategorySupport.getPropertyCategoryGetterName(name);
if (getterName != null) {
MetaMethod categoryMethod = getCategoryMethodGetter(senderForCMG, getterName, false);
MetaMethod categoryMethod = getCategoryMethodGetter(theClass, getterName, false);
if (categoryMethod != null)
method = categoryMethod;
}
Expand Down Expand Up @@ -2706,11 +2721,10 @@ public void setProperty(final Class sender, final Object object, final String na
}

// check for a category method named like a setter
if (!useSuper && !isStatic && GroovyCategorySupport.hasCategoryInCurrentThread()
&& name.length() > 0) {
String getterName = GroovyCategorySupport.getPropertyCategorySetterName(name);
if (getterName != null) {
MetaMethod categoryMethod = getCategoryMethodSetter(sender, getterName, false);
if (!useSuper && !isStatic && !name.isEmpty() && GroovyCategorySupport.hasCategoryInCurrentThread()) {
var setterName = GroovyCategorySupport.getPropertyCategorySetterName(name);
if (setterName != null) {
MetaMethod categoryMethod = getCategoryMethodSetter(theClass, setterName, false);
if (categoryMethod != null) {
method = categoryMethod;
arguments = new Object[]{newValue};
Expand All @@ -2732,7 +2746,7 @@ public void setProperty(final Class sender, final Object object, final String na
Object proxy = Proxy.newProxyInstance(
theClass.getClassLoader(),
new Class[]{method.getParameterTypes()[0].getTheClass()},
new ConvertedClosure((Closure) newValue, name));
new ConvertedClosure((Closure<?>) newValue, name));
arguments = new Object[]{proxy};
newValue = proxy;
} else {
Expand All @@ -2743,7 +2757,7 @@ public void setProperty(final Class sender, final Object object, final String na
//----------------------------------------------------------------------
// field
//----------------------------------------------------------------------
if (method == null && field != null) {
if (field != null && (method == null || isVisibleProperty(field, method, sender))) {
boolean mapInstance = (isMap && !isStatic);
if (field.isFinal()) {
if (mapInstance) { // GROOVY-8065
Expand All @@ -2763,7 +2777,7 @@ public void setProperty(final Class sender, final Object object, final String na
//----------------------------------------------------------------------
// check for a generic get method provided through a category
if (method == null && !useSuper && !isStatic && GroovyCategorySupport.hasCategoryInCurrentThread()) {
method = getCategoryMethodSetter(sender, "set", true);
method = getCategoryMethodSetter(theClass, "set", true);
if (method != null) arguments = new Object[]{name, newValue};
}
if (method == null && genericSetMethod != null && (genericSetMethod.isStatic() || !isStatic)) {
Expand Down Expand Up @@ -2791,9 +2805,9 @@ public void setProperty(final Class sender, final Object object, final String na
return;
}

//------------------------------------------------------------------
//----------------------------------------------------------------------
// java.util.Map put method
//------------------------------------------------------------------
//----------------------------------------------------------------------
if (isMap && !isStatic) {
((Map) object).put(name, newValue);
return;
Expand Down
52 changes: 37 additions & 15 deletions src/main/java/groovy/util/Node.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -294,42 +294,65 @@ private static List<Node> buildChildrenFromClosure(Closure c) {
* @param metaClass the original metaclass
* @param nodeClass the class whose metaclass we wish to override (this class or a subclass)
*/
protected static void setMetaClass(final MetaClass metaClass, Class nodeClass) {
protected static void setMetaClass(final MetaClass metaClass, final Class nodeClass) {
// TODO Is protected static a bit of a smell?
// TODO perhaps set nodeClass to be Class<? extends Node>
final MetaClass newMetaClass = new DelegatingMetaClass(metaClass) {
GroovySystem.getMetaClassRegistry().setMetaClass(nodeClass, new DelegatingMetaClass(metaClass) {

@Override
public Object getAttribute(final Object object, final String attribute) {
Node n = (Node) object;
return n.get("@" + attribute);
return ((Node) object).get("@" + attribute);
}

@Override
public Object getAttribute(final Class sender, final Object object, final String attribute, final boolean isSuper) {
return getAttribute(object, attribute);
}

@Override
public void setAttribute(final Object object, final String attribute, final Object newValue) {
Node n = (Node) object;
n.attributes().put(attribute, newValue);
((Node) object).attributes().put(attribute, newValue);
}

@Override
public void setAttribute(final Class sender, final Object object, final String attribute, final Object newValue, final boolean isSuper, final boolean isInner) {
setAttribute(object, attribute, newValue);
}

@Override
public Object getProperty(Object object, String property) {
public Object getProperty(final Object object, final String property) {
if (object instanceof Node) {
Node n = (Node) object;
return n.get(property);
return ((Node) object).get(property);
}
return super.getProperty(object, property);
}

@Override
public void setProperty(Object object, String property, Object newValue) {
public Object getProperty(final Class sender, final Object object, final String property, final boolean isSuper, final boolean isInner) {
if (object instanceof Node) {
return ((Node) object).get(property);
}
return super.getProperty(sender, object, property, isSuper, isInner);
}

@Override
public void setProperty(final Object object, final String property, final Object newValue) {
if (property.startsWith("@")) {
setAttribute(object, property.substring(1), newValue);
return;
} else {
super.setProperty(object, property, newValue);
}
delegate.setProperty(object, property, newValue);
}

};
GroovySystem.getMetaClassRegistry().setMetaClass(nodeClass, newMetaClass);
@Override
public void setProperty(final Class sender, final Object object, final String property, final Object newValue, final boolean isSuper, final boolean isInner) {
if (property.startsWith("@")) {
setAttribute(object, property.substring(1), newValue);
} else {
super.setProperty(sender, object, property, newValue, isSuper, isInner);
}
}
});
}

/**
Expand Down Expand Up @@ -786,7 +809,6 @@ public void print(PrintWriter out) {
new NodePrinter(out).print(this);
}


/**
* Converts the text of this GPathResult to an Integer object.
*
Expand Down
50 changes: 31 additions & 19 deletions src/main/java/groovy/util/NodeList.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,6 @@

import java.util.ArrayList;
import java.util.Collection;
import java.util.Iterator;
import java.util.List;

/**
* A List implementation which is returned by queries on a {@link Node}
Expand Down Expand Up @@ -74,38 +72,53 @@ public Object clone() {
return result;
}

protected static void setMetaClass(final Class nodelistClass, final MetaClass metaClass) {
final MetaClass newMetaClass = new DelegatingMetaClass(metaClass) {
protected static void setMetaClass(final Class nodeListClass, final MetaClass metaClass) {
GroovySystem.getMetaClassRegistry().setMetaClass(nodeListClass, new DelegatingMetaClass(metaClass) {

@Override
public Object getAttribute(final Object object, final String attribute) {
NodeList nl = (NodeList) object;
Iterator it = nl.iterator();
List result = new ArrayList();
while (it.hasNext()) {
Node node = (Node) it.next();
result.add(node.attributes().get(attribute));
NodeList list = (NodeList) object;
var result = new ArrayList<Object>(list.size());
for (Object node : list) {
var attributes = ((Node) node).attributes();
result.add(attributes.get(attribute));
}
return result;
}

@Override
public Object getAttribute(Class sender, Object object, String attribute, boolean isSuper) {
return getAttribute(object, attribute);
}

@Override
public void setAttribute(final Object object, final String attribute, final Object newValue) {
for (Object o : (NodeList) object) {
Node node = (Node) o;
node.attributes().put(attribute, newValue);
for (Object node : (NodeList) object) {
((Node) node).attributes().put(attribute, newValue);
}
}

@Override
public Object getProperty(Object object, String property) {
public void setAttribute(final Class sender, final Object object, final String attribute, final Object newValue, final boolean isSuper, final boolean isInner) {
setAttribute(object, attribute, newValue);
}

@Override
public Object getProperty(final Object object, final String property) {
if (object instanceof NodeList) {
NodeList nl = (NodeList) object;
return nl.getAt(property);
return ((NodeList) object).getAt(property);
}
return super.getProperty(object, property);
}
};
GroovySystem.getMetaClassRegistry().setMetaClass(nodelistClass, newMetaClass);

@Override
public Object getProperty(final Class sender, final Object object, final String property, final boolean isSuper, final boolean isInner) {
if (object instanceof NodeList) {
return ((NodeList) object).getAt(property);
}
return super.getProperty(sender, object, property, isSuper, isInner);
}
});
}

/**
Expand Down Expand Up @@ -197,5 +210,4 @@ public void plus(Closure c) {
((Node) o).plus(c);
}
}

}