Revision: 6872 Author: j...@google.com Date: Thu Nov 12 08:22:02 2009 Log: Fix a problem with local static classes, plus clean up naming, comments and formatting.
Patch by: jat Review by: rice http://code.google.com/p/google-web-toolkit/source/detail?r=6872 Modified: /trunk/dev/core/src/com/google/gwt/dev/javac/TypeOracleMediator.java /trunk/dev/core/src/com/google/gwt/dev/javac/asm/CollectClassData.java /trunk/dev/core/src/com/google/gwt/dev/javac/asm/CollectMethodData.java /trunk/dev/core/test/com/google/gwt/dev/javac/TypeOracleMediatorTest.java /trunk/dev/core/test/com/google/gwt/dev/javac/asm/CollectClassDataTest.java ======================================= --- /trunk/dev/core/src/com/google/gwt/dev/javac/TypeOracleMediator.java Thu Nov 12 06:59:32 2009 +++ /trunk/dev/core/src/com/google/gwt/dev/javac/TypeOracleMediator.java Thu Nov 12 08:22:02 2009 @@ -93,6 +93,9 @@ /** * Returns the binary name of a type. This is the same name that would be * returned by {...@link Class#getName()} for this type. + * + * @param type TypeOracle type to get the name for + * @return binary name for a type */ public static String computeBinaryClassName(JType type) { JPrimitiveType primitiveType = type.isPrimitive(); @@ -231,10 +234,18 @@ private Resolver resolver; + /** + * Construct a TypeOracleMediator. + */ public TypeOracleMediator() { this(null); } + /** + * Construct a TypeOracleMediator. + * + * @param typeOracle TypeOracle instance to use, or null to create a new one + */ // @VisibleForTesting public TypeOracleMediator(TypeOracle typeOracle) { if (typeOracle == null) { @@ -272,8 +283,12 @@ /** * Adds new units to an existing TypeOracle. + * + * @param logger logger to use + * @param units collection of compilation units to process */ - public void addNewUnits(TreeLogger logger, Collection<CompilationUnit> units) { + public void addNewUnits(TreeLogger logger, + Collection<CompilationUnit> units) { PerfLogger.start("TypeOracleMediator.addNewUnits"); // First collect all class data. classMap = new HashMap<String, CollectClassData>(); @@ -349,10 +364,16 @@ PerfLogger.end(); } + /** + * @return a map from binary class names to JRealClassType. + */ public Map<String, JRealClassType> getBinaryMapper() { return binaryMapper; } + /** + * @return the TypeOracle managed by the mediator. + */ public TypeOracle getTypeOracle() { return typeOracle; } @@ -390,7 +411,7 @@ String jpkgName = compiledClass.getPackageName(); JPackage pkg = typeOracle.getOrCreatePackage(jpkgName); boolean isIntf = (access & Opcodes.ACC_INTERFACE) != 0; - boolean isLocalType = classData.isLocal(); + boolean isLocalType = classData.hasNoExternalName(); String enclosingTypeName = null; if (enclosingClassData != null) { enclosingTypeName = InternalName.toSourceName(InternalName.getClassName(enclosingClassData.getName())); @@ -526,7 +547,7 @@ private CollectClassData processClass(CompiledClass compiledClass) { byte[] classBytes = compiledClass.getBytes(); ClassReader reader = new ClassReader(classBytes); - CollectClassData mcv = new CollectClassData(classBytes); + CollectClassData mcv = new CollectClassData(); ClassVisitor cv = mcv; if (false) { cv = new TraceClassVisitor(cv, new PrintWriter(System.out)); @@ -688,7 +709,7 @@ assert classData != null; int access = classData.getAccess(); - assert (!classData.getClassType().isLocal()); + assert (!classData.getClassType().hasNoExternalName()); logger = logger.branch(TreeLogger.SPAM, "Found type '" + type.getQualifiedSourceName() + "'", null); ======================================= --- /trunk/dev/core/src/com/google/gwt/dev/javac/asm/CollectClassData.java Thu Nov 12 06:32:35 2009 +++ /trunk/dev/core/src/com/google/gwt/dev/javac/asm/CollectClassData.java Thu Nov 12 08:22:02 2009 @@ -60,47 +60,44 @@ */ Anonymous { @Override - public boolean hasOuter() { + public boolean hasNoExternalName() { return true; } - - @Override - public boolean isLocal() { - return true; - } }, /** * A named class defined inside a method. */ Local { - @Override - public boolean hasHiddenConstructorArg() { - return true; - } + /* + * Note that we do not return true for hasHiddenConstructorArg since Local + * classes inside a static method will not have one and AFAICT there is no + * way to distinguish these cases without looking up the declaring method. + * However, since we are dropping any classes for which + * hasNoExternalName() returns true in TypeOracleMediator.addNewUnits, it + * doesn't matter if we leave the synthetic argument in the list. + */ @Override - public boolean hasOuter() { + public boolean hasNoExternalName() { return true; } - - @Override - public boolean isLocal() { - return true; - } }; + /** + * @return true if this class type has a hidden constructor argument + * for the containing instance (ie, this$0). + */ public boolean hasHiddenConstructorArg() { return false; } - public boolean hasOuter() { + /** + * @return true if this class type is not visible outside a method. + */ + public boolean hasNoExternalName() { return false; } - - public boolean isLocal() { - return false; - } } /** @@ -110,20 +107,26 @@ private final String desc; private final String value; + /** + * Construct the value of an Enum-valued annotation. + * + * @param desc type descriptor of this enum + * @param value actual value in this enum + */ public AnnotationEnum(String desc, String value) { this.desc = desc; this.value = value; } /** - * @return the descriptor + * @return the type descriptor of the enum type. */ public String getDesc() { return desc; } /** - * @return the value + * @return the annotation value. */ public String getValue() { return value; @@ -144,7 +147,6 @@ // internal names of interfaces private String[] interfaces; - private byte[] bytes; private List<CollectMethodData> methods = new ArrayList<CollectMethodData>(); private List<CollectFieldData> fields = new ArrayList<CollectFieldData>(); private int access; @@ -153,30 +155,25 @@ private String outerMethodDesc; private CollectClassData.ClassType classType = ClassType.TopLevel; - public CollectClassData(byte[] bytes) { - this.bytes = bytes; + /** + * Construct a visitor that will collect data about a class. + */ + public CollectClassData() { } /** - * @return the access + * @return the access flags for this class (ie, bitwise or of Opcodes.ACC_*). */ public int getAccess() { return access; } /** - * @return annotations on this class + * @return a list of annotations on this class. */ public List<CollectAnnotationData> getAnnotations() { return annotations; } - - /** - * @return the bytes - */ - public byte[] getBytes() { - return bytes; - } /** * @return the class type. @@ -186,21 +183,14 @@ } /** - * @return the fields + * @return a list of fields in this class. */ public List<CollectFieldData> getFields() { return fields; } - -// /** -// * @return the innerClasses -// */ -// public List<Resource> getInnerClasses() { -// return innerClasses; -// } /** - * @return the interfaces + * @return an array of internal names of interfaces implemented by this class. */ public String[] getInterfaces() { return interfaces; @@ -262,20 +252,20 @@ return superName; } + /** + * @return true if this class has no external name (ie, is defined inside + * a method). + */ public boolean hasNoExternalName() { - return classType == ClassType.Anonymous || classType == ClassType.Local; + return classType.hasNoExternalName(); } + /** + * @return true if this class has no source name at all. + */ public boolean isAnonymous() { return classType == ClassType.Anonymous; } - - public boolean isLocal() { - if (name.matches("\\$\\d") && !isAnonymous()) { - throw new IllegalStateException("Not anonymous with name of " + name); - } - return classType.isLocal(); - } @Override public String toString() { ======================================= --- /trunk/dev/core/src/com/google/gwt/dev/javac/asm/CollectMethodData.java Thu Nov 12 06:32:35 2009 +++ /trunk/dev/core/src/com/google/gwt/dev/javac/asm/CollectMethodData.java Thu Nov 12 08:22:02 2009 @@ -43,7 +43,17 @@ private int access; private int syntheticArgs; - @SuppressWarnings("unchecked") + /** + * Prepare to collect data for a method from bytecode. + * + * @param classType + * @param access + * @param name + * @param desc + * @param signature + * @param exceptions + */ + @SuppressWarnings("unchecked") // for new List[] public CollectMethodData(CollectClassData.ClassType classType, int access, String name, String desc, String signature, String[] exceptions) { this.access = access; @@ -55,9 +65,16 @@ argTypes = Type.getArgumentTypes(desc); // Non-static instance methods and constructors of non-static inner // classes have an extra synthetic parameter that isn't in the source, - // so we remove it. + // so we remove it. Note that for local classes, they may or may not + // have this synthetic parameter depending on whether the containing + // method is static, but we can't get that info here. However, since + // local classes are dropped from TypeOracle, we don't care. if (classType.hasHiddenConstructorArg() && "<init>".equals(name)) { // remove "this$1" as a parameter + if (argTypes.length < 1) { + throw new IllegalStateException( + "Missing synthetic argument in constructor"); + } syntheticArgs = 1; int n = argTypes.length - syntheticArgs; Type[] newArgTypes = new Type[n]; ======================================= --- /trunk/dev/core/test/com/google/gwt/dev/javac/TypeOracleMediatorTest.java Tue Nov 10 20:42:30 2009 +++ /trunk/dev/core/test/com/google/gwt/dev/javac/TypeOracleMediatorTest.java Thu Nov 12 08:22:02 2009 @@ -46,6 +46,9 @@ import java.util.Map; import java.util.Set; +/** + * Test TypeOracleMediator. + */ public class TypeOracleMediatorTest extends TestCase { private abstract class MutableJavaResource extends MockJavaResource { @@ -128,6 +131,38 @@ } }; + + protected CheckedJavaResource CU_AnonymousClass = new CheckedJavaResource( + "test", "Enclosing") { + + @Override + public void check(JClassType type) { + final String name = type.getSimpleSourceName(); + assertEquals("Enclosing", name); + checkEnclosing(type); + } + + public void checkEnclosing(JClassType type) { + assertEquals("Enclosing", type.getSimpleSourceName()); + assertEquals("test.Enclosing", type.getQualifiedSourceName()); + // verify the anonymous class doesn't show up + JClassType[] nested = type.getNestedTypes(); + assertEquals(0, nested.length); + } + + @Override + public String getSource() { + StringBuffer sb = new StringBuffer(); + sb.append("package test;\n"); + sb.append("public class Enclosing {\n"); + sb.append(" public static Object getLocal() {"); + sb.append(" return new Object() { };\n"); + sb.append(" }\n"); + sb.append("}\n"); + return sb.toString(); + } + }; + protected CheckedJavaResource CU_Assignable = new CheckedJavaResource( "test.sub", "Derived", "BaseInterface", "DerivedInterface", "Derived.Nested") { @@ -631,7 +666,7 @@ }; protected CheckedJavaResource CU_LocalClass = new CheckedJavaResource("test", - "Enclosing", "Enclosing.1") { + "Enclosing") { @Override public void check(JClassType type) { @@ -643,7 +678,7 @@ public void checkEnclosing(JClassType type) { assertEquals("Enclosing", type.getSimpleSourceName()); assertEquals("test.Enclosing", type.getQualifiedSourceName()); - // verify the anonymous class doesn't show up + // verify the local class doesn't show up JClassType[] nested = type.getNestedTypes(); assertEquals(0, nested.length); } @@ -654,13 +689,49 @@ sb.append("package test;\n"); sb.append("public class Enclosing {\n"); sb.append(" public static Object getLocal() {"); - sb.append(" return new Object() { };\n"); + sb.append(" class MyObject { }\n"); + sb.append(" return new MyObject();\n"); sb.append(" }\n"); sb.append("}\n"); return sb.toString(); } }; + protected CheckedJavaResource CU_LocalClass2 = new CheckedJavaResource("test", + "Enclosing") { + + @Override + public void check(JClassType type) { + final String name = type.getSimpleSourceName(); + assertEquals("Enclosing", name); + checkEnclosing(type); + } + + public void checkEnclosing(JClassType type) { + assertEquals("Enclosing", type.getSimpleSourceName()); + assertEquals("test.Enclosing", type.getQualifiedSourceName()); + // verify the local class doesn't show up + JClassType[] nested = type.getNestedTypes(); + assertEquals(0, nested.length); + } + + @Override + public String getSource() { + StringBuffer sb = new StringBuffer(); + sb.append("package test;\n"); + sb.append("public class Enclosing {\n"); + sb.append(" public int foo;\n"); + sb.append(" public Object getLocal() {\n"); + sb.append(" class MyObject {\n"); + sb.append(" int getFoo() { return foo; }\n"); + sb.append(" }\n"); + sb.append(" return new MyObject() {};\n"); + sb.append(" }\n"); + sb.append("}\n"); + return sb.toString(); + } + }; + protected CheckedJavaResource CU_MethodsAndParams = new CheckedJavaResource( "test", "Methods") { @@ -1189,6 +1260,14 @@ JClassType[] types = typeOracle.getTypes(); assertEquals(2, types.length); } + + public void testLocalWithSynthetic() throws TypeOracleException { + resources.add(CU_Object); + resources.add(CU_LocalClass2); + compileAndRefresh(); + JClassType[] types = typeOracle.getTypes(); + assertEquals(2, types.length); + } public void testMethodsAndParams() throws TypeOracleException { resources.add(CU_Object); ======================================= --- /trunk/dev/core/test/com/google/gwt/dev/javac/asm/CollectClassDataTest.java Thu Nov 12 06:32:35 2009 +++ /trunk/dev/core/test/com/google/gwt/dev/javac/asm/CollectClassDataTest.java Thu Nov 12 08:22:02 2009 @@ -72,6 +72,37 @@ annotatedField = field; } } + + /** + * Test local classes. + */ + public static class Three { + + public int foo; + + /** + * Static method that has a local class in it. + */ + public static void methodWithLocalStatic() { + class Foo { + } + + Foo x = new Foo(); + } + + /** + * Method that has a local class in it. + */ + public void methodWithLocal() { + class Foo { + Foo() { + foo = 1; + } + } + + Foo x = new Foo(); + } + } public void testAnonymous() { CollectClassData cd = collect(One.class.getName() + "$1"); @@ -215,6 +246,22 @@ assertEquals(Opcodes.ACC_PUBLIC , cd.getAccess() & ~Opcodes.ACC_SUPER); assertEquals(ClassType.Inner, cd.getClassType()); } + + public void testLocal() { + CollectClassData cd = collect(Three.class.getName() + "$2Foo"); + // Don't check for super bit, as it will depend on the JDK used to compile. + assertEquals(0, cd.getAccess() & ~Opcodes.ACC_SUPER); + assertEquals(ClassType.Local, cd.getClassType()); + assertEquals("methodWithLocal", cd.getOuterMethodName()); + } + + public void testLocalStatic() { + CollectClassData cd = collect(Three.class.getName() + "$1Foo"); + // Don't check for super bit, as it will depend on the JDK used to compile. + assertEquals(0, cd.getAccess() & ~Opcodes.ACC_SUPER); + assertEquals(ClassType.Local, cd.getClassType()); + assertEquals("methodWithLocalStatic", cd.getOuterMethodName()); + } private CollectClassData collect(Class<?> clazz) { return collect(clazz.getName()); @@ -223,7 +270,7 @@ private CollectClassData collect(String className) { byte[] bytes = getClassBytes(className); assertNotNull("Couldn't load bytes for " + className, bytes); - CollectClassData cv = new CollectClassData(bytes); + CollectClassData cv = new CollectClassData(); ClassReader reader = new ClassReader(bytes); reader.accept(cv, 0); return cv; -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
