Write correct InnerClasses attribute to class files

InnerClasses should contain entries for every inner and every outer class,
according to the JVMS

 #KT-3390 Fixed

compiler/backend/src/org/jetbrains/jet/codegen/CodegenUtil.java

@@ -16,7 +16,6 @@
 
 package org.jetbrains.jet.codegen;
 
-import com.google.common.collect.ImmutableList;
 import com.intellij.openapi.util.Condition;
 import com.intellij.psi.PsiElement;
 import com.intellij.util.containers.ContainerUtil;
@@ -41,7 +40,6 @@ import org.jetbrains.jet.lang.resolve.java.JvmAbi;
 import org.jetbrains.jet.lang.resolve.java.JvmClassName;
 import org.jetbrains.jet.lang.resolve.java.JvmStdlibNames;
 import org.jetbrains.jet.lang.resolve.name.Name;
-import org.jetbrains.jet.lang.resolve.scopes.JetScope;
 import org.jetbrains.jet.lang.types.JetType;
 import org.jetbrains.jet.lang.types.TypeUtils;
 import org.jetbrains.jet.lang.types.lang.KotlinBuiltIns;
@@ -243,20 +241,6 @@ public class CodegenUtil {
         return type.getConstructor().getDeclarationDescriptor().getOriginal() == classDescriptor.getOriginal();
     }
 
-    @SuppressWarnings("unchecked")
-    static Collection<ClassDescriptor> getInnerClassesAndObjects(ClassDescriptor classDescriptor) {
-        JetScope innerClassesScope = classDescriptor.getUnsubstitutedInnerClassesScope();
-        Collection<DeclarationDescriptor> inners = innerClassesScope.getAllDescriptors();
-        for (DeclarationDescriptor inner : inners) {
-            assert inner instanceof ClassDescriptor
-                    : "Not a class in inner classes scope of " + classDescriptor + ": " + inner;
-        }
-        return new ImmutableList.Builder<ClassDescriptor>()
-                .addAll((Collection) inners)
-                .addAll(innerClassesScope.getObjectDescriptors())
-                .build();
-    }
-
     public static boolean isCallInsideSameClassAsDeclared(CallableMemberDescriptor declarationDescriptor, CodegenContext context) {
         boolean isFakeOverride = declarationDescriptor.getKind() == CallableMemberDescriptor.Kind.FAKE_OVERRIDE;
         boolean isDelegate = declarationDescriptor.getKind() == CallableMemberDescriptor.Kind.DELEGATION;

compiler/backend/src/org/jetbrains/jet/codegen/ImplementationBodyCodegen.java

@@ -21,7 +21,10 @@ import com.intellij.openapi.progress.ProcessCanceledException;
 import com.intellij.openapi.util.Pair;
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
-import org.jetbrains.asm4.*;
+import org.jetbrains.asm4.AnnotationVisitor;
+import org.jetbrains.asm4.Label;
+import org.jetbrains.asm4.MethodVisitor;
+import org.jetbrains.asm4.Type;
 import org.jetbrains.asm4.commons.InstructionAdapter;
 import org.jetbrains.asm4.commons.Method;
 import org.jetbrains.jet.codegen.binding.CalculatedClosure;
@@ -44,7 +47,10 @@ import org.jetbrains.jet.lang.resolve.DescriptorUtils;
 import org.jetbrains.jet.lang.resolve.OverridingUtil;
 import org.jetbrains.jet.lang.resolve.calls.model.ResolvedCall;
 import org.jetbrains.jet.lang.resolve.constants.CompileTimeConstant;
-import org.jetbrains.jet.lang.resolve.java.*;
+import org.jetbrains.jet.lang.resolve.java.AsmTypeConstants;
+import org.jetbrains.jet.lang.resolve.java.JvmAbi;
+import org.jetbrains.jet.lang.resolve.java.JvmClassName;
+import org.jetbrains.jet.lang.resolve.java.JvmStdlibNames;
 import org.jetbrains.jet.lang.resolve.java.kt.DescriptorKindUtils;
 import org.jetbrains.jet.lang.resolve.name.Name;
 import org.jetbrains.jet.lang.types.JetType;
@@ -58,6 +64,8 @@ import static org.jetbrains.jet.codegen.AsmUtil.*;
 import static org.jetbrains.jet.codegen.CodegenUtil.*;
 import static org.jetbrains.jet.codegen.binding.CodegenBinding.*;
 import static org.jetbrains.jet.lang.resolve.BindingContextUtils.callableDescriptorToDeclaration;
+import static org.jetbrains.jet.lang.resolve.BindingContextUtils.descriptorToDeclaration;
+import static org.jetbrains.jet.lang.resolve.DescriptorUtils.*;
 import static org.jetbrains.jet.lang.resolve.java.AsmTypeConstants.JAVA_STRING_TYPE;
 import static org.jetbrains.jet.lang.resolve.java.AsmTypeConstants.OBJECT_TYPE;
 
@@ -177,6 +185,8 @@ public class ImplementationBodyCodegen extends ClassBodyCodegen {
 
         writeEnclosingMethod();
 
+        writeOuterClasses();
+
         writeInnerClasses();
 
         AnnotationCodegen.forClass(v.getVisitor(), typeMapper).genAnnotations(descriptor);
@@ -225,20 +235,30 @@ public class ImplementationBodyCodegen extends ClassBodyCodegen {
     }
 
     private void writeInnerClasses() {
-        for (ClassDescriptor innerClass : getInnerClassesAndObjects(descriptor)) {
+        Collection<ClassDescriptor> result = bindingContext.get(INNER_CLASSES, descriptor);
-            writeInnerClass(innerClass);
+        if (result != null) {
-        }
+            for (ClassDescriptor innerClass : result) {
-
+                writeInnerClass(innerClass);
-        ClassDescriptor classObjectDescriptor = descriptor.getClassObjectDescriptor();
+            }
-        if (classObjectDescriptor != null) {
-            int innerClassAccess = getVisibilityAccessFlag(classObjectDescriptor) | ACC_FINAL | ACC_STATIC;
-            v.visitInnerClass(classAsmType.getInternalName() + JvmAbi.CLASS_OBJECT_SUFFIX, classAsmType.getInternalName(),
-                              JvmAbi.CLASS_OBJECT_CLASS_NAME,
-                              innerClassAccess);
         }
     }
 
-    private void writeInnerClass(ClassDescriptor innerClass) {
+    private void writeOuterClasses() {
+        // JVMS7 (4.7.6): a nested class or interface member will have InnerClasses information
+        // for each enclosing class and for each immediate member
+        DeclarationDescriptor inner = descriptor;
+        while (true) {
+            if (inner == null || isTopLevelDeclaration(inner)) {
+                break;
+            }
+            if (inner instanceof ClassDescriptor && !isEnumClassObject(inner)) {
+                writeInnerClass((ClassDescriptor) inner);
+            }
+            inner = inner.getContainingDeclaration();
+        }
+    }
+
+    private void writeInnerClass(@NotNull ClassDescriptor innerClass) {
         // TODO: proper access
         int innerClassAccess = getVisibilityAccessFlag(innerClass);
         if (innerClass.getModality() == Modality.FINAL) {
@@ -260,9 +280,27 @@ public class ImplementationBodyCodegen extends ClassBodyCodegen {
         }
 
         // TODO: cache internal names
-        String outerClassInternalName = classAsmType.getInternalName();
+        DeclarationDescriptor containing = innerClass.getContainingDeclaration();
-        String innerClassInternalName = typeMapper.mapType(innerClass.getDefaultType(), JetTypeMapperMode.IMPL).getInternalName();
+        String outerClassInternalName = containing instanceof ClassDescriptor ? getInternalNameForImpl((ClassDescriptor) containing) : null;
-        v.visitInnerClass(innerClassInternalName, outerClassInternalName, innerClass.getName().getName(), innerClassAccess);
+
+        String innerClassInternalName;
+        String innerName;
+
+        if (isClassObject(innerClass)) {
+            innerName = JvmAbi.CLASS_OBJECT_CLASS_NAME;
+            innerClassInternalName = outerClassInternalName + JvmAbi.CLASS_OBJECT_SUFFIX;
+        }
+        else {
+            innerName = innerClass.getName().isSpecial() ? null : innerClass.getName().getName();
+            innerClassInternalName = getInternalNameForImpl(innerClass);
+        }
+
+        v.visitInnerClass(innerClassInternalName, outerClassInternalName, innerName, innerClassAccess);
+    }
+
+    @NotNull
+    private String getInternalNameForImpl(@NotNull ClassDescriptor descriptor) {
+        return typeMapper.mapType(descriptor.getDefaultType(), JetTypeMapperMode.IMPL).getInternalName();
     }
 
     private void writeClassSignatureIfNeeded(JvmClassSignature signature) {

compiler/backend/src/org/jetbrains/jet/codegen/binding/CodegenBinding.java

@@ -53,6 +53,8 @@ public class CodegenBinding {
 
     public static final WritableSlice<ClassDescriptor, Boolean> ENUM_ENTRY_CLASS_NEED_SUBCLASS = Slices.createSimpleSetSlice();
 
+    public static final WritableSlice<ClassDescriptor, Collection<ClassDescriptor>> INNER_CLASSES = Slices.createSimpleSlice();
+
     private CodegenBinding() {
     }
 
@@ -205,6 +207,23 @@ public class CodegenBinding {
         if (canHaveOuter(bindingTrace.getBindingContext(), classDescriptor) && !functionLiteral) {
             closure.setCaptureThis();
         }
+
+        if (enclosing != null) {
+            recordInnerClass(bindingTrace, enclosing, classDescriptor);
+        }
+    }
+
+    private static void recordInnerClass(
+            @NotNull BindingTrace bindingTrace,
+            @NotNull ClassDescriptor outer,
+            @NotNull ClassDescriptor inner
+    ) {
+        Collection<ClassDescriptor> innerClasses = bindingTrace.get(INNER_CLASSES, outer);
+        if (innerClasses == null) {
+            innerClasses = new ArrayList<ClassDescriptor>();
+            bindingTrace.record(INNER_CLASSES, outer, innerClasses);
+        }
+        innerClasses.add(inner);
     }
 
     public static void registerClassNameForScript(BindingTrace bindingTrace, @NotNull JetScript jetScript, @NotNull JvmClassName className) {

compiler/testData/codegen/classes/classObjectIsInnerClass.kt

@@ -0,0 +1,3 @@
+class A {
+    class object
+}

compiler/testData/codegen/innerClassInfo/anonymousClass.kt

@@ -0,0 +1,7 @@
+class A {
+    val B = object { }
+    
+    fun foo() {
+        val C = object { }
+    }
+}

compiler/testData/codegen/innerClassInfo/enumEntry.kt

@@ -0,0 +1,9 @@
+enum class E {
+    E1
+    
+    E2 {
+        override fun foo() {}
+    }
+    
+    open fun foo() {}
+}

compiler/testData/codegen/innerClassInfo/innerClassInfo.kt

@@ -0,0 +1,7 @@
+class A {
+    class B {
+        inner class C
+    }
+    
+    class object
+}

compiler/testData/codegen/innerClassInfo/localClass.kt

@@ -0,0 +1,5 @@
+class A {
+    fun foo() {
+        class B
+    }
+}

compiler/tests/org/jetbrains/jet/codegen/ClassGenTest.java

@@ -17,6 +17,7 @@
 package org.jetbrains.jet.codegen;
 
 import org.jetbrains.jet.ConfigurationKind;
+import org.jetbrains.jet.lang.resolve.java.JvmAbi;
 import org.jetbrains.jet.lang.resolve.java.PackageClassUtils;
 import org.jetbrains.jet.lang.resolve.name.FqName;
 
@@ -135,4 +136,13 @@ public class ClassGenTest extends CodegenTestCase {
         //        blackBoxFile("regressions/kt1213.kt");
     }
     */
+
+    public void testClassObjectIsInnerClass() throws Exception {
+        loadFile("classes/classObjectIsInnerClass.kt");
+        GeneratedClassLoader loader = createClassLoader(generateClassesInFile());
+        Class<?> a = loader.loadClass("A");
+        Class<?> aClassObject = loader.loadClass("A" + JvmAbi.CLASS_OBJECT_SUFFIX);
+        assertSameElements(a.getDeclaredClasses(), aClassObject);
+        assertEquals(a, aClassObject.getDeclaringClass());
+    }
 }

compiler/tests/org/jetbrains/jet/codegen/InnerClassInfoGenTest.java

@@ -0,0 +1,149 @@
+/*
+ * Copyright 2010-2013 JetBrains s.r.o.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.jetbrains.jet.codegen;
+
+import org.jetbrains.annotations.NotNull;
+import org.jetbrains.annotations.Nullable;
+import org.jetbrains.asm4.ClassReader;
+import org.jetbrains.asm4.ClassVisitor;
+import org.jetbrains.jet.ConfigurationKind;
+import org.jetbrains.jet.lang.resolve.java.JvmAbi;
+
+import java.util.ArrayList;
+import java.util.List;
+
+import static org.jetbrains.asm4.Opcodes.*;
+
+public class InnerClassInfoGenTest extends CodegenTestCase {
+    private ClassFileFactory factory;
+
+    @Override
+    protected void setUp() throws Exception {
+        super.setUp();
+
+        createEnvironmentWithMockJdkAndIdeaAnnotations(ConfigurationKind.JDK_ONLY);
+        loadFile("innerClassInfo/" + getTestName(true) + ".kt");
+        factory = generateClassesInFile();
+    }
+
+    @Override
+    protected void tearDown() throws Exception {
+        factory = null;
+        super.tearDown();
+    }
+
+
+    public void testInnerClassInfo() {
+        InnerClassAttribute innerB = new InnerClassAttribute("A$B", "A", "B", ACC_PUBLIC | ACC_STATIC | ACC_FINAL);
+        InnerClassAttribute innerC = new InnerClassAttribute("A$B$C", "A$B", "C", ACC_PUBLIC | ACC_FINAL);
+        InnerClassAttribute innerAClassObject = new InnerClassAttribute(
+                "A" + JvmAbi.CLASS_OBJECT_SUFFIX, "A", JvmAbi.CLASS_OBJECT_CLASS_NAME, ACC_PUBLIC | ACC_STATIC | ACC_FINAL);
+
+        extractAndCompareInnerClasses("A", innerB, innerAClassObject);
+        extractAndCompareInnerClasses("A$B", innerB, innerC);
+        extractAndCompareInnerClasses("A$B$C", innerB, innerC);
+        extractAndCompareInnerClasses("A" + JvmAbi.CLASS_OBJECT_SUFFIX, innerAClassObject);
+    }
+
+    public void testLocalClass() {
+        InnerClassAttribute innerB = new InnerClassAttribute("A$foo$B", null, "B", ACC_PUBLIC | ACC_STATIC | ACC_FINAL);
+
+        extractAndCompareInnerClasses("A", innerB);
+        extractAndCompareInnerClasses("A$foo$B", innerB);
+    }
+
+    public void testAnonymousClass() {
+        InnerClassAttribute innerB = new InnerClassAttribute("A$B$1", null, null, ACC_PUBLIC | ACC_STATIC | ACC_FINAL);
+        InnerClassAttribute innerC = new InnerClassAttribute("A$foo$C$1", null, null, ACC_PUBLIC | ACC_STATIC | ACC_FINAL);
+
+        extractAndCompareInnerClasses("A", innerB, innerC);
+        extractAndCompareInnerClasses("A$B$1", innerB);
+        extractAndCompareInnerClasses("A$foo$C$1", innerC);
+    }
+
+    public void testEnumEntry() {
+        InnerClassAttribute innerE2 = new InnerClassAttribute("E$E2", "E", "E2", ACC_STATIC | ACC_FINAL);
+
+        extractAndCompareInnerClasses("E", innerE2);
+        extractAndCompareInnerClasses("E$E2", innerE2);
+    }
+
+
+
+    private void extractAndCompareInnerClasses(@NotNull String className, @NotNull InnerClassAttribute... expectedInnerClasses) {
+        assertSameElements(extractInnerClasses(className), expectedInnerClasses);
+    }
+
+    @NotNull
+    private List<InnerClassAttribute> extractInnerClasses(@NotNull String className) {
+        byte[] bytes = factory.asBytes(className + ".class");
+        ClassReader reader = new ClassReader(bytes);
+        final List<InnerClassAttribute> result = new ArrayList<InnerClassAttribute>();
+
+        reader.accept(new ClassVisitor(ASM4) {
+            @Override
+            public void visitInnerClass(String name, String outerName, String innerName, int access) {
+                result.add(new InnerClassAttribute(name, outerName, innerName, access));
+            }
+        }, ClassReader.SKIP_CODE | ClassReader.SKIP_FRAMES);
+
+        return result;
+    }
+
+    private static class InnerClassAttribute {
+        private final String name;
+        @Nullable private final String outerName;
+        @Nullable private final String innerName;
+        private final int access;
+
+        private InnerClassAttribute(@NotNull String name, @Nullable String outerName, @Nullable String innerName, int access) {
+            this.name = name;
+            this.outerName = outerName;
+            this.innerName = innerName;
+            this.access = access;
+        }
+
+        @Override
+        public boolean equals(Object o) {
+            if (this == o) return true;
+            if (o == null || getClass() != o.getClass()) return false;
+
+            InnerClassAttribute attribute = (InnerClassAttribute) o;
+
+            if (!name.equals(attribute.name)) return false;
+            if (outerName != null ? !outerName.equals(attribute.outerName) : attribute.outerName != null) return false;
+            if (innerName != null ? !innerName.equals(attribute.innerName) : attribute.innerName != null) return false;
+            if (access != attribute.access) return false;
+
+            return true;
+        }
+
+        @Override
+        public int hashCode() {
+            int result = name.hashCode();
+            result = 31 * result + (outerName != null ? outerName.hashCode() : 0);
+            result = 31 * result + (innerName != null ? innerName.hashCode() : 0);
+            result = 31 * result + access;
+            return result;
+        }
+
+        @Override
+        public String toString() {
+            return String.format("InnerClass(name=%s, outerName=%s, innerName=%s, access=%d)", name, outerName, innerName, access);
+        }
+    }
+}