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/09/25 18:16:44 UTC
[groovy] 01/02: 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_2_5_X
in repository https://gitbox.apache.org/repos/asf/groovy.git
commit 468ac801ef03dd5cb1430a417d3641d7ace5ec11
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Fri Sep 25 13:06:46 2020 -0500
GROOVY-9699: SC: reposition checkOrMarkPrivateAccess for field reference
---
...icTypesBinaryExpressionMultiTypeDispatcher.java | 11 +--
.../transform/stc/StaticTypeCheckingVisitor.java | 94 ++++------------------
.../classgen/asm/sc/bugs/Groovy7361Bug.groovy | 32 +++++++-
3 files changed, 50 insertions(+), 87 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 7168996..8ab997b 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
@@ -51,7 +51,6 @@ import org.codehaus.groovy.classgen.asm.WriterController;
import org.codehaus.groovy.runtime.MetaClassHelper;
import org.codehaus.groovy.syntax.Token;
import org.codehaus.groovy.transform.sc.StaticCompilationMetadataKeys;
-import org.codehaus.groovy.transform.sc.StaticCompilationVisitor;
import org.codehaus.groovy.transform.stc.StaticTypeCheckingSupport;
import org.codehaus.groovy.transform.stc.StaticTypeCheckingVisitor;
import org.codehaus.groovy.transform.stc.StaticTypesMarker;
@@ -375,7 +374,6 @@ public class StaticTypesBinaryExpressionMultiTypeDispatcher extends BinaryExpres
*******/
WriterController controller = getController();
- StaticTypeCheckingVisitor visitor = new StaticCompilationVisitor(controller.getSourceUnit(), controller.getClassNode());
// let's replace this assignment to a subscript operator with a
// method call
// e.g. x[5] = 10
@@ -387,18 +385,15 @@ public class StaticTypesBinaryExpressionMultiTypeDispatcher extends BinaryExpres
rhsValueLoader.putNodeMetaData(StaticTypesMarker.INFERRED_TYPE,
controller.getTypeChooser().resolveType(parent, controller.getClassNode()));
}
- MethodCallExpression mce = new MethodCallExpression(
- receiver,
- "putAt",
- ae
- );
+ MethodCallExpression mce = new MethodCallExpression(receiver, "putAt", ae);
mce.setSourcePosition(parent);
- visitor.visitMethodCallExpression(mce);
+
OperandStack operandStack = controller.getOperandStack();
int height = operandStack.getStackLength();
mce.visit(controller.getAcg());
operandStack.pop();
operandStack.remove(operandStack.getStackLength()-height);
+
// return value of assignment
rhsValueLoader.visit(controller.getAcg());
}
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 b5f8d4f..e3d802e 100644
--- a/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
+++ b/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
@@ -178,8 +178,6 @@ import static org.codehaus.groovy.ast.tools.GeneralUtils.castX;
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.LowestUpperBoundClassNode;
import static org.codehaus.groovy.ast.tools.WideningCategories.isBigDecCategory;
@@ -642,17 +640,10 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
}
}
}
- }
-
- ClassNode actualType =
- findActualTypeByGenericsPlaceholderName(
- fieldNode.getOriginType().getUnresolvedName(),
- makeDeclaringAndActualGenericsTypeMapOfExactType(fieldNode.getDeclaringClass(), typeCheckingContext.getEnclosingClassNode())
- );
+ } else {
+ checkOrMarkPrivateAccess(vexp, fieldNode, isLHSOfEnclosingAssignment(vexp));
- if (null != actualType) {
- storeType(vexp, actualType);
- return;
+ storeType(vexp, getType(vexp));
}
}
@@ -1243,12 +1234,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();
@@ -3503,21 +3489,6 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
}
/**
- * e.g. c(b(a())), a() and b() are nested method call, but c() is not
- * new C(b(a())) a() and b() are nested method call
- *
- * a().b().c(), a() and b() are sandwiched method call, but c() is not
- *
- * a().b().c a() and b() are sandwiched method call
- *
- */
- private boolean isNestedOrSandwichedMethodCall() {
- return typeCheckingContext.getEnclosingMethodCalls().size() > 1
- || typeCheckingContext.getEnclosingConstructorCalls().size() > 0
- || typeCheckingContext.getEnclosingPropertyExpressions().size() > 0;
- }
-
- /**
* A special method handling the "withTrait" call for which the type checker knows more than
* what the type signature is able to tell. If "withTrait" is detected, then a new class node
* is created representing the list of trait interfaces.
@@ -4689,10 +4660,15 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
if (vexp.isSuperExpression()) return makeSuper();
final Variable variable = vexp.getAccessedVariable();
if (variable instanceof FieldNode) {
- FieldNode fieldNode = (FieldNode) variable;
+ 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);
- checkOrMarkPrivateAccess(vexp, fieldNode, isLHSOfEnclosingAssignment(vexp));
- return getType(fieldNode);
+ fieldType = resolveGenericsWithContext(placeholders, fieldType);
+ }
+ return fieldType;
}
if (variable != vexp && variable instanceof VariableExpression) {
return getType((Expression) variable);
@@ -4735,6 +4711,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 = ClassHelper.RANGE_TYPE.getPlainNodeReference();
RangeExpression re = (RangeExpression) exp;
@@ -4763,14 +4742,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) {
@@ -4810,43 +4781,12 @@ 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(AnnotatedNode an, ClassNode type) {
- if (!type.isUsingGenerics()) return type;
- Map<GenericsTypeName, GenericsType> connections = new HashMap<GenericsTypeName, GenericsType>();
- //TODO: inner classes mean a different this-type. This is ignored here!
- extractGenericsConnections(connections, typeCheckingContext.getEnclosingClassNode(), an.getDeclaringClass());
- type = applyGenericsContext(connections, type);
- return type;
- }
-
private ClassNode makeSuper() {
- ClassNode ret = typeCheckingContext.getEnclosingClassNode().getSuperClass();
- if (typeCheckingContext.isInStaticContext) {
- ClassNode staticRet = CLASS_Type.getPlainNodeReference();
- GenericsType gt = new GenericsType(ret);
- staticRet.setGenericsTypes(new GenericsType[]{gt});
- ret = staticRet;
- }
- return ret;
+ return makeType(typeCheckingContext.getEnclosingClassNode().getSuperClass(), typeCheckingContext.isInStaticContext);
}
private ClassNode makeThis() {
- ClassNode ret = typeCheckingContext.getEnclosingClassNode();
- if (typeCheckingContext.isInStaticContext) {
- ClassNode staticRet = CLASS_Type.getPlainNodeReference();
- GenericsType gt = new GenericsType(ret);
- staticRet.setGenericsTypes(new GenericsType[]{gt});
- ret = staticRet;
- }
- return ret;
+ return makeType(typeCheckingContext.getEnclosingClassNode(), typeCheckingContext.isInStaticContext);
}
private static ClassNode makeSelf(ClassNode trait) {
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 4eca251..6e2adc7 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
@@ -16,14 +16,13 @@
* specific language governing permissions and limitations
* under the License.
*/
-
-
package org.codehaus.groovy.classgen.asm.sc.bugs
import groovy.transform.stc.StaticTypeCheckingTestCase
import org.codehaus.groovy.classgen.asm.sc.StaticCompilationTestSupport
class Groovy7361Bug extends StaticTypeCheckingTestCase implements StaticCompilationTestSupport {
+
void testShouldNotThrowVerifyError() {
assertScript '''
class TaskManager {
@@ -59,4 +58,33 @@ class Groovy7361Bug extends StaticTypeCheckingTestCase implements StaticCompilat
assert a.foo() == [a:'b']
'''
}
+
+ // 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()
+ '''
+ }
}