You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@groovy.apache.org by em...@apache.org on 2021/02/24 19:11:02 UTC
[groovy] branch master updated: refactor assignment checking
This is an automated email from the ASF dual-hosted git repository.
emilles pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/groovy.git
The following commit(s) were added to refs/heads/master by this push:
new 1fcbd7e refactor assignment checking
1fcbd7e is described below
commit 1fcbd7ed874ee2a792870644276c7a89a416f379
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Wed Feb 24 13:10:48 2021 -0600
refactor assignment checking
---
.../transform/stc/StaticTypeCheckingSupport.java | 33 +++++++++----------
.../transform/stc/StaticTypeCheckingVisitor.java | 37 +++++++++++-----------
2 files changed, 34 insertions(+), 36 deletions(-)
diff --git a/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingSupport.java b/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingSupport.java
index 07a73e2..db1031a 100644
--- a/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingSupport.java
+++ b/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingSupport.java
@@ -645,6 +645,15 @@ public abstract class StaticTypeCheckingSupport {
}
public static boolean checkCompatibleAssignmentTypes(final ClassNode left, final ClassNode right, final Expression rightExpression, final boolean allowConstructorCoercion) {
+ // GROOVY-7307, GROOVY-9952, et al.
+ if (left.isGenericsPlaceHolder()) {
+ GenericsType[] genericsTypes = left.getGenericsTypes();
+ // should always be the case, but better safe than sorry
+ if (genericsTypes != null && genericsTypes.length == 1) {
+ return genericsTypes[0].isCompatibleWith(right);
+ }
+ }
+
ClassNode leftRedirect = left.redirect();
ClassNode rightRedirect = right.redirect();
if (leftRedirect == rightRedirect) return true;
@@ -653,9 +662,8 @@ public abstract class StaticTypeCheckingSupport {
return checkCompatibleAssignmentTypes(leftRedirect.getComponentType(), rightRedirect.getComponentType(), rightExpression, false);
}
- if (right == VOID_TYPE || right == void_WRAPPER_TYPE) {
- return left == VOID_TYPE || left == void_WRAPPER_TYPE;
- }
+ if (rightRedirect == void_WRAPPER_TYPE) return leftRedirect == VOID_TYPE;
+ if (rightRedirect == VOID_TYPE) return leftRedirect == void_WRAPPER_TYPE;
if (isNumberType(rightRedirect) || WideningCategories.isNumberCategory(rightRedirect)) {
if (BigDecimal_TYPE == leftRedirect || Number_TYPE == leftRedirect) {
@@ -676,7 +684,7 @@ public abstract class StaticTypeCheckingSupport {
// on an assignment everything that can be done by a GroovyCast is allowed
// anything can be assigned to an Object, String, Boolean or Class typed variable
- if (isWildcardLeftHandSide(leftRedirect) && !(boolean_TYPE.equals(left) && rightExpressionIsNull)) return true;
+ if (isWildcardLeftHandSide(left) && !(leftRedirect == boolean_TYPE && rightExpressionIsNull)) return true;
// char as left expression
if (leftRedirect == char_TYPE && rightRedirect == STRING_TYPE) {
@@ -689,17 +697,15 @@ public abstract class StaticTypeCheckingSupport {
return rightExpressionIsNull || (rightExpression instanceof ConstantExpression && rightExpression.getText().length() == 1);
}
- // if left is Enum and right is String or GString we do valueOf
- if (leftRedirect.isDerivedFrom(Enum_Type) && (rightRedirect == GSTRING_TYPE || rightRedirect == STRING_TYPE)) {
+ // if left is an enum and right is String or GString we do valueOf
+ if (leftRedirect.isDerivedFrom(Enum_Type) && (rightRedirect == STRING_TYPE || rightRedirect == GSTRING_TYPE)) {
return true;
}
// if right is array, map or collection we try invoking the constructor
if (allowConstructorCoercion && isGroovyConstructorCompatible(rightExpression)) {
// TODO: in case of the array we could maybe make a partial check
- if (leftRedirect.isArray() && rightRedirect.isArray()) {
- return checkCompatibleAssignmentTypes(leftRedirect.getComponentType(), rightRedirect.getComponentType());
- } else if (rightRedirect.isArray() && !leftRedirect.isArray()) {
+ if (rightRedirect.isArray() && !leftRedirect.isArray()) {
return false;
}
return true;
@@ -721,15 +727,6 @@ public abstract class StaticTypeCheckingSupport {
return true;
}
- if (left.isGenericsPlaceHolder()) {
- // GROOVY-7307
- GenericsType[] genericsTypes = left.getGenericsTypes();
- if (genericsTypes != null && genericsTypes.length == 1) {
- // should always be the case, but safe guard is better
- return genericsTypes[0].isCompatibleWith(right);
- }
- }
-
// GROOVY-7316: It is an apparently legal thing to allow this. It's not type safe, but it is allowed...
return right.isGenericsPlaceHolder();
}
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 ce53caa..e6fd34f 100644
--- a/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
+++ b/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
@@ -1250,31 +1250,32 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
return false;
}
- protected void typeCheckAssignment(final BinaryExpression assignmentExpression, final Expression leftExpression, final ClassNode leftExpressionType, final Expression rightExpression, final ClassNode inferredRightExpressionTypeOrig) {
- ClassNode inferredRightExpressionType = inferredRightExpressionTypeOrig;
+ protected void typeCheckAssignment(final BinaryExpression assignmentExpression, final Expression leftExpression, final ClassNode leftExpressionType, final Expression rightExpression, final ClassNode rightExpressionType) {
if (!typeCheckMultipleAssignmentAndContinue(leftExpression, rightExpression)) return;
// TODO: need errors for write-only too!
if (addedReadOnlyPropertyError(leftExpression)) return;
- ClassNode leftRedirect = leftExpressionType.redirect();
- // see if instanceof applies
- if (rightExpression instanceof VariableExpression && hasInferredReturnType(rightExpression) && assignmentExpression.getOperation().getType() == EQUAL) {
- inferredRightExpressionType = rightExpression.getNodeMetaData(INFERRED_RETURN_TYPE);
+ ClassNode rTypeInferred, rTypeWrapped; // check for instanceof and spreading
+ if (rightExpression instanceof VariableExpression && hasInferredReturnType(rightExpression) && assignmentExpression.getOperation().getType() == ASSIGN) {
+ rTypeInferred = rightExpression.getNodeMetaData(INFERRED_RETURN_TYPE);
+ } else {
+ rTypeInferred = rightExpressionType;
}
- ClassNode wrappedRHS = adjustTypeForSpreading(inferredRightExpressionType, leftExpression);
+ rTypeWrapped = adjustTypeForSpreading(rTypeInferred, leftExpression);
- // check types are compatible for assignment
- if (!checkCompatibleAssignmentTypes(leftRedirect, wrappedRHS, rightExpression)) {
- if (!extension.handleIncompatibleAssignment(leftExpressionType, inferredRightExpressionType, assignmentExpression)) {
- addAssignmentError(leftExpressionType, inferredRightExpressionType, assignmentExpression.getRightExpression());
+ if (!checkCompatibleAssignmentTypes(leftExpressionType, rTypeWrapped, rightExpression)) {
+ if (!extension.handleIncompatibleAssignment(leftExpressionType, rTypeInferred, assignmentExpression)) {
+ addAssignmentError(leftExpressionType, rTypeInferred, rightExpression);
}
} else {
- addPrecisionErrors(leftRedirect, leftExpressionType, inferredRightExpressionType, rightExpression);
- addListAssignmentConstructorErrors(leftRedirect, leftExpressionType, inferredRightExpressionType, rightExpression, assignmentExpression);
- addMapAssignmentConstructorErrors(leftRedirect, leftExpression, rightExpression);
- if (hasGStringStringError(leftExpressionType, wrappedRHS, rightExpression)) return;
- checkTypeGenerics(leftExpressionType, wrappedRHS, rightExpression);
+ ClassNode lTypeRedirect = leftExpressionType.redirect();
+ addPrecisionErrors(lTypeRedirect, leftExpressionType, rTypeInferred, rightExpression);
+ addListAssignmentConstructorErrors(lTypeRedirect, leftExpressionType, rTypeInferred, rightExpression, assignmentExpression);
+ addMapAssignmentConstructorErrors(lTypeRedirect, leftExpression, rightExpression);
+ if (!hasGStringStringError(leftExpressionType, rTypeWrapped, rightExpression)) {
+ checkTypeGenerics(leftExpressionType, rTypeWrapped, rightExpression);
+ }
}
}
@@ -5582,8 +5583,8 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
addStaticTypeError("Due to their dynamic nature, usage of categories is not possible with static type checking active", call);
}
- protected void addAssignmentError(final ClassNode leftType, final ClassNode rightType, final Expression assignmentExpression) {
- addStaticTypeError("Cannot assign value of type " + prettyPrintType(rightType) + " to variable of type " + prettyPrintType(leftType), assignmentExpression);
+ protected void addAssignmentError(final ClassNode leftType, final ClassNode rightType, final Expression expression) {
+ addStaticTypeError("Cannot assign value of type " + prettyPrintType(rightType) + " to variable of type " + prettyPrintType(leftType), expression);
}
protected void addUnsupportedPreOrPostfixExpressionError(final Expression expression) {