This is an automated email from the ASF dual-hosted git repository. emilles pushed a commit to branch GROOVY-11563 in repository https://gitbox.apache.org/repos/asf/groovy.git
commit fb66b2cf0133a3a1c39a703251cf13addc7732f0 Author: Eric Milles <[email protected]> AuthorDate: Sun Feb 2 10:37:48 2025 -0600 GROOVY-11563: STC: check compound assignment `x op= y` like `x = x op y` --- src/main/groovy/groovy/grape/GrapeIvy.groovy | 5 ++- .../transform/stc/StaticTypeCheckingVisitor.java | 48 +++++++++++++++++----- .../groovy/transform/stc/STCAssignmentTest.groovy | 13 +++++- 3 files changed, 52 insertions(+), 14 deletions(-) diff --git a/src/main/groovy/groovy/grape/GrapeIvy.groovy b/src/main/groovy/groovy/grape/GrapeIvy.groovy index 75486334c9..750466de1d 100644 --- a/src/main/groovy/groovy/grape/GrapeIvy.groovy +++ b/src/main/groovy/groovy/grape/GrapeIvy.groovy @@ -57,6 +57,7 @@ import org.codehaus.groovy.runtime.metaclass.MetaClassRegistryImpl import org.w3c.dom.Element import javax.xml.parsers.DocumentBuilderFactory + import java.text.ParseException import java.util.jar.JarFile import java.util.regex.Pattern @@ -589,7 +590,7 @@ class GrapeIvy implements GrapeEngine { List<String> versions = [] moduleDir.eachFileMatch(ivyFilePattern) { File ivyFile -> def m = ivyFilePattern.matcher(ivyFile.getName()) - if (m.matches()) versions += m.group(1) + if (m.matches()) versions.add(m.group(1)) } grapes[moduleDir.getName()] = versions } @@ -656,7 +657,7 @@ class GrapeIvy implements GrapeEngine { for (ArtifactDownloadReport adl : report.getAllArtifactsReports()) { // TODO: check artifact type, jar vs library, etc. if (adl.getLocalFile()) { - results += adl.getLocalFile().toURI() + results.add(adl.getLocalFile().toURI()) } } 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 f0cdc32950..b1e2a7f861 100644 --- a/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java +++ b/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java @@ -65,6 +65,7 @@ import org.codehaus.groovy.ast.expr.DeclarationExpression; import org.codehaus.groovy.ast.expr.ElvisOperatorExpression; import org.codehaus.groovy.ast.expr.EmptyExpression; import org.codehaus.groovy.ast.expr.Expression; +import org.codehaus.groovy.ast.expr.ExpressionTransformer; import org.codehaus.groovy.ast.expr.FieldExpression; import org.codehaus.groovy.ast.expr.LambdaExpression; import org.codehaus.groovy.ast.expr.ListExpression; @@ -815,15 +816,48 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport { int op = expression.getOperation().getType(); Expression leftExpression = expression.getLeftExpression(); Expression rightExpression = expression.getRightExpression(); + Expression wrongExpression = null; // GROOVY-10628, GROOVY-11563 + + if (isAssignment(op) && op != EQUAL) { + // for "x op= y", find type as if it was "x = x op y" + ExpressionTransformer cloneMaker = new ExpressionTransformer() { + @Override public Expression transform(final Expression expr) { + // TODO: is there a beter way to clone? + if (expr instanceof VariableExpression) { + var variable = (VariableExpression) expr; + var copy = new VariableExpression(variable.getAccessedVariable()); + copy.setClosureSharedVariable(variable.isClosureSharedVariable()); + copy.setUseReferenceDirectly(variable.isUseReferenceDirectly()); + copy.setInStaticContext(variable.isInStaticContext()); + copy.setSynthetic(variable.isSynthetic()); + copy.setType(variable.getType()); + copy.setSourcePosition(expr); + copy.copyNodeMetaData(expr); + return copy; + } + return expr.transformExpression(this); + } + }; + if (op == ELVIS_EQUAL) { + wrongExpression = elvisX(cloneMaker.transform(leftExpression), rightExpression); + } else { + wrongExpression = binX(cloneMaker.transform(leftExpression), Token.newSymbol(TokenUtil.removeAssignment(op), expression.getOperation().getStartLine(), expression.getOperation().getStartColumn()), rightExpression); + } + wrongExpression.setSourcePosition(expression); + op = EQUAL; + } leftExpression.visit(this); SetterInfo setterInfo = removeSetterInfo(leftExpression); ClassNode lType = null; +// TODO: can visit and getType be split out of if/else for simpler structure? if (setterInfo != null) { if (ensureValidSetter(expression, leftExpression, rightExpression, setterInfo)) { return; } lType = getType(leftExpression); +// TODO: ensureValidSetter expands compound operator and visits rightExpression +if (wrongExpression != null) wrongExpression.visit(this); } else { if (op != EQUAL && op != ELVIS_EQUAL) { lType = getType(leftExpression); @@ -832,7 +866,9 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport { applyTargetType(lType, rightExpression); } - rightExpression.visit(this); +// TODO: want to visit before left expression above... +if (wrongExpression != null) wrongExpression.visit(this); +else rightExpression.visit(this); } ClassNode rType = isNullConstant(rightExpression) @@ -847,15 +883,7 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport { if (resultType == null) resultType = boolean_TYPE; // GROOVY-10239 storeTargetMethod(expression, reverseExpression.getNodeMetaData(DIRECT_METHOD_CALL_TARGET)); } else { - resultType = getResultType(lType, op, rType, expression); - if (op == ELVIS_EQUAL) { - // TODO: Should this transform and visit be done before left and right are visited above? - Expression fullExpression = new ElvisOperatorExpression(leftExpression, rightExpression); - fullExpression.setSourcePosition(expression); - fullExpression.visit(this); - - resultType = getType(fullExpression); - } + resultType = getResultType(lType, op, wrongExpression != null ? getType(wrongExpression) : rType, expression); } if (resultType == null) { resultType = lType; diff --git a/src/test/groovy/transform/stc/STCAssignmentTest.groovy b/src/test/groovy/transform/stc/STCAssignmentTest.groovy index 52d6594f08..670cc96374 100644 --- a/src/test/groovy/transform/stc/STCAssignmentTest.groovy +++ b/src/test/groovy/transform/stc/STCAssignmentTest.groovy @@ -341,10 +341,19 @@ class STCAssignmentTest extends StaticTypeCheckingTestCase { 'Cannot find matching method java.lang.Integer#minus(java.lang.Object)' } + // GROOVY-11563 + void testNumberPlusEqualsString() { + shouldFailWithMessages ''' + Number n = 0 + n += "no" + ''', + 'Cannot assign value of type java.lang.String to variable of type java.lang.Number' + } + void testStringPlusEqualsString() { assertScript ''' - String str = 'test' - str += 'test2' + String s = 'prefix' + s += 'suffix' ''' }
