From c2b19b0f9ee996b38bac8a2a11ba78245ee8a119 Mon Sep 17 00:00:00 2001 From: Eric Milles Date: Fri, 19 Aug 2022 11:54:51 -0500 Subject: [PATCH] GROOVY-8283: field hides getter of super class (not interface) --- src/main/java/groovy/lang/MetaClassImpl.java | 27 +++++++--- src/test/groovy/bugs/Groovy8283.groovy | 56 ++++++++++++++++++++ 2 files changed, 77 insertions(+), 6 deletions(-) create mode 100644 src/test/groovy/bugs/Groovy8283.groovy diff --git a/src/main/java/groovy/lang/MetaClassImpl.java b/src/main/java/groovy/lang/MetaClassImpl.java index c32102989d37..3d34ee1a316e 100644 --- a/src/main/java/groovy/lang/MetaClassImpl.java +++ b/src/main/java/groovy/lang/MetaClassImpl.java @@ -1981,12 +1981,12 @@ public Object getProperty(final Class sender, final Object object, final String //---------------------------------------------------------------------- Tuple2 methodAndProperty = createMetaMethodAndMetaProperty(sender, sender, name, useSuper, isStatic); MetaMethod method = methodAndProperty.getV1(); + MetaProperty mp = methodAndProperty.getV2(); - if (method == null || isSpecialProperty(name)) { + if (method == null || isSpecialProperty(name) || isVisibleProperty(mp, method, sender)) { //------------------------------------------------------------------ // public field //------------------------------------------------------------------ - MetaProperty mp = methodAndProperty.getV2(); if (mp != null && Modifier.isPublic(mp.getModifiers())) { try { return mp.getProperty(object); @@ -2097,19 +2097,19 @@ public void setProperty(Object object, Object newValue) { //---------------------------------------------------------------------- Tuple2 methodAndProperty = createMetaMethodAndMetaProperty(sender, theClass, name, useSuper, isStatic); MetaMethod method = methodAndProperty.getV1(); + MetaProperty mp = methodAndProperty.getV2(); - if (method == null || isSpecialProperty(name)) { + if (method == null || isSpecialProperty(name) || isVisibleProperty(mp, method, sender)) { //------------------------------------------------------------------ // public field //------------------------------------------------------------------ - MetaProperty mp = methodAndProperty.getV2(); if (mp != null && Modifier.isPublic(mp.getModifiers())) { return mp; } - //---------------------------------------------------------------------- + //------------------------------------------------------------------ // java.util.Map get method - //---------------------------------------------------------------------- + //------------------------------------------------------------------ if (isMap && !isStatic) { return new MetaProperty(name, Object.class) { @Override @@ -2243,6 +2243,21 @@ private boolean isSpecialProperty(final String name) { return "class".equals(name) || (isMap && ("empty".equals(name) || "metaClass".equals(name))); } + private boolean isVisibleProperty(final MetaProperty field, final MetaMethod method, final Class sender) { + if (!(field instanceof CachedField)) return false; + if (Modifier.isPrivate(field.getModifiers())) 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 (!Modifier.isPublic(field.getModifiers()) + && !Modifier.isProtected(field.getModifiers()) + && !owner.getPackageName().equals(sender.getPackageName())) return false; + + // GROOVY-8283: non-private field that hides class access method + return !owner.isAssignableFrom(method.getDeclaringClass().getTheClass()) && !method.getDeclaringClass().isInterface(); + } + private Tuple2 createMetaMethodAndMetaProperty(final Class senderForMP, final Class senderForCMG, final String name, final boolean useSuper, final boolean isStatic) { MetaMethod method = null; MetaProperty mp = getMetaProperty(senderForMP, name, useSuper, isStatic); diff --git a/src/test/groovy/bugs/Groovy8283.groovy b/src/test/groovy/bugs/Groovy8283.groovy new file mode 100644 index 000000000000..3446c9c42b97 --- /dev/null +++ b/src/test/groovy/bugs/Groovy8283.groovy @@ -0,0 +1,56 @@ +/* + * 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 testFieldPropertyShadowing() { + assertScript ''' + 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 + } + class E extends D { + void test() { + assert foo.class == B + assert this.foo.class == B + assert this.@foo.class == B + assert this.getFoo().getClass() == A + + def that = new E() + assert that.foo.class == B + assert that.@foo.class == B + assert that.getFoo().getClass() == A + } + } + + new E().test() + assert new E().foo.class == A // not the field from this perspective + ''' + } +}