You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@groovy.apache.org by pa...@apache.org on 2020/07/18 03:26:36 UTC
[groovy] 01/03: GROOVY-9645: Inconsistencies in JavaBean naming for
property access
This is an automated email from the ASF dual-hosted git repository.
paulk pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/groovy.git
commit b7c273949732de6259997be9017e402ae0e8cd3b
Author: Paul King <pa...@asert.com.au>
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