This is an automated email from the ASF dual-hosted git repository. emilles pushed a commit to branch GROOVY-11562 in repository https://gitbox.apache.org/repos/asf/groovy.git
commit 98cc64961fadd42f24274424149d538b0336d309 Author: Eric Milles <[email protected]> AuthorDate: Sun Feb 9 10:42:56 2025 -0600 GROOVY-11562: final modifier of `MetaBeanProperty` --- .../java/groovy/lang/MetaArrayLengthProperty.java | 31 ++-- src/main/java/groovy/lang/MetaBeanProperty.java | 111 +++++++------- src/test/groovy/bugs/Groovy4098.groovy | 169 +++++++++++++++++++++ src/test/groovy/bugs/Groovy4098Bug.groovy | 151 ------------------ 4 files changed, 231 insertions(+), 231 deletions(-) diff --git a/src/main/java/groovy/lang/MetaArrayLengthProperty.java b/src/main/java/groovy/lang/MetaArrayLengthProperty.java index 1bb8f5ba16..c7687a3a94 100644 --- a/src/main/java/groovy/lang/MetaArrayLengthProperty.java +++ b/src/main/java/groovy/lang/MetaArrayLengthProperty.java @@ -18,39 +18,30 @@ */ package groovy.lang; +import java.lang.reflect.Array; +import java.lang.reflect.Modifier; /** - * Represents the length property of an array + * Represents the length property of an array. */ public class MetaArrayLengthProperty extends MetaProperty { - /** - * Sole constructor setting name to "length" and type to int - */ public MetaArrayLengthProperty() { super("length", int.class); } - /** - * Get this property from the given object. - * @param object an array - * @return the length of the array object - * @throws IllegalArgumentException if object is not an array - */ @Override - public Object getProperty(Object object) { - return java.lang.reflect.Array.getLength(object); + public int getModifiers() { + return Modifier.FINAL | Modifier.PUBLIC; } - /** - * Sets the property on the given object to the new value - * - * @param object on which to set the property - * @param newValue the new value of the property - * @throws RuntimeException if the property could not be set - */ @Override - public void setProperty(Object object, Object newValue) { + public Object getProperty(final Object object) { + return Array.getLength(object); + } + + @Override + public void setProperty(final Object object, final Object newValue) { throw new ReadOnlyPropertyException("length", object.getClass()); } } diff --git a/src/main/java/groovy/lang/MetaBeanProperty.java b/src/main/java/groovy/lang/MetaBeanProperty.java index 8c80091bc5..20f704166b 100644 --- a/src/main/java/groovy/lang/MetaBeanProperty.java +++ b/src/main/java/groovy/lang/MetaBeanProperty.java @@ -33,27 +33,49 @@ public class MetaBeanProperty extends MetaProperty { private MetaMethod setter; private CachedField field; - /** - * Sole constructor setting name, type (class), getter and setter. - */ - public MetaBeanProperty(String name, Class type, MetaMethod getter, MetaMethod setter) { + public MetaBeanProperty(final String name, final Class type, final MetaMethod getter, final MetaMethod setter) { super(name, type); this.getter = getter; this.setter = setter; } /** - * Get the property of the given object. + * Gets the visibility modifiers of the property as defined by the getter, setter and field. + */ + @Override + public int getModifiers() { + int modifiers; + MetaMethod getter = getGetter(); + MetaMethod setter = getSetter(); + final int staticAndVisibility = 0xF; + if (getter == null) { + modifiers = setter.getModifiers() & staticAndVisibility; + } else if (setter == null) { + modifiers = getter.getModifiers() & staticAndVisibility; + CachedField field = getField(); // GROOVY-11562: final modifier + if (field == null || field.isFinal()) modifiers |= Modifier.FINAL; + } else { + modifiers = (getter.getModifiers() & staticAndVisibility) | (setter.getModifiers() & staticAndVisibility); + if (Modifier.isPublic (modifiers)) modifiers &= ~(Modifier.PROTECTED | Modifier.PRIVATE); + if (Modifier.isProtected(modifiers)) modifiers &= ~Modifier.PRIVATE; + } + return modifiers; + } + + /** + * Gets the property of the given object. * * @param object which to be got * @return the property of the given object * @throws RuntimeException if the property could not be evaluated */ @Override - public Object getProperty(Object object) { + public Object getProperty(final Object object) { MetaMethod getter = getGetter(); if (getter == null) { - if (field != null) return field.getProperty(object); + if (getField() != null) { + return getField().getProperty(object); + } //TODO: create a WriteOnlyException class? throw new GroovyRuntimeException("Cannot read write-only property: " + name); } @@ -61,16 +83,17 @@ public class MetaBeanProperty extends MetaProperty { } /** - * Set the property on the given object to the new value. + * Sets the property on the given object to the new value. * * @param object on which to set the property * @param newValue the new value of the property * @throws RuntimeException if the property could not be set */ @Override - public void setProperty(Object object, Object newValue) { + public void setProperty(final Object object, Object newValue) { MetaMethod setter = getSetter(); if (setter == null) { + CachedField field = getField(); if (field != null && !field.isFinal()) { field.setProperty(object, newValue); return; @@ -81,79 +104,47 @@ public class MetaBeanProperty extends MetaProperty { setter.invoke(object, new Object[]{newValue}); } - /** - * Get the getter method. - * - * @return the getter method for this property. - */ - public MetaMethod getGetter() { - return getter; - } + //-------------------------------------------------------------------------- /** - * Get the setter method. - * - * @return the setter method for this property. + * Gets the field of this property. */ - public MetaMethod getSetter() { - return setter; + public CachedField getField() { + return field; } /** - * This is for MetaClass to patch up the object later when looking for get*() methods. - * - * @param getter The getter for this property + * Gets the getter method of this property. */ - void setGetter(MetaMethod getter) { - this.getter = getter; + public MetaMethod getGetter() { + return getter; } /** - * This is for MetaClass to patch up the object later when looking for set*() methods. - * - * @param setter The setter for this property + * Gets the setter method of this property. */ - void setSetter(MetaMethod setter) { - this.setter = setter; + public MetaMethod getSetter() { + return setter; } /** - * Gets the visibility modifiers for the property as defined by the getter and setter methods. - * - * @return the visibility modifier of the getter, the setter, or both depending on which exist + * Sets the field of this property. */ - @Override - public int getModifiers() { - MetaMethod getter = getGetter(); - MetaMethod setter = getSetter(); - if (setter != null && getter == null) return setter.getModifiers(); - if (getter != null && setter == null) return getter.getModifiers(); - int modifiers = getter.getModifiers() | setter.getModifiers(); - int visibility = 0; - if (Modifier.isPublic(modifiers)) visibility = Modifier.PUBLIC; - if (Modifier.isProtected(modifiers)) visibility = Modifier.PROTECTED; - if (Modifier.isPrivate(modifiers)) visibility = Modifier.PRIVATE; - int states = getter.getModifiers() & setter.getModifiers(); - states &= ~(Modifier.PUBLIC | Modifier.PROTECTED | Modifier.PRIVATE); - states |= visibility; - return states; + public void setField(final CachedField field) { + this.field = field; } /** - * Sets the field of this property - * - * @param field + * This is for MetaClass to patch up the object later when looking for get*() methods. */ - public void setField(CachedField field) { - this.field = field; + void setGetter(final MetaMethod getter) { + this.getter = getter; } /** - * Gets the field of this property - * - * @return The field of this property + * This is for MetaClass to patch up the object later when looking for set*() methods. */ - public CachedField getField() { - return field; + void setSetter(final MetaMethod setter) { + this.setter = setter; } } diff --git a/src/test/groovy/bugs/Groovy4098.groovy b/src/test/groovy/bugs/Groovy4098.groovy new file mode 100644 index 0000000000..b1b077543c --- /dev/null +++ b/src/test/groovy/bugs/Groovy4098.groovy @@ -0,0 +1,169 @@ +/* + * 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.shouldFail +import static java.lang.reflect.Modifier.isFinal + +final class Groovy4098 { + + public String propertyOne + public String propertyTwo + public String propertyThree + public String propertyFour + public final String propertyFive = 'five normal' + public final String propertySix = 'six normal' + + void setPropertyTwo(String propertyTwo) { + this.propertyTwo = propertyTwo + } + + String getPropertyThree() { + propertyThree + } + + String getPropertyFive() { + propertyFive + } + + String getPropertyFour() { + propertyFour + } + + void setPropertyFour(String propertyFour) { + this.propertyFour = propertyFour + } + + //-------------------------------------------------------------------------- + + @Test + void test1() { + propertyOne = 'one normal' + assert propertyOne == 'one normal' + + def metaProperty = getMetaClass().getMetaProperty('propertyOne') + metaProperty.setProperty(this, 'one mop') + assert metaProperty.getProperty(this) == 'one mop' + } + + @Test + void test2() { + propertyTwo = 'two normal' + assert propertyTwo == 'two normal' + + def metaProperty = getMetaClass().getMetaProperty('propertyTwo') + metaProperty.setProperty(this, 'two mop') + assert metaProperty.getProperty(this) == 'two mop' + } + + @Test + void test3() { + propertyThree = 'three normal' + assert propertyThree == 'three normal' + + def metaProperty = getMetaClass().getMetaProperty('propertyThree') + assert metaProperty.getProperty(this) == 'three normal' + metaProperty.setProperty(this, 'three mop') + assert metaProperty.getProperty(this) == 'three mop' + } + + @Test + void test4() { + propertyOne = 'four normal' + assert propertyOne == 'four normal' + + def metaProperty = getMetaClass().getMetaProperty('propertyFour') + metaProperty.setProperty(this, 'four mop') + assert metaProperty.getProperty(this) == 'four mop' + } + + @Test + void test5() { + assert propertyFive == 'five normal' + + def metaProperty = getMetaClass().getMetaProperty('propertyFive') + assert isFinal(metaProperty.getModifiers()) // GROOVY-11562 + assert metaProperty.getProperty(this) == 'five normal' + def err = shouldFail { + metaProperty.setProperty(this, 'five mop') + } + assert err =~ /Cannot set read-only property: propertyFive/ + } + + @Test + void test6() { + assert propertySix == 'six normal' + + def metaProperty = getMetaClass().getMetaProperty('propertySix') + assert metaProperty.getProperty(this) == 'six normal' + def err = shouldFail { + metaProperty.setProperty(this, 'six mop') + } + assert err =~ /Cannot set the property 'propertySix' because the backing field is final./ + } + + // + + @Test + void testProtected1() { + def p = new Groovy4098Child() + p.propertyOne = 'one normal' + assert p.propertyOne == 'one normal' + + def metaProperty = p.getMetaClass().getMetaProperty('propertyOne') + metaProperty.setProperty(p, 'one mop') + assert metaProperty.getProperty(p) == 'one mop' + } + + @Test + void testProtected2() { + def p = new Groovy4098Child() + p.propertyTwo = 'two normal' + assert p.propertyTwo == 'two normal' + + def metaProperty = p.getMetaClass().getMetaProperty('propertyTwo') + metaProperty.setProperty(p, 'two mop') + assert metaProperty.getProperty(p) == 'two mop' + } + + @Test + void testProtected3() { + def p = new Groovy4098Child() + p.propertyThree = 'three normal' + assert p.propertyThree == 'three normal' + + def metaProperty = p.getMetaClass().getMetaProperty('propertyThree') + assert metaProperty.getProperty(p) == 'three normal' + metaProperty.setProperty(p, 'three mop') + assert metaProperty.getProperty(p) == 'three mop' + } + + @Test + void testProtected4() { + def p = new Groovy4098Child() + p.propertyOne = 'four normal' + assert p.propertyOne == 'four normal' + + def metaProperty = p.getMetaClass().getMetaProperty('propertyFour') + metaProperty.setProperty(p, 'four mop') + assert metaProperty.getProperty(p) == 'four mop' + } +} diff --git a/src/test/groovy/bugs/Groovy4098Bug.groovy b/src/test/groovy/bugs/Groovy4098Bug.groovy deleted file mode 100644 index 99bd0831e9..0000000000 --- a/src/test/groovy/bugs/Groovy4098Bug.groovy +++ /dev/null @@ -1,151 +0,0 @@ -/* - * 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 groovy.test.GroovyTestCase - -class Groovy4098Bug extends GroovyTestCase { - public String propertyOne - public String propertyTwo - public String propertyThree - public String propertyFour - public final String propertyFive = "five normal" - public final String propertySix = "six normal" - - void setPropertyTwo(String propertyTwo) { - this.propertyTwo = propertyTwo - } - - String getPropertyThree() { - propertyThree - } - - String getPropertyFive() { - propertyFive - } - - String getPropertyFour() { - propertyFour - } - - void setPropertyFour(String propertyFour) { - this.propertyFour = propertyFour - } - - void testOne() { - propertyOne = "one normal" - assert propertyOne == "one normal" - - def metaProperty = this.metaClass.getMetaProperty("propertyOne") - metaProperty.setProperty(this, "one mop") - assert metaProperty.getProperty(this) == "one mop" - } - - void testTwo() { - propertyTwo = "two normal" - assert propertyTwo == "two normal" - - def metaProperty = this.metaClass.getMetaProperty("propertyTwo") - metaProperty.setProperty(this, "two mop") - assert metaProperty.getProperty(this) == "two mop" - } - - void testThree() { - propertyThree = "three normal" - assert propertyThree == "three normal" - - def metaProperty = this.metaClass.getMetaProperty("propertyThree") - assert metaProperty.getProperty(this) == "three normal" - metaProperty.setProperty(this, "three mop") - assert metaProperty.getProperty(this) == "three mop" - } - - void testFour() { - propertyOne = "four normal" - assert propertyOne == "four normal" - - def metaProperty = this.metaClass.getMetaProperty("propertyFour") - metaProperty.setProperty(this, "four mop") - assert metaProperty.getProperty(this) == "four mop" - } - - void testFive() { - assert propertyFive == "five normal" - - def metaProperty = this.metaClass.getMetaProperty("propertyFive") - assert metaProperty.getProperty(this) == "five normal" - def msg = shouldFail { - metaProperty.setProperty(this, "five mop") - } - assert msg == "Cannot set read-only property: propertyFive" - } - - void testSix() { - assert propertySix == "six normal" - - def metaProperty = this.metaClass.getMetaProperty("propertySix") - assert metaProperty.getProperty(this) == "six normal" - def msg = shouldFail { - metaProperty.setProperty(this, "six mop") - } - assert msg == "Cannot set the property 'propertySix' because the backing field is final." - } - - void testOneProtected() { - def p = new Groovy4098Child() - p.propertyOne = "one normal" - assert p.propertyOne == "one normal" - - def metaProperty = p.metaClass.getMetaProperty("propertyOne") - metaProperty.setProperty(p, "one mop") - assert metaProperty.getProperty(p) == "one mop" - } - - void testTwoProtected() { - def p = new Groovy4098Child() - p.propertyTwo = "two normal" - assert p.propertyTwo == "two normal" - - def metaProperty = p.metaClass.getMetaProperty("propertyTwo") - metaProperty.setProperty(p, "two mop") - assert metaProperty.getProperty(p) == "two mop" - } - - void testThreeProtected() { - def p = new Groovy4098Child() - p.propertyThree = "three normal" - assert p.propertyThree == "three normal" - - def metaProperty = p.metaClass.getMetaProperty("propertyThree") - assert metaProperty.getProperty(p) == "three normal" - metaProperty.setProperty(p, "three mop") - assert metaProperty.getProperty(p) == "three mop" - } - - void testFourProtected() { - def p = new Groovy4098Child() - p.propertyOne = "four normal" - assert p.propertyOne == "four normal" - - def metaProperty = p.metaClass.getMetaProperty("propertyFour") - metaProperty.setProperty(p, "four mop") - assert metaProperty.getProperty(p) == "four mop" - } - -} \ No newline at end of file
