This is an automated email from the ASF dual-hosted git repository. emilles pushed a commit to branch GROOVY-8283 in repository https://gitbox.apache.org/repos/asf/groovy.git
commit f3b35c57d93600b1cabe6767825c44bd306369b9 Author: Eric Milles <[email protected]> AuthorDate: Tue Oct 29 08:48:40 2024 -0500 GROOVY-8283: field hides getter or setter of super class (not interface) --- src/main/java/groovy/lang/MetaClassImpl.java | 43 ++++++--- src/test/groovy/bugs/Groovy8283.groovy | 132 +++++++++++++++++++++++++++ 2 files changed, 161 insertions(+), 14 deletions(-) diff --git a/src/main/java/groovy/lang/MetaClassImpl.java b/src/main/java/groovy/lang/MetaClassImpl.java index 20795be982..1b651504e7 100644 --- a/src/main/java/groovy/lang/MetaClassImpl.java +++ b/src/main/java/groovy/lang/MetaClassImpl.java @@ -1822,18 +1822,18 @@ public class MetaClassImpl implements MetaClass, MutableMetaClass { //---------------------------------------------------------------------- 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; } } @@ -1847,9 +1847,9 @@ public class MetaClassImpl implements MetaClass, MutableMetaClass { //------------------------------------------------------------------ // non-public field //------------------------------------------------------------------ - if (mp != null) { + if (prop != null) { try { - return mp.getProperty(object); + return prop.getProperty(object); } catch (GroovyRuntimeException e) { } } @@ -1944,14 +1944,14 @@ public class MetaClassImpl implements MetaClass, MutableMetaClass { //---------------------------------------------------------------------- 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; } //------------------------------------------------------------------ @@ -1974,8 +1974,8 @@ public class MetaClassImpl implements MetaClass, MutableMetaClass { //------------------------------------------------------------------ // non-public field //------------------------------------------------------------------ - if (mp != null) { - return mp; + if (prop != null) { + return prop; } } @@ -2110,6 +2110,21 @@ public class MetaClassImpl implements MetaClass, MutableMetaClass { 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); @@ -2757,7 +2772,7 @@ public class MetaClassImpl implements MetaClass, MutableMetaClass { //---------------------------------------------------------------------- // 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()) { diff --git a/src/test/groovy/bugs/Groovy8283.groovy b/src/test/groovy/bugs/Groovy8283.groovy new file mode 100644 index 0000000000..ec99829e2d --- /dev/null +++ b/src/test/groovy/bugs/Groovy8283.groovy @@ -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() + ''' + } +}
