GROOVY-8132: Owner properties should be preferred over properties of @Delegate field (closes #518) * Add test for method delegation precedence
Project: http://git-wip-us.apache.org/repos/asf/groovy/repo Commit: http://git-wip-us.apache.org/repos/asf/groovy/commit/ca83d508 Tree: http://git-wip-us.apache.org/repos/asf/groovy/tree/ca83d508 Diff: http://git-wip-us.apache.org/repos/asf/groovy/diff/ca83d508 Branch: refs/heads/parrot Commit: ca83d508bf4d670a5654f10bc253f30b6caf4c16 Parents: a1fac74 Author: Shil Sinha <[email protected]> Authored: Wed Mar 22 13:26:24 2017 -0400 Committer: Shil Sinha <[email protected]> Committed: Sat Apr 15 16:15:21 2017 -0400 ---------------------------------------------------------------------- .../transform/DelegateASTTransformation.java | 33 ++++++++++++++------ .../transform/DelegateTransformTest.groovy | 30 ++++++++++++++++++ 2 files changed, 53 insertions(+), 10 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/groovy/blob/ca83d508/src/main/org/codehaus/groovy/transform/DelegateASTTransformation.java ---------------------------------------------------------------------- diff --git a/src/main/org/codehaus/groovy/transform/DelegateASTTransformation.java b/src/main/org/codehaus/groovy/transform/DelegateASTTransformation.java index 949a260..8e1daa5 100644 --- a/src/main/org/codehaus/groovy/transform/DelegateASTTransformation.java +++ b/src/main/org/codehaus/groovy/transform/DelegateASTTransformation.java @@ -22,6 +22,7 @@ import groovy.lang.Delegate; import groovy.lang.GroovyObject; import groovy.lang.Lazy; +import groovy.lang.Reference; import org.codehaus.groovy.ast.ASTNode; import org.codehaus.groovy.ast.AnnotatedNode; import org.codehaus.groovy.ast.AnnotationNode; @@ -187,7 +188,7 @@ public class DelegateASTTransformation extends AbstractASTTransformation { private static void addSetterIfNeeded(DelegateDescription delegate, PropertyNode prop, String name, boolean allNames) { String setterName = "set" + Verifier.capitalize(name); if ((prop.getModifiers() & ACC_FINAL) == 0 - && delegate.owner.getSetterMethod(setterName) == null + && delegate.owner.getSetterMethod(setterName) == null && delegate.owner.getProperty(name) == null && !shouldSkipPropertyMethod(name, setterName, delegate.excludes, delegate.includes, allNames)) { delegate.owner.addMethod(setterName, ACC_PUBLIC, @@ -212,21 +213,33 @@ public class DelegateASTTransformation extends AbstractASTTransformation { if (cNode.getGetterMethod("get" + suffix) != null && cNode.getGetterMethod("is" + suffix) == null) willHaveIsAccessor = false; } + Reference<Boolean> ownerWillHaveGetAccessor = new Reference<Boolean>(); + Reference<Boolean> ownerWillHaveIsAccessor = new Reference<Boolean>(); + extractAccessorInfo(delegate.owner, name, ownerWillHaveGetAccessor, ownerWillHaveIsAccessor); + for (String prefix : new String[]{"get", "is"}) { String getterName = prefix + suffix; - if (delegate.owner.getGetterMethod(getterName) == null + if ((prefix.equals("get") && willHaveGetAccessor && !ownerWillHaveGetAccessor.get() + || prefix.equals("is") && willHaveIsAccessor && !ownerWillHaveIsAccessor.get()) && !shouldSkipPropertyMethod(name, getterName, delegate.excludes, delegate.includes, allNames)) { - if (prefix.equals("get") && willHaveGetAccessor || prefix.equals("is") && willHaveIsAccessor) { - delegate.owner.addMethod(getterName, - ACC_PUBLIC, - GenericsUtils.nonGeneric(prop.getType()), - Parameter.EMPTY_ARRAY, - null, - returnS(propX(delegate.getOp, name))); - } + delegate.owner.addMethod(getterName, + ACC_PUBLIC, + GenericsUtils.nonGeneric(prop.getType()), + Parameter.EMPTY_ARRAY, + null, + returnS(propX(delegate.getOp, name))); } } } + + private static void extractAccessorInfo(ClassNode owner, String name, Reference<Boolean> willHaveGetAccessor, Reference<Boolean> willHaveIsAccessor) { + String suffix = Verifier.capitalize(name); + boolean hasGetAccessor = owner.getGetterMethod("get" + suffix) != null; + boolean hasIsAccessor = owner.getGetterMethod("is" + suffix) != null; + PropertyNode prop = owner.getProperty(name); + willHaveGetAccessor.set(hasGetAccessor || (prop != null && !hasIsAccessor)); + willHaveIsAccessor.set(hasIsAccessor || (prop != null && !hasGetAccessor && prop.getOriginType().equals(ClassHelper.boolean_TYPE))); + } private static boolean shouldSkipPropertyMethod(String propertyName, String methodName, List<String> excludes, List<String> includes, boolean allNames) { return ((!allNames && deemedInternalName(propertyName)) http://git-wip-us.apache.org/repos/asf/groovy/blob/ca83d508/src/test/org/codehaus/groovy/transform/DelegateTransformTest.groovy ---------------------------------------------------------------------- diff --git a/src/test/org/codehaus/groovy/transform/DelegateTransformTest.groovy b/src/test/org/codehaus/groovy/transform/DelegateTransformTest.groovy index 646223a..7b5964a 100644 --- a/src/test/org/codehaus/groovy/transform/DelegateTransformTest.groovy +++ b/src/test/org/codehaus/groovy/transform/DelegateTransformTest.groovy @@ -780,6 +780,36 @@ assert foo.dm.x == '123' assert b.getC() """ } + + void testOwnerPropertyPreferredToDelegateProperty() { + assertScript ''' + class Foo { + String pls + @groovy.lang.Delegate + Bar bar + } + + class Bar { + String pls + } + assert new Foo(pls: 'ok').pls == 'ok' + ''' + } + + void testOwnerMethodPreferredToDelegateMethod() { + assertScript ''' + class Foo { + String pls() { 'foo pls' } + @groovy.lang.Delegate + Bar bar + } + + class Bar { + String pls() { 'bar pls' } + } + assert new Foo(bar: new Bar()).pls() == 'foo pls' + ''' + } } interface DelegateFoo {
