This is an automated email from the ASF dual-hosted git repository. paulk pushed a commit to branch GROOVY_3_0_X in repository https://gitbox.apache.org/repos/asf/groovy.git
commit 232786c25623bab2aa64df354b0ff11d6fa7960a Author: Paul King <[email protected]> AuthorDate: Fri Jul 17 17:49:05 2020 +1000 GROOVY-9645: Inconsistencies in JavaBean naming for property access --- src/main/java/groovy/lang/MetaClassImpl.java | 4 + .../apache/groovy/ast/tools/ClassNodeUtils.java | 7 +- .../groovy/runtime/GroovyCategorySupport.java | 17 +- .../transformers/BinaryExpressionTransformer.java | 7 + src/test/groovy/PropertyTest.groovy | 273 +++++++++++++++++++++ src/test/groovy/bugs/Groovy5852Bug.groovy | 51 ---- .../org/apache/groovy/util/BeanUtilsTest.groovy | 1 + 7 files changed, 302 insertions(+), 58 deletions(-) diff --git a/src/main/java/groovy/lang/MetaClassImpl.java b/src/main/java/groovy/lang/MetaClassImpl.java index f26af35..9d1a4e7 100644 --- a/src/main/java/groovy/lang/MetaClassImpl.java +++ b/src/main/java/groovy/lang/MetaClassImpl.java @@ -110,6 +110,7 @@ import java.util.Set; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentMap; +import static java.lang.Character.isUpperCase; import static org.apache.groovy.util.Arrays.concat; import static org.codehaus.groovy.ast.tools.GeneralUtils.inSamePackage; import static org.codehaus.groovy.reflection.ReflectionCache.isAssignableFrom; @@ -2087,6 +2088,9 @@ public class MetaClassImpl implements MetaClass, MutableMetaClass { private Tuple2<MetaMethod, MetaProperty> 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); + if (mp == null && isUpperCase(name.charAt(0)) && (name.length() < 2 || !isUpperCase(name.charAt(1)))) { + mp = getMetaProperty(senderForMP, BeanUtils.decapitalize(name), useSuper, isStatic); + } if (mp != null) { if (mp instanceof MetaBeanProperty) { MetaBeanProperty mbp = (MetaBeanProperty) mp; diff --git a/src/main/java/org/apache/groovy/ast/tools/ClassNodeUtils.java b/src/main/java/org/apache/groovy/ast/tools/ClassNodeUtils.java index c546316..65fc823 100644 --- a/src/main/java/org/apache/groovy/ast/tools/ClassNodeUtils.java +++ b/src/main/java/org/apache/groovy/ast/tools/ClassNodeUtils.java @@ -18,6 +18,7 @@ */ package org.apache.groovy.ast.tools; +import org.apache.groovy.util.BeanUtils; import org.codehaus.groovy.ast.ClassHelper; import org.codehaus.groovy.ast.ClassNode; import org.codehaus.groovy.ast.ConstructorNode; @@ -302,7 +303,11 @@ public class ClassNodeUtils { } public static boolean hasStaticProperty(ClassNode cNode, String propName) { - return getStaticProperty(cNode, propName) != null; + PropertyNode found = getStaticProperty(cNode, propName); + if (found == null) { + found = getStaticProperty(cNode, BeanUtils.decapitalize(propName)); + } + return found != null; } /** diff --git a/src/main/java/org/codehaus/groovy/runtime/GroovyCategorySupport.java b/src/main/java/org/codehaus/groovy/runtime/GroovyCategorySupport.java index 8843f11..c96780f 100644 --- a/src/main/java/org/codehaus/groovy/runtime/GroovyCategorySupport.java +++ b/src/main/java/org/codehaus/groovy/runtime/GroovyCategorySupport.java @@ -19,6 +19,7 @@ package org.codehaus.groovy.runtime; import groovy.lang.Closure; +import org.apache.groovy.util.BeanUtils; import org.codehaus.groovy.reflection.CachedClass; import org.codehaus.groovy.reflection.CachedMethod; import org.codehaus.groovy.reflection.ReflectionCache; @@ -174,9 +175,9 @@ public class GroovyCategorySupport { // Precondition: accessorName.length() > prefixLength private Map<String, String> putPropertyAccessor(int prefixLength, String accessorName, Map<String, String> map) { if (map == null) { - map = new HashMap<String, String>(); + map = new HashMap<String, String>(); } - String property = accessorName.substring(prefixLength, prefixLength+1).toLowerCase() + accessorName.substring(prefixLength+1); + String property = BeanUtils.decapitalize(accessorName.substring(prefixLength)); map.put(property, accessorName); return map; } @@ -199,12 +200,16 @@ public class GroovyCategorySupport { } - String getPropertyCategoryGetterName(String propertyName){ - return propertyGetterMap != null ? propertyGetterMap.get(propertyName) : null; + String getPropertyCategoryGetterName(String propertyName) { + if (propertyGetterMap == null) return null; + String getter = propertyGetterMap.get(propertyName); + return getter != null ? getter : propertyGetterMap.get(BeanUtils.decapitalize(propertyName)); } - String getPropertyCategorySetterName(String propertyName){ - return propertySetterMap != null ? propertySetterMap.get(propertyName) : null; + String getPropertyCategorySetterName(String propertyName) { + if (propertySetterMap == null) return null; + String setter = propertySetterMap.get(propertyName); + return setter != null ? setter : propertySetterMap.get(BeanUtils.decapitalize(propertyName)); } } diff --git a/src/main/java/org/codehaus/groovy/transform/sc/transformers/BinaryExpressionTransformer.java b/src/main/java/org/codehaus/groovy/transform/sc/transformers/BinaryExpressionTransformer.java index 1124d70..8523424 100644 --- a/src/main/java/org/codehaus/groovy/transform/sc/transformers/BinaryExpressionTransformer.java +++ b/src/main/java/org/codehaus/groovy/transform/sc/transformers/BinaryExpressionTransformer.java @@ -254,8 +254,10 @@ public class BinaryExpressionTransformer { Character cRight = tryCharConstant(right); if (cLeft != null || cRight != null) { Expression oLeft = (cLeft == null ? left : constX(cLeft, true)); + if (oLeft instanceof PropertyExpression && !hasCharType((PropertyExpression)oLeft)) return null; oLeft.setSourcePosition(left); Expression oRight = (cRight == null ? right : constX(cRight, true)); + if (oRight instanceof PropertyExpression && !hasCharType((PropertyExpression)oRight)) return null; oRight.setSourcePosition(right); bin.setLeftExpression(oLeft); bin.setRightExpression(oRight); @@ -265,6 +267,11 @@ public class BinaryExpressionTransformer { return null; } + private static boolean hasCharType(PropertyExpression pe) { + ClassNode inferredType = pe.getNodeMetaData(StaticTypesMarker.INFERRED_RETURN_TYPE); + return inferredType != null && ClassHelper.Character_TYPE.equals(ClassHelper.getWrapper(inferredType)); + } + private static Character tryCharConstant(final Expression expr) { if (expr instanceof ConstantExpression && ClassHelper.STRING_TYPE.equals(expr.getType())) { String value = (String) ((ConstantExpression) expr).getValue(); diff --git a/src/test/groovy/PropertyTest.groovy b/src/test/groovy/PropertyTest.groovy index 010b33c..5efbc57 100644 --- a/src/test/groovy/PropertyTest.groovy +++ b/src/test/groovy/PropertyTest.groovy @@ -276,6 +276,279 @@ class PropertyTest extends GroovyTestCase { ''' } + // GROOVY-9618 + void testJavaBeanNamingForPropertyAccess() { + assertScript ''' + class A { + String getProp() { 'Prop' } + String getSomeProp() { 'SomeProp' } + String getX() { 'X' } + String getDB() { 'DB' } + String getXML() { 'XML' } + String getaProp() { 'aProp' } + String getAProp() { 'AProp' } + String getpNAME() { 'pNAME' } + } + new A().with { + assert prop == 'Prop' + assert Prop == 'Prop' + assert x == 'X' + assert X == 'X' + assert someProp == 'SomeProp' + assert SomeProp == 'SomeProp' + assert DB == 'DB' + assert XML == 'XML' + assert AProp == 'AProp' + assert aProp == 'aProp' + assert pNAME == 'pNAME' + } + ''' + } + + // GROOVY-9618 + void testJavaBeanNamingForPropertyAccessCS() { + assertScript ''' + class A { + String getProp() { 'Prop' } + String getSomeProp() { 'SomeProp' } + String getX() { 'X' } + String getDB() { 'DB' } + String getXML() { 'XML' } + String getaProp() { 'aProp' } + String getAProp() { 'AProp' } + String getpNAME() { 'pNAME' } + } + class B { + @groovy.transform.CompileStatic + def method() { + new A().with { + assert prop == 'Prop' + assert Prop == 'Prop' + assert x == 'X' + assert X == 'X' + assert someProp == 'SomeProp' + assert SomeProp == 'SomeProp' + assert DB == 'DB' + assert XML == 'XML' + assert AProp == 'AProp' + assert aProp == 'aProp' + assert pNAME == 'pNAME' + } + } + } + new B().method() + ''' + } + + // GROOVY-9618 + void testJavaBeanNamingForStaticPropertyAccess() { + assertScript ''' + class A { + static String getProp() { 'Prop' } + static String getSomeProp() { 'SomeProp' } + static String getX() { 'X' } + static String getDB() { 'DB' } + static String getXML() { 'XML' } + static String getaProp() { 'aProp' } + static String getAProp() { 'AProp' } + static String getpNAME() { 'pNAME' } + } + A.with { + assert prop == 'Prop' + assert Prop == 'Prop' + assert x == 'X' + assert X == 'X' + assert someProp == 'SomeProp' + assert SomeProp == 'SomeProp' + assert DB == 'DB' + assert XML == 'XML' + assert AProp == 'AProp' + assert aProp == 'aProp' + assert pNAME == 'pNAME' + } + ''' + } + + // GROOVY-9618 + void testJavaBeanNamingForStaticPropertyAccessCS() { + assertScript ''' + class A { + static String getProp() { 'Prop' } + static String getSomeProp() { 'SomeProp' } + static String getX() { 'X' } + static String getDB() { 'DB' } + static String getXML() { 'XML' } + static String getaProp() { 'aProp' } + static String getAProp() { 'AProp' } + static String getpNAME() { 'pNAME' } + } + class B { + @groovy.transform.CompileStatic + static method() { + A.with { + assert prop == 'Prop' + assert Prop == 'Prop' + assert x == 'X' // TODO fix CCE + assert X == 'X' // TODO fix CCE + assert someProp == 'SomeProp' + assert SomeProp == 'SomeProp' + assert DB == 'DB' + assert XML == 'XML' + assert AProp == 'AProp' + assert aProp == 'aProp' + assert pNAME == 'pNAME' + } + } + } + B.method() + ''' + } + + // GROOVY-9618 + void testJavaBeanNamingForPropertyStaticImport() { + assertScript ''' + class A { + static String getProp() { 'Prop' } + static String getSomeProp() { 'SomeProp' } + static String getX() { 'X' } + static String getDB() { 'DB' } + static String getXML() { 'XML' } + static String getaProp() { 'aProp' } + static String getAProp() { 'AProp' } + static String getpNAME() { 'pNAME' } + } + + import static A.* + + assert prop == 'Prop' + assert Prop == 'Prop' + assert x == 'X' + assert X == 'X' + assert someProp == 'SomeProp' + assert SomeProp == 'SomeProp' + assert DB == 'DB' + assert XML == 'XML' + assert AProp == 'AProp' + assert aProp == 'aProp' + assert pNAME == 'pNAME' + ''' + } + + // GROOVY-9618 + void testJavaBeanNamingForPropertyStaticImportCS() { + assertScript ''' + class A { + static String getProp() { 'Prop' } + static String getSomeProp() { 'SomeProp' } + static String getX() { 'X' } + static String getDB() { 'DB' } + static String getXML() { 'XML' } + static String getaProp() { 'aProp' } + static String getAProp() { 'AProp' } + static String getpNAME() { 'pNAME' } + } + + import static A.* + + class B { + @groovy.transform.CompileStatic + def method() { + assert prop == 'Prop' + assert Prop == 'Prop' + assert x == 'X' + assert X == 'X' + assert someProp == 'SomeProp' + assert SomeProp == 'SomeProp' + assert DB == 'DB' + assert XML == 'XML' + assert AProp == 'AProp' + assert aProp == 'aProp' + assert pNAME == 'pNAME' + } + } + new B().method() + ''' + } + + // GROOVY-9618 + void testJavaBeanNamingForPropertyAccessWithCategory() { + assertScript ''' + class A {} + class ACategory { + static String getProp(A self) { 'Prop' } + static String getSomeProp(A self) { 'SomeProp' } + static String getX(A self) { 'X' } + static String getDB(A self) { 'DB' } + static String getXML(A self) { 'XML' } + static String getaProp(A self) { 'aProp' } + static String getAProp(A self) { 'AProp' } + static String getpNAME(A self) { 'pNAME' } + } + use(ACategory) { + def a = new A() + assert a.prop == 'Prop' + assert a.Prop == 'Prop' + assert a.x == 'X' + assert a.X == 'X' + assert a.someProp == 'SomeProp' + assert a.SomeProp == 'SomeProp' + assert a.DB == 'DB' + assert a.XML == 'XML' + assert a.AProp == 'AProp' + assert a.aProp == 'aProp' + assert a.pNAME == 'pNAME' + } + ''' + } + + void testMissingPropertyWithStaticImport() { // GROOVY-5852+GROOVY-9618 + + def errMsg = shouldFail ''' + class Constants { + static final pi = 3.14 + } + + import static Constants.* + + assert PI.toString().startsWith('3') + ''' + + assert errMsg.contains('No such property: PI for class:') + } + + void testMissingPropertyWithDirectUsage() { // GROOVY-5852+GROOVY-9618 + def errMsg = shouldFail ''' + class Constants { + static final pi = 3.14 + } + + assert Constants.PI.toString().startsWith('3') + ''' + + assert errMsg.contains('No such property: PI for class: Constants') + } + + void testPropertyDirectUsageWithAllowableCaseChange() { // GROOVY-5852+GROOVY-9618 + assertScript ''' + class Constants { + static final pi = 3.14 + } + + assert Constants.Pi.toString().startsWith('3') + ''' + } + + void testPropertyStaticImportWithAllowableCaseChange() { // GROOVY-5852+GROOVY-9618 + assertScript ''' + class Constants { + static final pi = 3.14 + } + + import static Constants.* + + assert Pi.toString().startsWith('3') + ''' + } } class Base { diff --git a/src/test/groovy/bugs/Groovy5852Bug.groovy b/src/test/groovy/bugs/Groovy5852Bug.groovy deleted file mode 100644 index 1264880..0000000 --- a/src/test/groovy/bugs/Groovy5852Bug.groovy +++ /dev/null @@ -1,51 +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 Groovy5852Bug extends GroovyTestCase { - void testMissingProperty() { - def errMsg = shouldFail ''' - class Constants { - static final pi = 3.14 - } - - import static Constants.* - - println Pi - ''' - - assert errMsg.contains('No such property: Pi for class:') - } - - void testMissingProperty2() { - def errMsg = shouldFail ''' - class Constants { - static final pi = 3.14 - } - - import static Constants.* - - println Constants.Pi - ''' - - assert errMsg.contains('No such property: Pi for class: Constants') - } -} diff --git a/src/test/org/apache/groovy/util/BeanUtilsTest.groovy b/src/test/org/apache/groovy/util/BeanUtilsTest.groovy index 96a14ef..f40e2a5 100644 --- a/src/test/org/apache/groovy/util/BeanUtilsTest.groovy +++ b/src/test/org/apache/groovy/util/BeanUtilsTest.groovy @@ -40,6 +40,7 @@ class BeanUtilsTest { assert capitalize('Prop') == 'Prop' assert capitalize('prop') == 'Prop' assert capitalize('someProp') == 'SomeProp' + assert capitalize('X') == 'X' assert capitalize('DB') == 'DB' assert capitalize('XML') == 'XML' assert capitalize('aProp') == 'aProp' // GROOVY-3211
