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/20 20:25:54 UTC
[groovy] 01/01: 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 GROOVY-9699
in repository https://gitbox.apache.org/repos/asf/groovy.git
commit 8de9b7edc711eb13e59618e1474d308472cfc025
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Thu Aug 20 15:23:12 2020 -0500
GROOVY-9699: SC: reposition checkOrMarkPrivateAccess for field reference
and drop extra visitation for "putAt" method call transformation
---
...icTypesBinaryExpressionMultiTypeDispatcher.java | 2 -
.../transform/stc/StaticTypeCheckingVisitor.java | 48 +++++++---------------
.../classgen/asm/sc/bugs/Groovy7361Bug.groovy | 29 +++++++++++++
3 files changed, 44 insertions(+), 35 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..a2e814d 100644
--- a/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
+++ b/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
@@ -594,6 +594,8 @@ 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)) {
@@ -1265,12 +1267,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 +4808,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 +4859,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 +4890,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 +4929,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()
+ '''
+ }
}