Skip to content

Commit

Permalink
GROOVY-8283: field hides getter or setter of super class (not interface)
Browse files Browse the repository at this point in the history
  • Loading branch information
eric-milles committed Oct 29, 2024
1 parent 65506f4 commit ec44e38
Show file tree
Hide file tree
Showing 2 changed files with 161 additions and 14 deletions.
43 changes: 29 additions & 14 deletions src/main/java/groovy/lang/MetaClassImpl.java
Original file line number Diff line number Diff line change
Expand Up @@ -1824,18 +1824,18 @@ public Object getProperty(final Class sender, final Object object, final String
//----------------------------------------------------------------------
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 @@ -1849,9 +1849,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 Down Expand Up @@ -1946,14 +1946,14 @@ public void setProperty(Object object, Object newValue) {
//----------------------------------------------------------------------
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;
}

//------------------------------------------------------------------
Expand All @@ -1976,8 +1976,8 @@ public void setProperty(Object object, Object newValue) {
//------------------------------------------------------------------
// non-public field
//------------------------------------------------------------------
if (mp != null) {
return mp;
if (prop != null) {
return prop;
}
}

Expand Down Expand Up @@ -2112,6 +2112,21 @@ private boolean isSpecialProperty(final String name) {
return "class".equals(name) || (isMap && ("empty".equals(name)));
}

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(sender, name, useSuper, isStatic);
Expand Down Expand Up @@ -2772,7 +2787,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))
&& (!isMap || isStatic // GROOVY-8065
|| field.isPublic())) { // GROOVY-11367
if (!field.isFinal()) {
Expand Down
132 changes: 132 additions & 0 deletions src/test/groovy/bugs/Groovy8283.groovy
Original file line number Diff line number Diff line change
@@ -0,0 +1,132 @@
/*
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
* distributed with this work for additional information
* regarding copyright ownership. The ASF licenses this file
* to you under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/
package groovy.bugs

import org.junit.Test

import static groovy.test.GroovyAssert.assertScript

final class Groovy8283 {

@Test
void testReadFieldPropertyShadowing() {
def shell = new GroovyShell()
shell.parse '''package p
class A {}
class B {}
class C {
protected A foo = new A()
A getFoo() { return foo }
}
class D extends C {
protected B foo = new B() // hides A#foo; should hide A#getFoo in subclasses
}
'''
assertScript shell, '''import p.*
class E extends D {
void test() {
assert foo.class == B
assert this.foo.class == B
assert [email protected] == B
assert this.getFoo().getClass() == A
def that = new E()
assert that.foo.class == B
assert [email protected] == B
assert that.getFoo().getClass() == A
}
}
new E().test()
assert new E().foo.class == A // not the field from this perspective
'''
}

@Test
void testWriteFieldPropertyShadowing() {
def shell = new GroovyShell()
shell.parse '''package p
class A {}
class B {}
class C {
boolean setter
protected A foo = new A()
A getFooA() { return this.@foo }
void setFoo(A a) { setter = true; this.@foo = a }
}
class D extends C {
protected B foo = new B() // hides A#foo; should hide A#setFoo in subclasses
B getFooB() { return this.@foo }
}
'''
assertScript shell, '''import p.*
class E extends D {
void test1() {
foo = null
assert !setter
assert fooA != null
assert fooB == null
}
void test2() {
this.foo = null
assert !setter
assert fooA != null
assert fooB == null
}
void test3() {
this.@foo = null
assert !setter
assert fooA != null
assert fooB == null
}
void test4() {
this.setFoo(null)
assert setter
assert fooA == null
assert fooB != null
}
void test5() {
def that = new E()
that.foo = null
assert !that.setter
assert that.fooA != null
assert that.fooB == null
that = new E()
that.@foo = null
assert !that.setter
assert that.fooA != null
assert that.fooB == null
that = new E()
that.setFoo(null)
assert that.setter
assert that.fooA == null
assert that.fooB != null
}
}
new E().test1()
new E().test2()
new E().test3()
new E().test4()
new E().test5()
'''
}
}

0 comments on commit ec44e38

Please sign in to comment.