Refactor KotlinJvmBinaryClass, VirtualFileKotlinClass, etc.

VirtualFileKotlinClass now reads its header and name on creation. This is not
lazy enough and may be slower in some circumstances, but has the following
advantage: if anything is wrong in the header, a VirtualFileKotlinClass
instance will not be created at all, making it nearly impossible for the client
code to operate on invalid data causing all kinds of exceptions

compiler/cli/src/org/jetbrains/jet/cli/jvm/compiler/CliVirtualFileFinder.java

@@ -37,23 +37,15 @@ public class CliVirtualFileFinder extends VirtualFileKotlinClassFinder implement
     @Override
     public VirtualFile findVirtualFileWithHeader(@NotNull FqName className) {
         for (VirtualFile root : classPath) {
-            VirtualFile fileInRoot = findKotlinFile(className, root);
-            if (fileInRoot != null) {
+            VirtualFile fileInRoot = findFileInRoot(className.asString(), root, '.');
+            //NOTE: currently we use VirtualFileFinder to find Kotlin binaries only
+            if (fileInRoot != null && KotlinBinaryClassCache.getKotlinBinaryClass(fileInRoot) != null) {
                 return fileInRoot;
             }
         }
         return null;
     }
 
-    private static VirtualFile findKotlinFile(@NotNull FqName className, @NotNull VirtualFile root) {
-        VirtualFile vFile = findFileInRoot(className.asString(), root, '.');
-        //NOTE: currently we use VirtualFileFinder to find Kotlin binaries only
-        if (vFile != null && KotlinBinaryClassCache.getKotlinBinaryClass(vFile).getClassHeader() != null) {
-            return vFile;
-        }
-        return null;
-    }
-
     @Override
     public VirtualFile findVirtualFile(@NotNull String internalName) {
         for (VirtualFile root : classPath) {

compiler/frontend.java/src/org/jetbrains/jet/lang/resolve/kotlin/KotlinBinaryClassCache.java

@@ -17,29 +17,29 @@
 package org.jetbrains.jet.lang.resolve.kotlin;
 
 import com.intellij.openapi.components.ServiceManager;
+import com.intellij.openapi.util.Ref;
 import com.intellij.openapi.vfs.VirtualFile;
 import com.intellij.util.containers.SLRUCache;
 import org.jetbrains.annotations.NotNull;
-import org.jetbrains.jet.storage.LockBasedStorageManager;
+import org.jetbrains.annotations.Nullable;
 
 public final class KotlinBinaryClassCache {
 
     // This cache must be small: we only query the same file a few times in a row (from different places)
     // Since it is on application level we should be careful about this cache. Consider profiling multiple projects indexing simultaneously.
-    private final SLRUCache<VirtualFile, KotlinJvmBinaryClass> cache = new SLRUCache<VirtualFile, KotlinJvmBinaryClass>(2, 2) {
+    private final SLRUCache<VirtualFile, Ref<VirtualFileKotlinClass>> cache = new SLRUCache<VirtualFile, Ref<VirtualFileKotlinClass>>(2, 2) {
         @NotNull
         @Override
-        public KotlinJvmBinaryClass createValue(VirtualFile virtualFile) {
-            // Operations under this lock are not supposed to involve other locks
-            return new VirtualFileKotlinClass(new LockBasedStorageManager(), virtualFile);
+        public Ref<VirtualFileKotlinClass> createValue(VirtualFile virtualFile) {
+            return Ref.create(VirtualFileKotlinClass.create(virtualFile));
         }
     };
 
-    @NotNull
+    @Nullable
     public static KotlinJvmBinaryClass getKotlinBinaryClass(@NotNull VirtualFile file) {
         KotlinBinaryClassCache service = ServiceManager.getService(KotlinBinaryClassCache.class);
         synchronized (service.cache) {
-            return service.cache.get(file);
+            return service.cache.get(file).get();
         }
     }
 }

compiler/frontend.java/src/org/jetbrains/jet/lang/resolve/kotlin/VirtualFileKotlinClass.java

@@ -19,7 +19,6 @@ package org.jetbrains.jet.lang.resolve.kotlin;
 import com.intellij.openapi.diagnostic.Logger;
 import com.intellij.openapi.util.Ref;
 import com.intellij.openapi.vfs.VirtualFile;
-import kotlin.Function0;
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
 import org.jetbrains.asm4.ClassReader;
@@ -30,9 +29,6 @@ import org.jetbrains.jet.lang.resolve.java.JvmClassName;
 import org.jetbrains.jet.lang.resolve.kotlin.header.KotlinClassHeader;
 import org.jetbrains.jet.lang.resolve.kotlin.header.ReadKotlinClassHeaderAnnotationVisitor;
 import org.jetbrains.jet.lang.resolve.name.Name;
-import org.jetbrains.jet.storage.NotNullLazyValue;
-import org.jetbrains.jet.storage.NullableLazyValue;
-import org.jetbrains.jet.storage.StorageManager;
 import org.jetbrains.jet.utils.UtilsPackage;
 
 import static org.jetbrains.asm4.ClassReader.*;
@@ -42,27 +38,49 @@ public class VirtualFileKotlinClass implements KotlinJvmBinaryClass {
     private final static Logger LOG = Logger.getInstance(VirtualFileKotlinClass.class);
 
     private final VirtualFile file;
-    private final NotNullLazyValue<JvmClassName> className;
-    private final NullableLazyValue<KotlinClassHeader> classHeader;
+    private final JvmClassName className;
+    private final KotlinClassHeader classHeader;
 
-    public VirtualFileKotlinClass(@NotNull StorageManager storageManager, @NotNull VirtualFile file) {
+    private VirtualFileKotlinClass(@NotNull VirtualFile file, @NotNull JvmClassName className, @NotNull KotlinClassHeader classHeader) {
         this.file = file;
-        this.className = storageManager.createLazyValue(
-                new Function0<JvmClassName>() {
-                    @Override
-                    public JvmClassName invoke() {
-                        return computeClassName();
-                    }
+        this.className = className;
+        this.classHeader = classHeader;
+    }
+
+    @Nullable
+    /* package */ static VirtualFileKotlinClass create(@NotNull VirtualFile file) {
+        try {
+            final ReadKotlinClassHeaderAnnotationVisitor readHeaderVisitor = new ReadKotlinClassHeaderAnnotationVisitor();
+            final Ref<JvmClassName> classNameRef = Ref.create();
+            new ClassReader(file.contentsToByteArray()).accept(new ClassVisitor(ASM4) {
+                @Override
+                public void visit(int version, int access, String name, String signature, String superName, String[] interfaces) {
+                    classNameRef.set(JvmClassName.byInternalName(name));
                 }
-        );
-        this.classHeader = storageManager.createNullableLazyValue(
-                new Function0<KotlinClassHeader>() {
-                    @Override
-                    public KotlinClassHeader invoke() {
-                        return ReadKotlinClassHeaderAnnotationVisitor.read(VirtualFileKotlinClass.this);
-                    }
+
+                @Override
+                public org.jetbrains.asm4.AnnotationVisitor visitAnnotation(String desc, boolean visible) {
+                    return convertAnnotationVisitor(readHeaderVisitor, desc);
                 }
-        );
+
+                @Override
+                public void visitEnd() {
+                    readHeaderVisitor.visitEnd();
+                }
+            }, SKIP_CODE | SKIP_DEBUG | SKIP_FRAMES);
+
+            JvmClassName className = classNameRef.get();
+            if (className == null) return null;
+
+            KotlinClassHeader header = readHeaderVisitor.createHeader();
+            if (header == null) return null;
+
+            return new VirtualFileKotlinClass(file, className, header);
+        }
+        catch (Throwable e) {
+            LOG.warn(renderFileReadingErrorMessage(file), e);
+            return null;
+        }
     }
 
     @NotNull
@@ -71,32 +89,15 @@ public class VirtualFileKotlinClass implements KotlinJvmBinaryClass {
     }
 
     @NotNull
-    private JvmClassName computeClassName() {
-        final Ref<JvmClassName> classNameRef = Ref.create();
-        try {
-            new ClassReader(file.contentsToByteArray()).accept(new ClassVisitor(ASM4) {
-                @Override
-                public void visit(int version, int access, String name, String signature, String superName, String[] interfaces) {
-                    classNameRef.set(JvmClassName.byInternalName(name));
-                }
-            }, SKIP_CODE | SKIP_DEBUG | SKIP_FRAMES);
-        }
-        catch (Throwable e) {
-            logFileReadingError(e);
-            throw UtilsPackage.rethrow(e);
-        }
-        return classNameRef.get();
+    @Override
+    public JvmClassName getClassName() {
+        return className;
     }
 
     @NotNull
-    @Override
-    public JvmClassName getClassName() {
-        return className.invoke();
-    }
-
     @Override
     public KotlinClassHeader getClassHeader() {
-        return classHeader.invoke();
+        return classHeader;
     }
 
     @Override
@@ -115,7 +116,7 @@ public class VirtualFileKotlinClass implements KotlinJvmBinaryClass {
             }, SKIP_CODE | SKIP_DEBUG | SKIP_FRAMES);
         }
         catch (Throwable e) {
-            logFileReadingError(e);
+            LOG.error(renderFileReadingErrorMessage(file), e);
             throw UtilsPackage.rethrow(e);
         }
     }
@@ -200,7 +201,7 @@ public class VirtualFileKotlinClass implements KotlinJvmBinaryClass {
             }, SKIP_CODE | SKIP_DEBUG | SKIP_FRAMES);
         }
         catch (Throwable e) {
-            logFileReadingError(e);
+            LOG.error(renderFileReadingErrorMessage(file), e);
             throw UtilsPackage.rethrow(e);
         }
     }
@@ -211,6 +212,13 @@ public class VirtualFileKotlinClass implements KotlinJvmBinaryClass {
         return JvmClassName.byInternalName(desc.substring(1, desc.length() - 1));
     }
 
+    @NotNull
+    private static String renderFileReadingErrorMessage(@NotNull VirtualFile file) {
+        return "Could not read file: " + file.getPath() + "\n"
+               + "Size in bytes: " + file.getLength() + "\n"
+               + "File type: " + file.getFileType().getName();
+    }
+
     @Override
     public int hashCode() {
         return file.hashCode();
@@ -225,13 +233,4 @@ public class VirtualFileKotlinClass implements KotlinJvmBinaryClass {
     public String toString() {
         return getClass().getSimpleName() + ": " + file.toString();
     }
-
-    private void logFileReadingError(@NotNull Throwable e) {
-        LOG.error(
-                "Could not read file: " + file.getPath() + "\n"
-                + "Size in bytes: " + file.getLength() + "\n"
-                + "File type: " + file.getFileType().getName(),
-                e
-        );
-    }
 }

core/descriptor.loader.java/src/org/jetbrains/jet/lang/resolve/java/lazy/resolvers.kt

@@ -75,8 +75,8 @@ fun LazyJavaResolverContext.findClassInJava(fqName: FqName): JavaClassLookupResu
     }
 
     val kotlinClass = kotlinClassFinder.findKotlinClass(fqName)
-    val header = kotlinClass?.getClassHeader()
-    if (kotlinClass != null && header != null) {
+    if (kotlinClass != null) {
+        val header = kotlinClass.getClassHeader()
         if (header.kind == KotlinClassHeader.Kind.CLASS) {
             val descriptor = packageFragmentProvider.resolveKotlinBinaryClass(kotlinClass)
             if (descriptor != null) {

core/descriptor.loader.java/src/org/jetbrains/jet/lang/resolve/kotlin/DeserializedDescriptorResolver.java

@@ -117,8 +117,6 @@ public final class DeserializedDescriptorResolver {
     @Nullable
     private String[] readData(@NotNull KotlinJvmBinaryClass kotlinClass, @NotNull KotlinClassHeader.Kind expectedKind) {
         KotlinClassHeader header = kotlinClass.getClassHeader();
-        if (header == null) return null;
-
         if (header.getKind() == KotlinClassHeader.Kind.INCOMPATIBLE_ABI_VERSION) {
             errorReporter.reportIncompatibleAbiVersion(kotlinClass, header.getVersion());
         }

core/descriptor.loader.java/src/org/jetbrains/jet/lang/resolve/kotlin/KotlinJvmBinaryClass.java

@@ -30,7 +30,7 @@ public interface KotlinJvmBinaryClass {
 
     void visitMembers(@NotNull MemberVisitor visitor);
 
-    @Nullable
+    @NotNull
     KotlinClassHeader getClassHeader();
 
     interface MemberVisitor {

core/descriptor.loader.java/src/org/jetbrains/jet/lang/resolve/kotlin/header/ReadKotlinClassHeaderAnnotationVisitor.java

@@ -20,7 +20,6 @@ import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
 import org.jetbrains.jet.lang.resolve.java.AbiVersionUtil;
 import org.jetbrains.jet.lang.resolve.java.JvmClassName;
-import org.jetbrains.jet.lang.resolve.kotlin.KotlinJvmBinaryClass;
 import org.jetbrains.jet.lang.resolve.name.FqName;
 import org.jetbrains.jet.lang.resolve.name.Name;
 
@@ -53,16 +52,6 @@ public class ReadKotlinClassHeaderAnnotationVisitor implements AnnotationVisitor
     private KotlinClassHeader.Kind headerKind = null;
     private KotlinSyntheticClass.Kind syntheticClassKind = null;
 
-    private ReadKotlinClassHeaderAnnotationVisitor() {
-    }
-
-    @Nullable
-    public static KotlinClassHeader read(@NotNull KotlinJvmBinaryClass kotlinClass) {
-        ReadKotlinClassHeaderAnnotationVisitor visitor = new ReadKotlinClassHeaderAnnotationVisitor();
-        kotlinClass.loadClassAnnotations(visitor);
-        return visitor.createHeader();
-    }
-
     @Nullable
     public KotlinClassHeader createHeader() {
         if (headerKind == null) {

idea/src/org/jetbrains/jet/plugin/caches/JetFromJavaDescriptorHelper.java

@@ -29,8 +29,6 @@ import org.jetbrains.jet.lang.descriptors.ClassKind;
 import org.jetbrains.jet.lang.resolve.java.JavaResolverPsiUtils;
 import org.jetbrains.jet.lang.resolve.kotlin.KotlinBinaryClassCache;
 import org.jetbrains.jet.lang.resolve.kotlin.KotlinJvmBinaryClass;
-import org.jetbrains.jet.lang.resolve.kotlin.VirtualFileFinder;
-import org.jetbrains.jet.lang.resolve.kotlin.header.KotlinClassHeader;
 import org.jetbrains.jet.lang.resolve.name.FqName;
 import org.jetbrains.jet.lang.resolve.name.Name;
 
@@ -113,9 +111,8 @@ public class JetFromJavaDescriptorHelper {
         VirtualFile virtualFile = getVirtualFileForPsiClass(psiClass);
         if (virtualFile != null) {
             KotlinJvmBinaryClass kotlinClass = KotlinBinaryClassCache.getKotlinBinaryClass(virtualFile);
-            KotlinClassHeader header = kotlinClass.getClassHeader();
-            if (header != null) {
-                return header.getAnnotationData();
+            if (kotlinClass != null) {
+                return kotlinClass.getClassHeader().getAnnotationData();
             }
         }
         return null;

idea/src/org/jetbrains/jet/plugin/libraries/DecompiledTextFactory.kt

@@ -35,10 +35,9 @@ public fun buildDecompiledText(
         resolver: ResolverForDecompiler = DeserializerForDecompiler(classFile)
 ): DecompiledText {
     val kotlinClass = KotlinBinaryClassCache.getKotlinBinaryClass(classFile)
-    val classFqName = kotlinClass.getClassName().getFqNameForClassNameWithoutDollars()
-    val classFileHeader = kotlinClass.getClassHeader()
-    assert(classFileHeader != null) { "Decompiled data factory shouldn't be called on an unsupported file: " + classFile }
-    val kind = classFileHeader!!.kind
+    assert(kotlinClass != null) { "Decompiled data factory shouldn't be called on an unsupported file: " + classFile }
+    val classFqName = kotlinClass!!.getClassName().getFqNameForClassNameWithoutDollars()
+    val kind = kotlinClass.getClassHeader().kind
     val packageFqName = classFqName.parent()
 
     return if (kind == KotlinClassHeader.Kind.PACKAGE_FACADE) {

idea/src/org/jetbrains/jet/plugin/libraries/DecompiledUtils.kt

@@ -30,7 +30,7 @@ public fun isKotlinCompiledFile(file: VirtualFile): Boolean {
     if (isKotlinCompiledFileWithIncompatibleAbiVersion(file)) {
         return false
     }
-    val header = KotlinBinaryClassCache.getKotlinBinaryClass(file).getClassHeader()
+    val header = KotlinBinaryClassCache.getKotlinBinaryClass(file)?.getClassHeader()
     return header != null && header.syntheticClassKind != KotlinSyntheticClass.Kind.TRAIT_IMPL
 }
 
@@ -38,7 +38,7 @@ public fun isKotlinCompiledFileWithIncompatibleAbiVersion(file: VirtualFile): Bo
     if (file.getExtension() != StdFileTypes.CLASS.getDefaultExtension()) {
         return false
     }
-    val header = KotlinBinaryClassCache.getKotlinBinaryClass(file).getClassHeader()
+    val header = KotlinBinaryClassCache.getKotlinBinaryClass(file)?.getClassHeader()
     return header?.kind == KotlinClassHeader.Kind.INCOMPATIBLE_ABI_VERSION
 }
 
@@ -49,6 +49,6 @@ public fun isKotlinInternalCompiledFile(file: VirtualFile): Boolean {
     if (ClassFileViewProvider.isInnerClass(file)) {
         return true
     }
-    val header = KotlinBinaryClassCache.getKotlinBinaryClass(file).getClassHeader()
+    val header = KotlinBinaryClassCache.getKotlinBinaryClass(file)?.getClassHeader()
     return header?.kind == KotlinClassHeader.Kind.SYNTHETIC_CLASS
 }

idea/src/org/jetbrains/jet/plugin/libraries/DeserializerForDecompiler.kt

@@ -46,7 +46,8 @@ import org.jetbrains.jet.lang.resolve.kotlin.ConstantDescriptorDeserializer
 
 public fun DeserializerForDecompiler(classFile: VirtualFile): DeserializerForDecompiler {
     val kotlinClass = KotlinBinaryClassCache.getKotlinBinaryClass(classFile)
-    val classFqName = kotlinClass.getClassName().getFqNameForClassNameWithoutDollars()
+    assert(kotlinClass != null) { "Decompiled data factory shouldn't be called on an unsupported file: " + classFile }
+    val classFqName = kotlinClass!!.getClassName().getFqNameForClassNameWithoutDollars()
     val packageFqName = classFqName.parent()
     return DeserializerForDecompiler(classFile.getParent()!!, packageFqName)
 }
@@ -174,7 +175,7 @@ public class DeserializerForDecompiler(val packageDirectory: VirtualFile, val di
     }
 
     private fun deserializeBinaryClass(kotlinClass: KotlinJvmBinaryClass): ClassDescriptor {
-        val data = kotlinClass.getClassHeader()?.annotationData
+        val data = kotlinClass.getClassHeader().annotationData
         if (data == null) {
             LOG.error("Annotation data missing for ${kotlinClass.getClassName()}")
         }

idea/src/org/jetbrains/jet/plugin/vfilefinder/KotlinClassFileIndex.java

@@ -76,8 +76,7 @@ public final class KotlinClassFileIndex extends ScalarIndexExtension<FqName> {
         public Map<FqName, Void> map(FileContent inputData) {
             try {
                 KotlinJvmBinaryClass kotlinClass = KotlinBinaryClassCache.getKotlinBinaryClass(inputData.getFile());
-                KotlinClassHeader header = kotlinClass.getClassHeader();
-                if (header != null && header.getKind() != KotlinClassHeader.Kind.INCOMPATIBLE_ABI_VERSION) {
+                if (kotlinClass != null && kotlinClass.getClassHeader().getKind() != KotlinClassHeader.Kind.INCOMPATIBLE_ABI_VERSION) {
                     return Collections.singletonMap(kotlinClass.getClassName().getFqNameForClassNameWithoutDollars(), null);
                 }
             }

idea/tests/org/jetbrains/jet/plugin/libraries/InternalCompiledClassesTest.kt

@@ -79,7 +79,7 @@ public class InternalCompiledClassesTest : JetLightCodeInsightFixtureTestCase()
     }
 
     private fun isSyntheticClassOfKind(kind: KotlinSyntheticClass.Kind) : VirtualFile.() -> Boolean = {
-        val header = KotlinBinaryClassCache.getKotlinBinaryClass(this).getClassHeader()
+        val header = KotlinBinaryClassCache.getKotlinBinaryClass(this)?.getClassHeader()
         header?.syntheticClassKind == kind
     }