Next round of refactoring after review

compiler/rmi/rmi-interface/src/org/jetbrains/kotlin/rmi/DaemonParams.kt

@@ -37,46 +37,6 @@ public val COMPILE_DAEMON_MEMORY_THRESHOLD_INFINITE: Long = 0L
 
 val COMPILER_ID_DIGEST = "MD5"
 
-//open class PropExtractor<C, V, P: KProperty1<C, V>>(val dest: C,
-//                                                    val prop: P,
-//                                                    val name: String,
-//                                                    val convert: ((v: V) -> String?) = { it.toString() },
-//                                                    val skipIf: ((v: V) -> Boolean) = { false },
-//                                                    val mergeWithDelimiter: String? = null)
-//{
-//    constructor(dest: C, prop: P, convert: ((v: V) -> String?) = { it.toString() }, skipIf: ((v: V) -> Boolean) = { false }) : this(dest, prop, prop.name, convert, skipIf)
-//    open fun extract(prefix: String = COMPILE_DAEMON_CMDLINE_OPTIONS_PREFIX): List<String> =
-//            when {
-//                skipIf(prop.get(dest)) -> listOf<String>()
-//                mergeWithDelimiter != null -> listOf(prefix + name + mergeWithDelimiter + convert(prop.get(dest))).filterNotNull()
-//                else -> listOf(prefix + name, convert(prop.get(dest))).filterNotNull()
-//            }
-//}
-//
-//class BoolPropExtractor<C, P: KMutableProperty1<C, Boolean>>(dest: C, prop: P, name: String? = null)
-//    : PropExtractor<C, Boolean, P>(dest, prop, name ?: prop.name, convert = { null }, skipIf = { !prop.get(dest) })
-//
-//class RestPropExtractor<C, P: KMutableProperty1<C, out MutableCollection<String>>>(dest: C, prop: P) : PropExtractor<C, MutableCollection<String>, P>(dest, prop, convert = { null }) {
-//    override fun extract(prefix: String): List<String> = prop.get(dest).map { prefix + it }
-//}
-//
-//
-//open class PropParser<C, V, P: KMutableProperty1<C, V>>(val dest: C,
-//                                                        val prop: P, alternativeNames: List<String>,
-//                                                        val parse: (s: String) -> V,
-//                                                        val allowMergedArg: Boolean = false) {
-//    val names = listOf(prop.name) + alternativeNames
-//    constructor(dest: C, prop: P, parse: (s: String) -> V, allowMergedArg: Boolean = false) : this(dest, prop, listOf(), parse, allowMergedArg)
-//    fun apply(s: String) = prop.set(dest, parse(s))
-//}
-//
-//class BoolPropParser<C, P: KMutableProperty1<C, Boolean>>(dest: C, prop: P): PropParser<C, Boolean, P>(dest, prop, { true })
-//
-//class RestPropParser<C, P: KMutableProperty1<C, MutableCollection<String>>>(dest: C, prop: P): PropParser<C, MutableCollection<String>, P>(dest, prop, { arrayListOf() }) {
-//    fun add(s: String) { prop.get(dest).add(s) }
-//}
-
-// --------------------------------------------------------
 
 open class PropMapper<C, V, P: KMutableProperty1<C, V>>(val dest: C,
                                                         val prop: P,
@@ -89,10 +49,10 @@ open class PropMapper<C, V, P: KMutableProperty1<C, V>>(val dest: C,
     open fun toArgs(prefix: String = COMPILE_DAEMON_CMDLINE_OPTIONS_PREFIX): List<String> =
             when {
                 skipIf(prop.get(dest)) -> listOf<String>()
-                mergeDelimiter != null -> listOf(prefix + names.first() + mergeDelimiter + toString(prop.get(dest))).filterNotNull()
+                mergeDelimiter != null -> listOf( listOf(prefix + names.first(), toString(prop.get(dest))).filterNotNull().joinToString(mergeDelimiter))
                 else -> listOf(prefix + names.first(), toString(prop.get(dest))).filterNotNull()
             }
-    fun apply(s: String) = prop.set(dest, fromString(s))
+    open fun apply(s: String) = prop.set(dest, fromString(s))
 }
 
 class StringPropMapper<C, P: KMutableProperty1<C, String>>(dest: C,
@@ -113,69 +73,65 @@ class RestPropMapper<C, P: KMutableProperty1<C, MutableCollection<String>>>(dest
     : PropMapper<C, MutableCollection<String>, P>(dest = dest, prop = prop, toString = { null }, fromString = { arrayListOf() }) 
 {
     override fun toArgs(prefix: String): List<String> = prop.get(dest).map { prefix + it }
+    override fun apply(s: String) = add(s)
     fun add(s: String) { prop.get(dest).add(s) }
 }
 
-// ------------------------------------------
+
+inline fun <T, R: Any> Iterable<T>.firstMapOrNull(mappingPredicate: (T) -> Pair<Boolean, R?>): R? {
+    for (element in this) {
+        val (found, mapped) = mappingPredicate(element)
+        if (found) return mapped
+    }
+    return null
+}
+
 
 fun Iterable<String>.filterExtractProps(propMappers: List<PropMapper<*,*,*>>, prefix: String, restParser: RestPropMapper<*,*>? = null) : Iterable<String>  {
-    var currentPropMapper: PropMapper<*,*,*>? = null
+
-    var matchingOption = ""
+    val iter = iterator()
-    val res = filter { param ->
+    val rest = arrayListOf<String>()
-        if (currentPropMapper == null) {
+
-            val propMapper = propMappers.find {
+    while (iter.hasNext()) {
-                it !is RestPropMapper<*,*> &&
+        val param = iter.next()
-                it.names.any { name ->
+        val (propMapper, matchingOption) = propMappers.firstMapOrNull {
-                    if (param.startsWith(prefix + name)) {
+            val name = if (it !is RestPropMapper<*,*>) it.names.firstOrNull { param.startsWith(prefix + it) } else null
-                        matchingOption = prefix + name
+            Pair(name != null, Pair(it, name))
-                        true
+        } ?: Pair(null, null)
+
+        when {
+            propMapper != null -> {
+                val optionLength = prefix.length() + matchingOption!!.length()
+                when {
+                    propMapper is BoolPropMapper<*,*> -> {
+                        if (param.length() > optionLength)
+                            throw IllegalArgumentException("Invalid switch option '$param', expecting $prefix$matchingOption without arguments")
+                        propMapper.apply("")
                     }
-                    else {
+                    param.length() > optionLength ->
-                        false
+                        if (param[optionLength] != '=') {
-                    }
+                            if (propMapper.mergeDelimiter == null)
-                }
+                                throw IllegalArgumentException("Invalid option syntax '$param', expecting $prefix$matchingOption[= ]<arg>")
-            }
+                            propMapper.apply(param.substring(optionLength))
-            when {
-                propMapper != null -> {
-                    val optionLength = matchingOption.length()
-                    when {
-                        propMapper is BoolPropMapper<*,*> -> {
-                            if (param.length() > optionLength)
-                                throw IllegalArgumentException("Invalid switch option '$param', expecting $matchingOption without arguments")
-                            propMapper.apply("")
                         }
-                        param.length() > optionLength ->
+                        else {
-                            if (param[optionLength] != '=') {
+                            propMapper.apply(param.substring(optionLength + 1))
-                                if (propMapper.mergeDelimiter == null)
+                        }
-                                    throw IllegalArgumentException("Invalid option syntax '$param', expecting $matchingOption[= ]<arg>")
+                    else -> {
-                                propMapper.apply(param.substring(optionLength))
+                        if (!iter.hasNext()) throw IllegalArgumentException("Expecting argument for the option $prefix$matchingOption")
-                            }
+                        propMapper.apply(iter.next())
-                            else {
-                                propMapper.apply(param.substring(optionLength + 1))
-                            }
-                        else ->
-                            currentPropMapper = propMapper
                     }
-                    false
                 }
-                restParser != null && param.startsWith(prefix) -> {
-                    restParser.add(param.removePrefix(prefix))
-                    false
-                }
-                else -> true
             }
-        }
+            restParser != null && param.startsWith(prefix) ->
-        else {
+                restParser.add(param.removePrefix(prefix))
-            currentPropMapper!!.apply(param)
+            else -> rest.add(param)
-            currentPropMapper = null
-            false
         }
     }
-    if (currentPropMapper != null) 
+    return rest
-        throw IllegalArgumentException("Expecting argument for the option $matchingOption")
-    return res
 }
 
+
 // TODO: find out how to create more generic variant using first constructor
 //fun<C> C.propsToParams() {
 //    val kc = C::class
@@ -196,7 +152,7 @@ public data class DaemonJVMOptions(
         public var maxMemory: String = "",
         public var maxPermSize: String = "",
         public var reservedCodeCacheSize: String = "",
-        public var otherJvmParams: MutableCollection<String> = arrayListOf()
+        public var jvmParams: MutableCollection<String> = arrayListOf()
 ) : OptionsGroup {
 
     override val mappers: List<PropMapper<*,*,*>>
@@ -206,7 +162,7 @@ public data class DaemonJVMOptions(
                         restMapper)
 
     val restMapper: RestPropMapper<*,*>
-        get() = RestPropMapper(this, ::otherJvmParams)
+        get() = RestPropMapper(this, ::jvmParams)
 }
 
 public data class DaemonOptions(
@@ -292,7 +248,7 @@ public fun configureDaemonLaunchingOptions(opts: DaemonJVMOptions, inheritMemory
         ManagementFactory.getRuntimeMXBean().inputArguments.filterExtractProps(opts.mappers, "-")
 
     System.getProperty(COMPILE_DAEMON_JVM_OPTIONS_PROPERTY)?.let {
-        opts.otherJvmParams.addAll( it.trim('"', '\'').split(",").filterExtractProps(opts.mappers, "-", opts.restMapper))
+        opts.jvmParams.addAll(it.trim('"', '\'').split(",").filterExtractProps(opts.mappers, "-", opts.restMapper))
     }
     return opts
 }

compiler/rmi/rmi-server/src/org/jetbrains/kotlin/rmi/service/CompileServiceImpl.kt

@@ -17,6 +17,7 @@
 package org.jetbrains.kotlin.service
 
 import org.jetbrains.kotlin.cli.common.CLICompiler
+import org.jetbrains.kotlin.cli.common.ExitCode
 import org.jetbrains.kotlin.config.Services
 import org.jetbrains.kotlin.incremental.components.LookupTracker
 import org.jetbrains.kotlin.load.kotlin.incremental.components.IncrementalCache
@@ -182,7 +183,7 @@ class CompileServiceImpl<Compiler: CLICompiler<*>>(
             when (outputFormat) {
                 CompileService.OutputFormat.PLAIN -> compiler.exec(printStream, *args)
                 CompileService.OutputFormat.XML -> compiler.execAndOutputXml(printStream, Services.EMPTY, *args)
-            }.code
+            }
         }
 
     override fun remoteIncrementalCompile(args: Array<out String>, caches: Map<String, CompileService.RemoteIncrementalCache>, outputStream: RemoteOutputStream, outputFormat: CompileService.OutputFormat): Int =
@@ -190,13 +191,13 @@ class CompileServiceImpl<Compiler: CLICompiler<*>>(
             when (outputFormat) {
                 CompileService.OutputFormat.PLAIN -> throw NotImplementedError("Only XML output is supported in remote incremental compilation")
                 CompileService.OutputFormat.XML -> compiler.execAndOutputXml(printStream, createCompileServices(caches), *args)
-            }.code
+            }
         }
 
-    fun doCompile(args: Array<out String>, errStream: RemoteOutputStream, body: (PrintStream) -> Int): Int =
+    fun doCompile(args: Array<out String>, errStream: RemoteOutputStream, body: (PrintStream) -> ExitCode): Int =
         ifAlive {
             checkedCompile(args) {
-                body( PrintStream( RemoteOutputStreamClient(errStream)))
+                body( PrintStream( RemoteOutputStreamClient(errStream))).code
             }
         }
 }