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 2020/08/24 22:12:53 UTC
[groovy] branch master updated: GROOVY-9699: SC: reposition
checkOrMarkPrivateAccess for field reference
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 97ba01a GROOVY-9699: SC: reposition checkOrMarkPrivateAccess for field reference
97ba01a is described below
commit 97ba01a4d4237a8fc7863818d59fb793dbf7236c
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Mon Aug 24 17:12:40 2020 -0500
GROOVY-9699: SC: reposition checkOrMarkPrivateAccess for field reference
and drop extra visitation for "putAt" method call transformation
* reuse getType for field in visitVariableExpression and simplify its flow
closes #1349
---
...icTypesBinaryExpressionMultiTypeDispatcher.java | 2 -
.../transform/stc/StaticTypeCheckingVisitor.java | 89 ++++++++--------------
.../classgen/asm/sc/bugs/Groovy7361Bug.groovy | 29 +++++++
3 files changed, 59 insertions(+), 61 deletions(-)
diff --git a/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticTypesBinaryExpressionMultiTypeDispatcher.java b/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticTypesBinaryExpressionMultiTypeDispatcher.java
index 26685e5..9c835fd 100644
--- a/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticTypesBinaryExpressionMultiTypeDispatcher.java
+++ b/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticTypesBinaryExpressionMultiTypeDispatcher.java
@@ -44,7 +44,6 @@ import org.codehaus.groovy.classgen.asm.VariableSlotLoader;
import org.codehaus.groovy.classgen.asm.WriterController;
import org.codehaus.groovy.syntax.Token;
import org.codehaus.groovy.syntax.TokenUtil;
-import org.codehaus.groovy.transform.sc.StaticCompilationVisitor;
import org.objectweb.asm.Label;
import org.objectweb.asm.MethodVisitor;
import org.objectweb.asm.Opcodes;
@@ -393,7 +392,6 @@ public class StaticTypesBinaryExpressionMultiTypeDispatcher extends BinaryExpres
MethodCallExpression call = callX(receiver, "putAt", args(subscript, rhsValueLoader));
call.setSafe(safe);
call.setSourcePosition(enclosing);
- call.visit(new StaticCompilationVisitor(controller.getSourceUnit(), controller.getClassNode()));
OperandStack operandStack = controller.getOperandStack();
int height = operandStack.getStackLength();
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 6b69ee6..d237f31 100644
--- a/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
+++ b/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
@@ -189,8 +189,6 @@ import static org.codehaus.groovy.ast.tools.GeneralUtils.isOrImplements;
import static org.codehaus.groovy.ast.tools.GeneralUtils.localVarX;
import static org.codehaus.groovy.ast.tools.GeneralUtils.thisPropX;
import static org.codehaus.groovy.ast.tools.GeneralUtils.varX;
-import static org.codehaus.groovy.ast.tools.GenericsUtils.findActualTypeByGenericsPlaceholderName;
-import static org.codehaus.groovy.ast.tools.GenericsUtils.makeDeclaringAndActualGenericsTypeMapOfExactType;
import static org.codehaus.groovy.ast.tools.GenericsUtils.toGenericTypesString;
import static org.codehaus.groovy.ast.tools.WideningCategories.isBigDecCategory;
import static org.codehaus.groovy.ast.tools.WideningCategories.isBigIntCategory;
@@ -552,7 +550,10 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
final Variable accessedVariable = vexp.getAccessedVariable();
final TypeCheckingContext.EnclosingClosure enclosingClosure = typeCheckingContext.getEnclosingClosure();
- VariableExpression localVariableExpression = null;
+ if (accessedVariable == null) {
+ return;
+ }
+
if (accessedVariable instanceof DynamicVariable) {
// a dynamic variable is either a closure property, a class member referenced from a closure, or an undeclared variable
@@ -594,20 +595,14 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
if (enclosingClosure != null) {
tryVariableExpressionAsProperty(vexp, name);
} else {
+ checkOrMarkPrivateAccess(vexp, (FieldNode) accessedVariable, typeCheckingContext.isTargetOfEnclosingAssignment(vexp));
+
// GROOVY-9454
ClassNode inferredType = getInferredTypeFromTempInfo(vexp, null);
if (inferredType != null && !inferredType.equals(OBJECT_TYPE)) {
vexp.putNodeMetaData(INFERRED_RETURN_TYPE, inferredType);
} else {
- // GROOVY-7691
- FieldNode fieldNode = (FieldNode) accessedVariable;
- ClassNode fieldType = findActualTypeByGenericsPlaceholderName(
- fieldNode.getOriginType().getUnresolvedName(),
- makeDeclaringAndActualGenericsTypeMapOfExactType(fieldNode.getDeclaringClass(), typeCheckingContext.getEnclosingClassNode())
- );
- if (fieldType != null) {
- storeType(vexp, fieldType);
- }
+ storeType(vexp, getType(vexp));
}
}
} else if (accessedVariable instanceof PropertyNode) {
@@ -621,25 +616,21 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
SetterInfo setterInfo = removeSetterInfo(leftExpression);
if (setterInfo != null) {
Expression rightExpression = enclosingBinaryExpression.getRightExpression();
- if (!ensureValidSetter(vexp, leftExpression, rightExpression, setterInfo)) {
- return;
- }
+ ensureValidSetter(vexp, leftExpression, rightExpression, setterInfo);
}
}
}
- } else if (accessedVariable instanceof Parameter) {
- if (enclosingClosure == null) {
- localVariableExpression = new ParameterVariableExpression((Parameter) accessedVariable);
- }
- } else if (accessedVariable instanceof VariableExpression) {
- if (enclosingClosure == null) {
- localVariableExpression = (VariableExpression) accessedVariable;
+ } else if (enclosingClosure == null) {
+ VariableExpression localVariable;
+ if (accessedVariable instanceof Parameter) {
+ Parameter parameter = (Parameter) accessedVariable;
+ localVariable = new ParameterVariableExpression(parameter);
+ } else {
+ localVariable = (VariableExpression) accessedVariable;
}
- }
- if (localVariableExpression != null) {
- ClassNode inferredType = localVariableExpression.getNodeMetaData(INFERRED_TYPE);
- inferredType = getInferredTypeFromTempInfo(localVariableExpression, inferredType);
+ ClassNode inferredType = localVariable.getNodeMetaData(INFERRED_TYPE);
+ inferredType = getInferredTypeFromTempInfo(localVariable, inferredType);
if (inferredType != null && !inferredType.equals(OBJECT_TYPE)) {
vexp.putNodeMetaData(INFERRED_RETURN_TYPE, inferredType);
}
@@ -1265,12 +1256,7 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
ClassNode inferredRightExpressionType = inferredRightExpressionTypeOrig;
if (!typeCheckMultipleAssignmentAndContinue(leftExpression, rightExpression)) return;
- if (leftExpression instanceof VariableExpression
- && ((VariableExpression) leftExpression).getAccessedVariable() instanceof FieldNode) {
- checkOrMarkPrivateAccess(leftExpression, (FieldNode) ((VariableExpression) leftExpression).getAccessedVariable(), true);
- }
-
- //TODO: need errors for write-only too!
+ // TODO: need errors for write-only too!
if (addedReadOnlyPropertyError(leftExpression)) return;
ClassNode leftRedirect = leftExpressionType.redirect();
@@ -4811,9 +4797,15 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
if (vexp.isSuperExpression()) return makeSuper();
Variable variable = vexp.getAccessedVariable();
if (variable instanceof FieldNode) {
- FieldNode fieldNode = (FieldNode) variable;
- checkOrMarkPrivateAccess(vexp, fieldNode, typeCheckingContext.isTargetOfEnclosingAssignment(vexp));
- return getType(fieldNode);
+ ClassNode fieldType = variable.getOriginType();
+ if (isUsingGenericsOrIsArrayUsingGenerics(fieldType)) {
+ boolean isStatic = (variable.getModifiers() & Opcodes.ACC_STATIC) != 0;
+ ClassNode thisType = typeCheckingContext.getEnclosingClassNode(), declType = ((FieldNode) variable).getDeclaringClass();
+ Map<GenericsTypeName, GenericsType> placeholders = resolvePlaceHoldersFromDeclaration(thisType, declType, null, isStatic);
+
+ fieldType = resolveGenericsWithContext(placeholders, fieldType);
+ }
+ return fieldType;
}
if (variable != vexp && variable instanceof VariableExpression) {
return getType((Expression) variable);
@@ -4856,6 +4848,9 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
ClassNode ret = getInferredReturnType(exp);
return ret != null ? ret : ((MethodNode) exp).getReturnType();
}
+ if (exp instanceof FieldNode || exp instanceof PropertyNode) {
+ return ((Variable) exp).getOriginType();
+ }
if (exp instanceof RangeExpression) {
ClassNode plain = RANGE_TYPE.getPlainNodeReference();
RangeExpression re = (RangeExpression) exp;
@@ -4884,14 +4879,6 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
if (exp instanceof Parameter) {
return ((Parameter) exp).getOriginType();
}
- if (exp instanceof FieldNode) {
- FieldNode fn = (FieldNode) exp;
- return getGenericsResolvedTypeOfFieldOrProperty(fn, fn.getOriginType());
- }
- if (exp instanceof PropertyNode) {
- PropertyNode pn = (PropertyNode) exp;
- return getGenericsResolvedTypeOfFieldOrProperty(pn, pn.getOriginType());
- }
if (exp instanceof ClosureExpression) {
ClassNode irt = getInferredReturnType(exp);
if (irt != null) {
@@ -4931,22 +4918,6 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
return null;
}
- /**
- * resolves a Field or Property node generics by using the current class and
- * the declaring class to extract the right meaning of the generics symbols
- *
- * @param an a FieldNode or PropertyNode
- * @param type the origin type
- * @return the new ClassNode with corrected generics
- */
- private ClassNode getGenericsResolvedTypeOfFieldOrProperty(final AnnotatedNode an, final ClassNode type) {
- if (!type.isUsingGenerics()) return type;
- Map<GenericsTypeName, GenericsType> connections = new HashMap<>();
- // TODO: inner classes mean a different this-type. This is ignored here!
- extractGenericsConnections(connections, typeCheckingContext.getEnclosingClassNode(), an.getDeclaringClass());
- return applyGenericsContext(connections, type);
- }
-
private static ClassNode makeSelf(final ClassNode trait) {
ClassNode selfType = trait;
Set<ClassNode> selfTypes = Traits.collectSelfTypes(selfType, new LinkedHashSet<>());
diff --git a/src/test/org/codehaus/groovy/classgen/asm/sc/bugs/Groovy7361Bug.groovy b/src/test/org/codehaus/groovy/classgen/asm/sc/bugs/Groovy7361Bug.groovy
index e7f26a4..9878851 100644
--- a/src/test/org/codehaus/groovy/classgen/asm/sc/bugs/Groovy7361Bug.groovy
+++ b/src/test/org/codehaus/groovy/classgen/asm/sc/bugs/Groovy7361Bug.groovy
@@ -58,4 +58,33 @@ final class Groovy7361Bug extends StaticTypeCheckingTestCase implements StaticCo
assert new A().m() == [x:'y']
'''
}
+
+ // GROOVY-9699
+ void testShouldNotEmitErrorForSubscriptPrivateAccess() {
+ assertScript '''
+ class A {
+ private static final java.util.regex.Pattern PATTERN = ~/.*/
+ void checkList() {
+ def list = []
+ def closure = { ->
+ list << PATTERN.pattern()
+ }
+ closure()
+ }
+ void checkMap() {
+ def map = [:]
+ def closure = { ->
+ map[PATTERN.pattern()] = 1
+ }
+ closure()
+ }
+ }
+ class B extends A {
+ }
+ new A().checkList()
+ new B().checkList()
+ new A().checkMap()
+ new B().checkMap()
+ '''
+ }
}