Fixes after review

Author: ligee

compiler/cli/src/org/jetbrains/kotlin/cli/jvm/compiler/JvmDependenciesIndex.kt

@@ -58,6 +58,20 @@ interface JvmDependenciesIndex {
     ): Set<String>
 }
 
+interface JvmDependenciesIndexFactory {
+    fun makeIndexFor(roots: List<JavaRoot>): JvmDependenciesIndex
+}
+
+class JvmStaticDependenciesIndexFactory : JvmDependenciesIndexFactory {
+    override fun makeIndexFor(roots: List<JavaRoot>): JvmDependenciesIndex = JvmDependenciesIndexImpl(roots)
+}
+
+class JvmUpdatableDependenciesIndexFactory : JvmDependenciesIndexFactory {
+    override fun makeIndexFor(roots: List<JavaRoot>): JvmDependenciesIndex = JvmDependenciesDynamicCompoundIndex().apply {
+        addIndex(JvmDependenciesIndexImpl(roots))
+    }
+}
+
 // speeds up finding files/classes in classpath/java source roots
 // NOT THREADSAFE, needs to be adapted/removed if we want compiler to be multithreaded
 // the main idea of this class is for each package to store roots which contains it to avoid excessive file system traversal
@@ -355,9 +369,10 @@ class JvmDependenciesDynamicCompoundIndex() : JvmDependenciesIndex {
     }
 
     override fun collectKnownClassNamesInPackage(packageFqName: FqName): Set<String> = lock.read {
-        indices.fold(hashSetOf(), { s, index -> s.addAll(index.collectKnownClassNamesInPackage(packageFqName)); s })
+        indices.flatMapTo(hashSetOf()) { it.collectKnownClassNamesInPackage(packageFqName) }
     }
 }
 
 private fun IntArrayList.lastOrNull() = if (isEmpty) null else get(size() - 1)
-private val IntArrayList.indices: IntRange get() = 0..(size() - 1)
+private val IntArrayList.indices: IntRange get() = 0..(size() - 1)
+

compiler/cli/src/org/jetbrains/kotlin/cli/jvm/compiler/KotlinCoreEnvironment.kt

@@ -154,15 +154,15 @@ class KotlinCoreEnvironment private constructor(
         }
 
         val initialRoots = configuration.getList(JVMConfigurationKeys.CONTENT_ROOTS).classpathRoots()
-        val initialIndex = JvmDependenciesIndexImpl(initialRoots)
-        updateClasspathFromRootsIndex(initialIndex)
-
-        // replacing index with updatable one only for REPL mode, so we can add roots to classpath then compiling
-        // subsequent lines in REPL
-        rootsIndex = if (!configuration.getBoolean(CommonConfigurationKeys.REPL_MODE)) initialIndex
-                     else JvmDependenciesDynamicCompoundIndex().apply {
-                         addIndex(initialIndex)
-                     }
+
+        // TODO: pass index factory in the configuration to avoid selection logic here
+        // for a moment using updatable index for REPL mode, so we can add roots to classpath then compiling subsequent lines in REPL
+        val indexFactory = if (configuration.getBoolean(CommonConfigurationKeys.REPL_MODE)) JvmUpdatableDependenciesIndexFactory()
+                           else JvmStaticDependenciesIndexFactory()
+
+        rootsIndex = indexFactory.makeIndexFor(initialRoots)
+        updateClasspathFromRootsIndex(rootsIndex)
+
         (ServiceManager.getService(project, CoreJavaFileManager::class.java)
             as KotlinCliJavaFileManagerImpl).initIndex(rootsIndex)
 

compiler/frontend/src/org/jetbrains/kotlin/script/scriptAnnotationsPreprocessing.kt

@@ -48,12 +48,12 @@ internal class KtAnnotationWrapper(val psi: KtAnnotationEntry, val targetClass:
             // TODO: consider inspecting `trace` to find diagnostics reported during the computation (such as division by zero, integer overflow, invalid annotation parameters etc.)
             val argName = arg.getArgumentName()?.asName?.toString()
             // TODO: consider reusing arguments mapping logic from compiler code
+            // TODO: find out how to properly report problems from here (now using bogus arg names as an indicator)
             val paramName = when {
                 argName == null && !namedStarted && targetAnnParams == null -> "$" // TODO: using invalid name here. Drop when annotation constructors will be accessible (se above)
-                argName == null && !namedStarted -> targetAnnParams?.get(i)?.name ?: throw IllegalArgumentException("Unnamed argument for $name at $i")
-                argName == null && namedStarted -> throw IllegalArgumentException("Invalid argument sequence for $name at arg $i")
-                targetAnnParams != null && targetAnnParams.none { it.name == argName } ->
-                    throw IllegalArgumentException("Unknown argument $argName for $name")
+                argName == null && !namedStarted -> targetAnnParams?.get(i)?.name ?: "$(Unnamed argument for $name at $i)"
+                argName == null && namedStarted -> "$(Invalid argument sequence for $name at arg $i)"
+                targetAnnParams != null && targetAnnParams.none { it.name == argName } -> "$(Unknown argument $argName for $name)"
                 else -> {
                     namedStarted = true
                     argName!!
@@ -64,7 +64,7 @@ internal class KtAnnotationWrapper(val psi: KtAnnotationEntry, val targetClass:
         res
     }
 
-    internal class AnnProxyInvocationHandler<out K: KClass<out Any>>(val targetAnnClass: K, val annParams: Map<String, Any?>) : InvocationHandler {
+    internal class AnnProxyInvocationHandler(val targetAnnClass: KClass<out Annotation>, val annParams: Map<String, Any?>) : InvocationHandler {
         override fun invoke(proxy: Any?, method: Method?, params: Array<out Any>?): Any? = method?.let {
             // TODO: the functionality with checking annParams size is here only to workaround missing access to constructors in annotations. Drop as soon as possible (see above)
             annParams[it.name] ?: if (annParams.size == 1) annParams.values.firstOrNull() else null