From 961607673da3220787882cb4b546658da9f44f4f Mon Sep 17 00:00:00 2001
From: Ilya Chernikov <ilya.chernikov@jetbrains.com>
Date: Tue, 23 Jul 2019 15:08:20 +0200
Subject: [PATCH] [minor] Refactor properties collection builder:

add a helper to replace the value only if it is absent or default
rearrange some functions
---
 .../host/configurationFromTemplate.kt         | 16 +++------
 .../experimental/util/propertiesCollection.kt | 22 +++++++-----
 .../jvmhost/test/ConfigurationDslTest.kt      | 35 +++++++++++++++++++
 3 files changed, 53 insertions(+), 20 deletions(-)

diff --git a/libraries/scripting/common/src/kotlin/script/experimental/host/configurationFromTemplate.kt b/libraries/scripting/common/src/kotlin/script/experimental/host/configurationFromTemplate.kt
index 8f316a5a812..14c17b12284 100644
--- a/libraries/scripting/common/src/kotlin/script/experimental/host/configurationFromTemplate.kt
+++ b/libraries/scripting/common/src/kotlin/script/experimental/host/configurationFromTemplate.kt
@@ -72,18 +72,10 @@ fun createEvaluationConfigurationFromTemplate(
 private fun ScriptCompilationConfiguration.Builder.propertiesFromTemplate(
     templateClass: KClass<*>, baseClassType: KotlinType, mainAnnotation: KotlinScript
 ) {
-    if (baseClass() == null) {
-        baseClass(if (templateClass == baseClassType.fromClass) baseClassType else KotlinType(templateClass))
-    }
-    if (fileExtension() == null) {
-        fileExtension(mainAnnotation.fileExtension)
-    }
-    if (filePathPattern() == null) {
-        filePathPattern(mainAnnotation.filePathPattern)
-    }
-    if (displayName() == null) {
-        displayName(mainAnnotation.displayName)
-    }
+    baseClass.replaceOnlyDefault(if (templateClass == baseClassType.fromClass) baseClassType else KotlinType(templateClass))
+    fileExtension.replaceOnlyDefault(mainAnnotation.fileExtension)
+    filePathPattern.replaceOnlyDefault(mainAnnotation.filePathPattern)
+    displayName.replaceOnlyDefault(mainAnnotation.displayName)
 }
 
 private val KClass<*>.kotlinScriptAnnotation: KotlinScript
diff --git a/libraries/scripting/common/src/kotlin/script/experimental/util/propertiesCollection.kt b/libraries/scripting/common/src/kotlin/script/experimental/util/propertiesCollection.kt
index 61d2a8d96f0..a1e1856751b 100644
--- a/libraries/scripting/common/src/kotlin/script/experimental/util/propertiesCollection.kt
+++ b/libraries/scripting/common/src/kotlin/script/experimental/util/propertiesCollection.kt
@@ -89,12 +89,26 @@ open class PropertiesCollection(private val properties: Map<Key<*>, Any?> = empt
             }
         }
 
+        fun <T> PropertiesCollection.Key<T>.replaceOnlyDefault(v: T?) {
+            if (!data.containsKey(this) || data[this] == this.getDefaultValue(PropertiesCollection(data))) {
+                data[this] = v
+            }
+        }
+
+        // generic for lists
+
         fun <T> PropertiesCollection.Key<in List<T>>.putIfAny(vals: Iterable<T>?) {
             if (vals?.any() == true) {
                 data[this] = if (vals is List) vals else vals.toList()
             }
         }
 
+        operator fun <T> PropertiesCollection.Key<in List<T>>.invoke(vararg vals: T) {
+            append(vals.asIterable())
+        }
+
+        // generic for maps:
+
         @JvmName("putIfAny_map")
         fun <K, V> PropertiesCollection.Key<in Map<K, V>>.putIfAny(vals: Iterable<Pair<K, V>>?) {
             if (vals?.any() == true) {
@@ -108,14 +122,6 @@ open class PropertiesCollection(private val properties: Map<Key<*>, Any?> = empt
             }
         }
 
-        // generic for lists
-
-        operator fun <T> PropertiesCollection.Key<in List<T>>.invoke(vararg vals: T) {
-            append(vals.asIterable())
-        }
-
-        // generic for maps:
-
         operator fun <K, V> PropertiesCollection.Key<Map<K, V>>.invoke(vararg vs: Pair<K, V>) {
             append(vs.asIterable())
         }
diff --git a/libraries/scripting/jvm-host-test/test/kotlin/script/experimental/jvmhost/test/ConfigurationDslTest.kt b/libraries/scripting/jvm-host-test/test/kotlin/script/experimental/jvmhost/test/ConfigurationDslTest.kt
index 9a816184d96..b8672c322ac 100644
--- a/libraries/scripting/jvm-host-test/test/kotlin/script/experimental/jvmhost/test/ConfigurationDslTest.kt
+++ b/libraries/scripting/jvm-host-test/test/kotlin/script/experimental/jvmhost/test/ConfigurationDslTest.kt
@@ -125,6 +125,41 @@ class ConfigurationDslTest : TestCase() {
 
         Assert.assertEquals(propAnn1 + propAnn12, (evalRes.returnValue as ResultValue.Value).value)
     }
+
+    @Test
+    fun testDefaultConfiguration() {
+        val script = "val x = 1".toScriptSource()
+
+        val evalRes = runBlocking {
+            JvmScriptCompiler(defaultJvmScriptingHostConfiguration).invoke(script, ScriptCompilationConfiguration()).onSuccess {
+                BasicJvmScriptEvaluator().invoke(it, ScriptEvaluationConfiguration())
+            }.valueOrThrow()
+        }
+
+        val scriptObj = evalRes.returnValue.scriptInstance!!
+
+        Assert.assertEquals(Any::class.java, scriptObj::class.java.superclass)
+    }
+
+    @Test
+    fun testReplaceOnlyDefault() {
+        val conf = ScriptCompilationConfiguration {
+            displayName("1")
+            baseClass(KotlinType(Any::class))
+        }
+
+        val conf2 = conf.with {
+            displayName.replaceOnlyDefault("2")
+            baseClass.replaceOnlyDefault(KotlinType(Int::class))
+            fileExtension.replaceOnlyDefault("ktx")
+            filePathPattern.replaceOnlyDefault("x.*x")
+        }
+
+        Assert.assertEquals("1", conf2[ScriptCompilationConfiguration.displayName])
+        Assert.assertEquals(KotlinType(Int::class), conf2[ScriptCompilationConfiguration.baseClass])
+        Assert.assertEquals("ktx", conf2[ScriptCompilationConfiguration.fileExtension])
+        Assert.assertEquals("x.*x", conf2[ScriptCompilationConfiguration.filePathPattern])
+    }
 }
 
 @Target(AnnotationTarget.FILE)