Mainly, this fixes an overlooked case of type parameter exposure, namely a field whose type is an *array* of parameterized types, for example "Foo<T>[]". Additionally, there are some documentation improvements and minor cleanups. Review by: mmendez git-svn-id: https://google-web-toolkit.googlecode.com/svn/trunk@2912 8db76d5a-ed1c-0410-87a9-c151d255dfc7
diff --git a/user/src/com/google/gwt/user/rebind/rpc/SerializableTypeOracleBuilder.java b/user/src/com/google/gwt/user/rebind/rpc/SerializableTypeOracleBuilder.java index e096154..a1bf11b 100644 --- a/user/src/com/google/gwt/user/rebind/rpc/SerializableTypeOracleBuilder.java +++ b/user/src/com/google/gwt/user/rebind/rpc/SerializableTypeOracleBuilder.java
@@ -62,39 +62,33 @@ * </p> * * <p> - * It then traverses the type hierarchy of each of the types in these method - * signatures in order to discover additional types which it might need to - * include. For the purposes of this explanation we define a root type to be any - * type which appears in the RemoteService method signatures or the type of any - * non-final, instance field which is part of a type that qualifies for - * serialization. The builder will fail if a root is not serializable and it has - * no subtypes that are. + * To find the serializable types, it includes the root types, and then it + * iteratively traverses the type hierarchy and the fields of any type already + * deemed serializable. To improve the accuracy of the traversal there is a + * computations of the exposure of type parameters. When the traversal reaches a + * parameterized type, these exposure values are used to determine how to treat + * the arguments. * </p> * * <p> - * To improve the accuracy of the traversal there is a computations of the - * exposure of type parameters. When the traversal reaches a parameterized type, - * these exposure values are used to determine how to treat the arguments. - * </p> - * - * <p> - * A type qualifies for serialization if it is automatically or manually - * serializable. Automatic serialization is selected if the type is assignable - * to {@link IsSerializable} or {@link Serializable} or if the type is a - * primitive type such as int, boolean, etc. Manual serialization is selected if - * there exists another type with the same fully qualified name concatenated - * with "_CustomFieldSerializer". If a type qualifies for both manual and - * automatic serialization, manual serialization is preferred. + * A type qualifies for serialization if it or one of its subtypes is + * automatically or manually serializable. Automatic serialization is selected + * if the type is assignable to {@link IsSerializable} or {@link Serializable} + * or if the type is a primitive type such as int, boolean, etc. Manual + * serialization is selected if there exists another type with the same fully + * qualified name concatenated with "_CustomFieldSerializer". If a type + * qualifies for both manual and automatic serialization, manual serialization + * is preferred. * </p> */ public class SerializableTypeOracleBuilder { - interface Path { + private interface Path { Path getParent(); String toString(); } - enum TypeState { + private enum TypeState { /** * The instantiability of a type has been determined. */ @@ -249,22 +243,6 @@ } /** - * Type parameter is exposed. - */ - static final int EXPOSURE_DIRECT = 0; - - /** - * Type parameter is exposed as a bounded array. The value is the max bound of - * the exposure. - */ - static final int EXPOSURE_MIN_BOUNDED_ARRAY = EXPOSURE_DIRECT + 1; - - /** - * Type parameter is not exposed. - */ - static final int EXPOSURE_NONE = -1; - - /** * Compares {@link JType}s according to their qualified source names. */ static final Comparator<JType> JTYPE_COMPARATOR = new Comparator<JType>() { @@ -399,6 +377,11 @@ } } + /** + * Return <code>true</code> if a class's fields should be considered for + * serialization. If it returns <code>false</code> then none of the fields + * of this class should be serialized. + */ static boolean shouldConsiderFieldsForSerialization(TreeLogger logger, JClassType type, boolean isSpeculative, TypeFilter filter) { if (!isAllowedByFilter(logger, filter, type, isSpeculative)) { @@ -681,6 +664,12 @@ private final TypeParameterExposureComputer typeParameterExposureComputer = new TypeParameterExposureComputer( typeFilter); + /** + * The set of type parameters that appear in one of the root types. + * TODO(spoon): It would be cleaner to delete this field, and instead to have + * {@link #addRootType(TreeLogger, JType)} replace parameters with wildcard + * types. Then the root types would not contain any parameters. + */ private Set<JTypeParameter> typeParametersInRootTypes = new HashSet<JTypeParameter>(); /** @@ -1347,7 +1336,7 @@ Path path = createTypeArgumentPath(parent, baseType, paramIndex, typeArg); int exposure = getTypeParameterExposure(baseType, paramIndex); switch (exposure) { - case EXPOSURE_DIRECT: { + case TypeParameterExposureComputer.EXPOSURE_DIRECT: { TreeLogger branch = logger.branch( TreeLogger.DEBUG, "Checking type argument " @@ -1358,7 +1347,7 @@ return checkTypeInstantiable(branch, typeArg, true, path) || mightNotBeExposed(baseType, paramIndex); } - case EXPOSURE_NONE: + case TypeParameterExposureComputer.EXPOSURE_NONE: // Ignore this argument logger.branch(TreeLogger.DEBUG, "Ignoring type argument " + paramIndex + " of type '" + baseType.getParameterizedQualifiedSourceName() @@ -1366,7 +1355,7 @@ return true; default: { - assert (exposure >= EXPOSURE_MIN_BOUNDED_ARRAY); + assert (exposure >= TypeParameterExposureComputer.EXPOSURE_MIN_BOUNDED_ARRAY); TreeLogger branch = logger.branch( TreeLogger.DEBUG, "Checking type argument "
diff --git a/user/src/com/google/gwt/user/rebind/rpc/TypeParameterExposureComputer.java b/user/src/com/google/gwt/user/rebind/rpc/TypeParameterExposureComputer.java index e107c92..b3ddb78 100644 --- a/user/src/com/google/gwt/user/rebind/rpc/TypeParameterExposureComputer.java +++ b/user/src/com/google/gwt/user/rebind/rpc/TypeParameterExposureComputer.java
@@ -41,6 +41,9 @@ * Helper class for type parameter flow information. */ class TypeParameterFlowInfo { + /** + * The class that declares this type parameter. + */ private final JGenericType baseType; /** @@ -52,7 +55,7 @@ */ private final Map<TypeParameterFlowInfo, Integer> causesExposure = new LinkedHashMap<TypeParameterFlowInfo, Integer>(); - private int exposure = SerializableTypeOracleBuilder.EXPOSURE_NONE; + private int exposure = EXPOSURE_NONE; private final Map<TypeParameterFlowInfo, Boolean> isTransitivelyAffectedByCache = new HashMap<TypeParameterFlowInfo, Boolean>(); @@ -75,14 +78,6 @@ this.ordinal = ordinal; } - /** - * TODO: We only need to go up the hierarchy until we find the first non RPC - * qualifying type. - * - * TODO: We need to ignore fields that would be ignored by serialization. - * - * @return - */ public boolean checkDirectExposure() { boolean didChange = false; JClassType type = baseType; @@ -134,6 +129,13 @@ return listeners; } + /** + * Return whether it is possible for the parameter not to be exposed. For + * example, if a class has one subclass that uses the parameter and another + * that does not, then the parameter is exposed (exposure >= + * <code>EXPOSURE_DIRECT</code>) but this method will return + * <code>false</code>. + */ public boolean getMightNotBeExposed() { return mightNotBeExposed; } @@ -208,6 +210,8 @@ } private void computeIndirectExposureCauses() { + // TODO(spoon): this only needs to consider immediate subtypes, not all + // subtypes JClassType[] subtypes = baseType.getSubtypes(); for (JClassType subtype : subtypes) { JGenericType isGeneric = subtype.isGenericType(); @@ -242,7 +246,7 @@ continue; } - JParameterizedType isParameterized = field.getType().isParameterized(); + JParameterizedType isParameterized = field.getType().getLeafType().isParameterized(); if (isParameterized == null) { continue; } @@ -334,6 +338,22 @@ } } + /** + * Type parameter is exposed. + */ + static final int EXPOSURE_DIRECT = 0; + + /** + * Type parameter is exposed as a bounded array. The value is the max bound of + * the exposure. + */ + static final int EXPOSURE_MIN_BOUNDED_ARRAY = EXPOSURE_DIRECT + 1; + + /** + * Type parameter is not exposed. + */ + static final int EXPOSURE_NONE = -1; + private TypeFilter typeFilter; private final Map<JTypeParameter, TypeParameterFlowInfo> typeParameterToFlowInfo = new IdentityHashMap<JTypeParameter, TypeParameterFlowInfo>(); @@ -389,8 +409,7 @@ /** * Return the parameter flow info for a type parameter specified by class and * index. If the flow info did not previously exist, create it and add it to - * the work list. TODO(spoon): if the flow info has already been computed, - * then make a perfect/no-search flow info for this type parameter. + * the work list. */ private TypeParameterFlowInfo getFlowInfo(JGenericType type, int index) { JTypeParameter typeParameter = type.getTypeParameters()[index];
diff --git a/user/test/com/google/gwt/user/rebind/rpc/SerializableTypeOracleBuilderTest.java b/user/test/com/google/gwt/user/rebind/rpc/SerializableTypeOracleBuilderTest.java index d3e96ce..9b6580d 100644 --- a/user/test/com/google/gwt/user/rebind/rpc/SerializableTypeOracleBuilderTest.java +++ b/user/test/com/google/gwt/user/rebind/rpc/SerializableTypeOracleBuilderTest.java
@@ -68,6 +68,7 @@ this.maybeInstantiated = maybeInstantiated; } + @Override public boolean equals(Object obj) { if (this == obj) { return true; @@ -89,6 +90,10 @@ } } + private static final int EXPOSURE_DIRECT = TypeParameterExposureComputer.EXPOSURE_DIRECT; + + private static final int EXPOSURE_NONE = TypeParameterExposureComputer.EXPOSURE_NONE; + private static void addICRSE(Set<CompilationUnit> units) { StringBuffer code = new StringBuffer(); code.append("package com.google.gwt.user.client.rpc;\n"); @@ -335,7 +340,7 @@ JClassType cantSerialize = to.getType("CantSerialize"); JParameterizedType listOfCantSerialize = to.getParameterizedType(list, - new JClassType[] {cantSerialize}); + makeArray(cantSerialize)); SerializableTypeOracleBuilder sob = new SerializableTypeOracleBuilder( logger, to); @@ -400,7 +405,118 @@ } /* - * Tests that arrays of type variables that do not cause infinite expansion. + * Tests arrays of parameterized types. + */ + public void testArrayOfParameterizedTypes() throws UnableToCompleteException, + NotFoundException { + Set<CompilationUnit> units = new HashSet<CompilationUnit>(); + addStandardClasses(units); + + { + // A<T> exposes its param + StringBuilder code = new StringBuilder(); + code.append("import java.io.Serializable;\n"); + code.append("public class A<T> implements Serializable {\n"); + code.append(" T t;\n"); + code.append("}\n"); + units.add(createMockCompilationUnit("A", code)); + } + + { + StringBuilder code = new StringBuilder(); + code.append("import java.io.Serializable;\n"); + code.append("public class AList<T> implements Serializable {\n"); + code.append(" A<T>[] as;\n"); + code.append("}\n"); + units.add(createMockCompilationUnit("AList", code)); + } + + { + // B<T> does not expose its param + StringBuilder code = new StringBuilder(); + code.append("import java.io.Serializable;\n"); + code.append("public class B<T> implements Serializable {\n"); + code.append("}\n"); + units.add(createMockCompilationUnit("B", code)); + } + + { + StringBuilder code = new StringBuilder(); + code.append("import java.io.Serializable;\n"); + code.append("public class BList<T> implements Serializable {\n"); + code.append(" B<T>[] bs;\n"); + code.append("}\n"); + units.add(createMockCompilationUnit("BList", code)); + } + + { + // A random serializable class + StringBuilder code = new StringBuilder(); + code.append("import java.io.Serializable;\n"); + code.append("public class Ser1 implements Serializable {\n"); + code.append("}\n"); + units.add(createMockCompilationUnit("Ser1", code)); + } + + { + // A random serializable class + StringBuilder code = new StringBuilder(); + code.append("import java.io.Serializable;\n"); + code.append("public class Ser2 implements Serializable {\n"); + code.append("}\n"); + units.add(createMockCompilationUnit("Ser2", code)); + } + + { + StringBuilder code = new StringBuilder(); + code.append("import java.io.Serializable;\n"); + code.append("public class Root implements Serializable {\n"); + code.append(" AList<Ser1> alist;\n"); + code.append(" BList<Ser2> blist;\n"); + code.append("}\n"); + units.add(createMockCompilationUnit("Root", code)); + } + + TreeLogger logger = createLogger(); + TypeOracle to = TypeOracleTestingUtils.buildTypeOracle(logger, units); + + JGenericType a = to.getType("A").isGenericType(); + JGenericType b = to.getType("B").isGenericType(); + JGenericType alist = to.getType("AList").isGenericType(); + JGenericType blist = to.getType("BList").isGenericType(); + JClassType ser1 = to.getType("Ser1"); + JClassType ser2 = to.getType("Ser2"); + JClassType root = to.getType("Root"); + + SerializableTypeOracleBuilder sob = new SerializableTypeOracleBuilder( + logger, to); + sob.addRootType(logger, root); + + assertEquals(EXPOSURE_DIRECT, sob.getTypeParameterExposure(a, 0)); + assertEquals(EXPOSURE_NONE, sob.getTypeParameterExposure(b, 0)); + assertEquals(EXPOSURE_DIRECT, sob.getTypeParameterExposure(alist, 0)); + assertEquals(EXPOSURE_NONE, sob.getTypeParameterExposure(blist, 0)); + + SerializableTypeOracle so = sob.build(logger); + + JArrayType aArray = to.getArrayType(a.getRawType()); + JArrayType bArray = to.getArrayType(b.getRawType()); + + assertSerializableTypes(so, root, alist.getRawType(), blist.getRawType(), + aArray, bArray, a.getRawType(), b.getRawType(), ser1); + + assertInstantiable(so, alist.getRawType()); + assertInstantiable(so, blist.getRawType()); + assertInstantiable(so, a.getRawType()); + assertInstantiable(so, b.getRawType()); + assertInstantiable(so, aArray); + assertInstantiable(so, bArray); + assertInstantiable(so, ser1); + assertNotInstantiableOrFieldSerializable(so, ser2); + } + + /* + * Tests arrays of type variables that do not cause infinite expansion. */ public void testArrayOfTypeParameter() throws UnableToCompleteException, NotFoundException { @@ -452,7 +568,7 @@ JClassType javaLangString = to.getType(String.class.getName()); JParameterizedType cOfString = to.getParameterizedType(c, - new JClassType[] {javaLangString}); + makeArray(javaLangString)); SerializableTypeOracleBuilder sob = new SerializableTypeOracleBuilder( logger, to); sob.addRootType(logger, cOfString); @@ -757,7 +873,7 @@ JClassType serializableArgument = to.getType("SerializableArgument"); JParameterizedType aOfString = to.getParameterizedType(a, - new JClassType[] {serializableArgument}); + makeArray(serializableArgument)); SerializableTypeOracleBuilder sob = new SerializableTypeOracleBuilder( logger, to); sob.addRootType(logger, aOfString); @@ -812,7 +928,7 @@ JClassType unusedSerializableArgument = to.getType("UnusedSerializableArgument"); JParameterizedType aOfString = to.getParameterizedType(a, - new JClassType[] {unusedSerializableArgument}); + makeArray(unusedSerializableArgument)); SerializableTypeOracleBuilder sob = new SerializableTypeOracleBuilder( logger, to); sob.addRootType(logger, aOfString); @@ -860,7 +976,7 @@ JClassType javaLangString = to.getType(String.class.getName()); JParameterizedType aOfString = to.getParameterizedType(a, - new JClassType[] {javaLangString}); + makeArray(javaLangString)); SerializableTypeOracleBuilder sob = new SerializableTypeOracleBuilder( logger, to); sob.addRootType(logger, aOfString); @@ -908,12 +1024,12 @@ JClassType javaLangString = to.getType(String.class.getName()); JParameterizedType aOfString = to.getParameterizedType(a, - new JClassType[] {javaLangString}); + makeArray(javaLangString)); SerializableTypeOracleBuilder sob = new SerializableTypeOracleBuilder( logger, to); - assertEquals(0, sob.getTypeParameterExposure(a, 0)); - assertEquals(0, sob.getTypeParameterExposure(b, 0)); + assertEquals(EXPOSURE_DIRECT, sob.getTypeParameterExposure(a, 0)); + assertEquals(EXPOSURE_DIRECT, sob.getTypeParameterExposure(b, 0)); sob.addRootType(logger, aOfString); SerializableTypeOracle so = sob.build(logger); @@ -1169,12 +1285,12 @@ JClassType javaLangString = to.getType(String.class.getName()); JParameterizedType aOfString = to.getParameterizedType(a, - new JClassType[] {javaLangString}); + makeArray(javaLangString)); SerializableTypeOracleBuilder stob = new SerializableTypeOracleBuilder( logger, to); stob.addRootType(logger, aOfString); - assertTrue(stob.getTypeParameterExposure(a, 0) < 0); + assertEquals(EXPOSURE_NONE, stob.getTypeParameterExposure(a, 0)); SerializableTypeOracle so = stob.build(logger); @@ -1387,7 +1503,7 @@ JClassType b = to.getType("B"); JTypeParameter syntheticTypeParam = new JTypeParameter("U", 0); - syntheticTypeParam.setBounds(new JClassType[] {b}); + syntheticTypeParam.setBounds(makeArray(b)); JParameterizedType parameterizedType = to.getParameterizedType(a, new JClassType[] {syntheticTypeParam}); @@ -1400,4 +1516,8 @@ assertInstantiable(so, b); assertSerializableTypes(so, rawA, b); } + + private JClassType[] makeArray(JClassType... elements) { + return elements; + } }