From d56990f04999a8495b0dde2f3bf1efa42886201b Mon Sep 17 00:00:00 2001 From: Eric Milles Date: Thu, 2 Nov 2023 14:33:49 -0500 Subject: [PATCH] GROOVY-11006: STC: setter assignment conversion 3_0_X backport --- .../stc/StaticTypeCheckingVisitor.java | 66 ++++++++++--------- .../stc/FieldsAndPropertiesSTCTest.groovy | 2 +- .../transform/stc/GenericsSTCTest.groovy | 2 +- .../transform/stc/STCAssignmentTest.groovy | 30 ++++++--- 4 files changed, 56 insertions(+), 44 deletions(-) diff --git a/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java b/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java index c771047bf66..648d1066dda 100644 --- a/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java +++ b/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java @@ -976,20 +976,31 @@ private boolean ensureValidSetter(final Expression expression, final Expression // we know that the RHS type is a closure // but we must check if the binary expression is an assignment // because we need to check if a setter uses @DelegatesTo - VariableExpression ve = varX("%", setterInfo.receiverType); - ve.setType(setterInfo.receiverType); // same as origin type + VariableExpression receiver = varX("%", setterInfo.receiverType); + receiver.setType(setterInfo.receiverType); // same as origin type - Function firstParamType = (method) -> { - ClassNode type = method.getParameters()[0].getOriginType(); - if (!method.isStatic() && !(method instanceof ExtensionMethodNode) && GenericsUtils.hasUnresolvedGenerics(type)) { - Map spec = extractPlaceHolders(setterInfo.receiverType, method.getDeclaringClass()); - type = applyGenericsContext(spec, type); + Function setterCall = (value) -> { + typeCheckingContext.pushEnclosingBinaryExpression(null); // GROOVY-10628 + try { + MethodCallExpression call = callX(receiver, setterInfo.name, value); + call.setImplicitThis(false); + visitMethodCallExpression(call); + return call.getNodeMetaData(DIRECT_METHOD_CALL_TARGET); + } finally { + typeCheckingContext.popEnclosingBinaryExpression(); + } + }; + + Function setterType = (setter) -> { + ClassNode type = setter.getParameters()[0].getOriginType(); + if (!setter.isStatic() && !(setter instanceof ExtensionMethodNode) && GenericsUtils.hasUnresolvedGenerics(type)) { + type = applyGenericsContext(extractPlaceHolders(setterInfo.receiverType, setter.getDeclaringClass()), type); } return type; }; - // for compound assignment "x op= y" find type as if it was "x = (x op y)" Expression valueExpression = rightExpression; + // for "x op= y", find type as if it was "x = x op y" if (isCompoundAssignment(expression)) { Token op = ((BinaryExpression) expression).getOperation(); if (op.getType() == ELVIS_EQUAL) { // GROOVY-10419: "x ?= y" @@ -999,43 +1010,34 @@ private boolean ensureValidSetter(final Expression expression, final Expression valueExpression = binX(leftExpression, op, rightExpression); } } - MethodCallExpression call = new MethodCallExpression(ve, setterInfo.name, valueExpression); - typeCheckingContext.pushEnclosingBinaryExpression(null); // GROOVY-10628: LHS re-purposed - try { - call.setImplicitThis(false); - visitMethodCallExpression(call); - } finally { - typeCheckingContext.popEnclosingBinaryExpression(); - } - MethodNode directSetterCandidate = call.getNodeMetaData(DIRECT_METHOD_CALL_TARGET); - if (directSetterCandidate == null) { - // this may happen if there's a setter of type boolean/String/Class, and that we are using the property - // notation AND that the RHS is not a boolean/String/Class + + MethodNode methodTarget = setterCall.apply(valueExpression); + if (methodTarget == null && !isCompoundAssignment(expression)) { + // if no direct match, try implicit conversion for (MethodNode setter : setterInfo.setters) { - ClassNode type = getWrapper(firstParamType.apply(setter)); - if (Boolean_TYPE.equals(type) || STRING_TYPE.equals(type) || CLASS_Type.equals(type)) { - call = new MethodCallExpression(ve, setterInfo.name, castX(type, valueExpression)); - call.setImplicitThis(false); - visitMethodCallExpression(call); - directSetterCandidate = call.getNodeMetaData(DIRECT_METHOD_CALL_TARGET); - if (directSetterCandidate != null) { + ClassNode lType = setterType.apply(setter); + ClassNode rType = getDeclaredOrInferredType(valueExpression); + if (checkCompatibleAssignmentTypes(lType, rType, valueExpression, false)) { + methodTarget = setterCall.apply(castX(lType, valueExpression)); + if (methodTarget != null) { break; } } } } - if (directSetterCandidate != null) { + + if (methodTarget != null) { for (MethodNode setter : setterInfo.setters) { - if (setter == directSetterCandidate) { - leftExpression.putNodeMetaData(DIRECT_METHOD_CALL_TARGET, directSetterCandidate); + if (setter == methodTarget) { + leftExpression.putNodeMetaData(DIRECT_METHOD_CALL_TARGET, methodTarget); leftExpression.removeNodeMetaData(INFERRED_TYPE); // clear assumption - storeType(leftExpression, firstParamType.apply(setter)); + storeType(leftExpression, setterType.apply(methodTarget)); break; } } return false; } else { - ClassNode firstSetterType = firstParamType.apply(setterInfo.setters.get(0)); + ClassNode firstSetterType = setterType.apply(setterInfo.setters.get(0)); addAssignmentError(firstSetterType, getType(valueExpression), expression); return true; } diff --git a/src/test/groovy/transform/stc/FieldsAndPropertiesSTCTest.groovy b/src/test/groovy/transform/stc/FieldsAndPropertiesSTCTest.groovy index 9da08b8eacf..d4036a64990 100644 --- a/src/test/groovy/transform/stc/FieldsAndPropertiesSTCTest.groovy +++ b/src/test/groovy/transform/stc/FieldsAndPropertiesSTCTest.groovy @@ -1199,7 +1199,7 @@ class FieldsAndPropertiesSTCTest extends StaticTypeCheckingTestCase { new FooWorker().doSomething() ''', - 'Cannot assign value of type java.util.ArrayList to variable of type java.util.List ' + 'Cannot assign java.util.ArrayList to: java.util.List ' } void testAICAsStaticProperty() { diff --git a/src/test/groovy/transform/stc/GenericsSTCTest.groovy b/src/test/groovy/transform/stc/GenericsSTCTest.groovy index 2c3b41166ee..94e52cf887f 100644 --- a/src/test/groovy/transform/stc/GenericsSTCTest.groovy +++ b/src/test/groovy/transform/stc/GenericsSTCTest.groovy @@ -2561,7 +2561,7 @@ class GenericsSTCTest extends StaticTypeCheckingTestCase { x = Collections.emptyList() } """, - 'Cannot assign value of type java.util.List to variable of type java.util.List ' + 'Cannot assign java.util.List to: java.util.List ' } } diff --git a/src/test/groovy/transform/stc/STCAssignmentTest.groovy b/src/test/groovy/transform/stc/STCAssignmentTest.groovy index 3ab0838d501..ab4e634a4a4 100644 --- a/src/test/groovy/transform/stc/STCAssignmentTest.groovy +++ b/src/test/groovy/transform/stc/STCAssignmentTest.groovy @@ -168,14 +168,12 @@ class STCAssignmentTest extends StaticTypeCheckingTestCase { assertScript ''' class C { int i - - static main(args) { - def c = new C() - c.i = 5 - c.i += 10 - assert c.i == 15 - } } + def c = new C(i: 5) + def ret = c.i += 10 + + assert c.i == 15 + assert ret == 15 ''' } @@ -768,10 +766,22 @@ class STCAssignmentTest extends StaticTypeCheckingTestCase { assertScript ''' double m() { double a = 10d - float b = 1f + float b = 1f double c = a+b } - assert m()==11d + assert m() == 11.0 + ''' + } + + // GROOVY-11006 + void testDoubleVsLiteral() { + assertScript ''' + void setValue(double d) { + } + final double zero = 0.0 + value = zero + value = 1.1 + value = 1 ''' } @@ -864,7 +874,7 @@ class STCAssignmentTest extends StaticTypeCheckingTestCase { ''' } - //GROOVY-6435 + // GROOVY-6435 void testBigDecAndBigIntSubclass() { assertScript ''' class MyDecimal extends BigDecimal {