Refactor annotation resolving, some tests fixed, more tests needed

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

@@ -67,8 +67,7 @@ internal class KtAnnotationWrapper(val psi: KtAnnotationEntry, val targetClass:
 
     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
+            method?.name?.let { annParams[it] }
         }
     }
 

libraries/tools/kotlin-maven-plugin-test/src/it/test-executeKotlinScriptWithTemplate/pom.xml

@@ -22,7 +22,7 @@
      </dependencies>
 
     <properties>
-        <kotlin.compiler.scriptTemplates>org.jetbrains.kotlin.script.util.templates.StandardScriptTemplate</kotlin.compiler.scriptTemplates>
+        <kotlin.compiler.scriptTemplates>org.jetbrains.kotlin.script.util.templates.StandardArgsScriptTemplateWithLocalResolving</kotlin.compiler.scriptTemplates>
         <kotlin.compiler.scriptClasspath>${org.jetbrains.kotlin:kotlin-script-util:jar}</kotlin.compiler.scriptClasspath>
     </properties>
 

libraries/tools/kotlin-maven-plugin-test/src/it/test-executeKotlinScriptWithTemplate/script.kts

@@ -3,7 +3,7 @@ import java.lang.Exception
 if (!args.isEmpty())
     println("some args passed")
 
-if (this !is org.jetbrains.kotlin.script.util.templates.StandardScriptTemplate)
+if (this !is org.jetbrains.kotlin.script.util.templates.StandardArgsScriptTemplateWithLocalResolving)
     throw Exception("Unexpected script base class")
 
 println("Hello from Kotlin script file!")

libraries/tools/kotlin-script-util/src/main/kotlin/org/jetbrains/kotlin/script/util/annotations.kt

@@ -20,11 +20,11 @@ package org.jetbrains.kotlin.script.util
 // in case of maven resolver the maven coordinates string is accepted (resolved with com.jcabi.aether library)
 @Target(AnnotationTarget.FILE, AnnotationTarget.EXPRESSION, AnnotationTarget.LOCAL_VARIABLE, AnnotationTarget.FUNCTION, AnnotationTarget.PROPERTY)
 @Retention(AnnotationRetention.RUNTIME)
-annotation class DependsOn(val value: String)
+annotation class DependsOn(val value: String = "", val groupId: String = "", val artifactId: String = "", val version: String = "")
 
 // only flat directory repositories are supported now, so value should be a path to a directory with jars
 // TODO: support other types of repos
 @Target(AnnotationTarget.FILE, AnnotationTarget.EXPRESSION, AnnotationTarget.LOCAL_VARIABLE, AnnotationTarget.FUNCTION, AnnotationTarget.PROPERTY)
 @Retention(AnnotationRetention.RUNTIME)
-annotation class Repository(val value: String)
+annotation class Repository(val value: String = "", val id: String = "", val url: String = "")
 

libraries/tools/kotlin-script-util/src/main/kotlin/org/jetbrains/kotlin/script/util/context.kt

@@ -52,19 +52,3 @@ private inline fun <R> ifFailed(default: R, block: () -> R) = try {
     default
 }
 
-private val defaultScriptContextClasspath: List<File> by lazy {
-    classpathFromClass(Thread.currentThread().contextClassLoader, KotlinAnnotatedScriptDependenciesResolver::class)
-    ?: classpathFromClasspathProperty()
-    ?: classpathFromClassloader(Thread.currentThread().contextClassLoader)
-    ?: emptyList()
-}
-
-// NOTE: context-based resolvers may behave unexpectedly with daemon-based REPLs.
-// since the context is calculated at the point of resolver creation
-// TODO: consider removing as confusing
-class ContextBasedResolver :
-        KotlinAnnotatedScriptDependenciesResolver(defaultScriptContextClasspath, arrayListOf())
-
-class ContextAndAnnotationsBasedResolver :
-        KotlinAnnotatedScriptDependenciesResolver(defaultScriptContextClasspath, arrayListOf(DirectResolver(), MavenResolver()))
-

libraries/tools/kotlin-script-util/src/main/kotlin/org/jetbrains/kotlin/script/util/resolve.kt

@@ -25,10 +25,7 @@ import org.jetbrains.kotlin.utils.addToStdlib.check
 import java.io.File
 import java.lang.Exception
 import java.lang.IllegalArgumentException
-import java.net.URL
-import java.net.URLClassLoader
 import java.util.concurrent.Future
-import kotlin.reflect.KClass
 
 open class KotlinAnnotatedScriptDependenciesResolver(val baseClassPath: List<File>, resolvers: Iterable<Resolver>)
     : ScriptDependenciesResolver
@@ -53,13 +50,14 @@ open class KotlinAnnotatedScriptDependenciesResolver(val baseClassPath: List<Fil
     }
 
     private fun resolveFromAnnotations(script: ScriptContents): List<File> {
-        script.annotations.forEach {
-            when (it) {
-                is Repository -> File(it.value).check { it.exists() && it.isDirectory }?.let { resolvers.add(FlatLibDirectoryResolver(it)) }
-                                                ?: throw IllegalArgumentException("Illegal argument for Repository annotation: ${it.value}")
+        script.annotations.forEach { annotation ->
+            when (annotation) {
+                is Repository -> FlatLibDirectoryResolver.tryCreate(annotation)?.apply { resolvers.add(this) }
+                        ?: resolvers.find { it is MavenResolver }?.check { (it as MavenResolver).tryAddRepo(annotation) }
+                        ?: throw IllegalArgumentException("Illegal argument for Repository annotation: $annotation")
                 is DependsOn -> {}
-                is InvalidScriptResolverAnnotation -> throw Exception("Invalid annotation ${it.name}", it.error)
-                else -> throw Exception("Unknown annotation ${it.javaClass}")
+                is InvalidScriptResolverAnnotation -> throw Exception("Invalid annotation ${annotation.name}", annotation.error)
+                else -> throw Exception("Unknown annotation ${annotation.javaClass}")
             }
         }
         return script.annotations.filterIsInstance(DependsOn::class.java).flatMap { dep ->
@@ -69,5 +67,8 @@ open class KotlinAnnotatedScriptDependenciesResolver(val baseClassPath: List<Fil
     }
 }
 
-class AnnotationsBasedResolver :
+class LocalFilesResolver :
+        KotlinAnnotatedScriptDependenciesResolver(emptyList(), arrayListOf(DirectResolver()))
+
+class FilesAndMavenResolver :
         KotlinAnnotatedScriptDependenciesResolver(emptyList(), arrayListOf(DirectResolver(), MavenResolver()))

libraries/tools/kotlin-script-util/src/main/kotlin/org/jetbrains/kotlin/script/util/resolvers/basic.kt

@@ -17,6 +17,8 @@
 package org.jetbrains.kotlin.script.util.resolvers
 
 import org.jetbrains.kotlin.script.util.DependsOn
+import org.jetbrains.kotlin.script.util.Repository
+import org.jetbrains.kotlin.utils.addToStdlib.check
 import java.io.File
 import java.lang.IllegalArgumentException
 
@@ -26,7 +28,7 @@ interface Resolver {
 
 class DirectResolver : Resolver {
     override fun tryResolve(dependsOn: DependsOn): Iterable<File>? =
-            if (dependsOn.value.isNotBlank() && File(dependsOn.value).exists()) listOf(File(dependsOn.value)) else null
+            dependsOn.value?.let(::File)?.check { it.exists() && (it.isFile || it.isDirectory) }?.let { listOf(it) }
 }
 
 class FlatLibDirectoryResolver(val path: File) : Resolver {
@@ -36,9 +38,11 @@ class FlatLibDirectoryResolver(val path: File) : Resolver {
     }
 
     override fun tryResolve(dependsOn: DependsOn): Iterable<File>? =
-            when {
-                dependsOn.value.isNotBlank() && File(path, dependsOn.value).exists() -> listOf(File(path, dependsOn.value))
-                // TODO: add coordinates and wildcard matching
-                else -> null
-            }
+            // TODO: add coordinates and wildcard matching
+            dependsOn.value?.let{ File(path, it) }?.check { it.exists() && (it.isFile || it.isDirectory) }?.let { listOf(it) }
+
+    companion object {
+        fun tryCreate(annotation: Repository): FlatLibDirectoryResolver? =
+                annotation.value?.check(String::isNotBlank)?.let(::File)?.check { it.exists() && it.isDirectory }?.let(::FlatLibDirectoryResolver)
+    }
 }

libraries/tools/kotlin-script-util/src/main/kotlin/org/jetbrains/kotlin/script/util/resolvers/maven.kt

@@ -14,20 +14,27 @@
  * limitations under the License.
  */
 
+@file:DependsOn("org.funktionale:funktionale:0.9.6")
+
 package org.jetbrains.kotlin.script.util.resolvers
 
 import com.jcabi.aether.Aether
 import org.jetbrains.kotlin.script.util.DependsOn
+import org.jetbrains.kotlin.script.util.Repository
+import org.jetbrains.kotlin.utils.addToStdlib.check
+import org.jetbrains.kotlin.utils.rethrow
 import java.io.File
 import java.util.*
 import org.sonatype.aether.repository.RemoteRepository
 import org.sonatype.aether.resolution.DependencyResolutionException
 import org.sonatype.aether.util.artifact.DefaultArtifact
 import org.sonatype.aether.util.artifact.JavaScopes
+import java.net.MalformedURLException
+import java.net.URL
 
 val mavenCentral = RemoteRepository("maven-central", "default", "http://repo1.maven.org/maven2/")
 
-class MavenResolver(val reportError: ((String) -> Unit) = {}): Resolver {
+class MavenResolver(val reportError: ((String) -> Unit)? = null): Resolver {
 
     // TODO: make robust
     val localRepo = File(File(System.getProperty("user.home")!!, ".m2"), "repository")
@@ -36,23 +43,59 @@ class MavenResolver(val reportError: ((String) -> Unit) = {}): Resolver {
 
     private fun currentRepos() = if (repos.isEmpty()) arrayListOf(mavenCentral) else repos
 
+    private fun String?.isValidParam() = if (this != null && isNotBlank()) true else false
+
     override fun tryResolve(dependsOn: DependsOn): Iterable<File>? {
-        if (dependsOn.value.count { it == ':' } == 2) {
-            try {
-                val deps = Aether(currentRepos(), localRepo).resolve(
-                        DefaultArtifact(dependsOn.value),
-                        JavaScopes.RUNTIME)
-                if (deps != null)
-                    return deps.map { it.file }
-                else {
-                    reportError("resolving [${dependsOn.value}] failed: no results")
-                }
+
+        fun error(msg: String) {
+            reportError?.invoke(msg) ?: throw RuntimeException(msg)
+        }
+
+        fun String?.orNullIfBlank(): String? = this?.check(String::isNotBlank)
+
+        val artifactId: DefaultArtifact = when {
+            dependsOn.groupId.isValidParam() || dependsOn.artifactId.isValidParam() -> {
+                val s1 = dependsOn.groupId
+                val s2 = dependsOn.artifactId
+                DefaultArtifact(dependsOn.groupId.orNullIfBlank(), dependsOn.artifactId.orNullIfBlank(), null, dependsOn.version.orNullIfBlank())
             }
-            catch (e: DependencyResolutionException) {
-                reportError("resolving [${dependsOn.value}] failed: $e")
+            dependsOn.value.isValidParam() && dependsOn.value.count { it == ':' } == 2 -> {
+                val s = dependsOn.value
+                DefaultArtifact(s)
             }
-            return listOf()
+            else -> {
+                error("Unknown set of arguments to maven resolver: ${dependsOn.value}")
+                return null
+            }
+        }
+
+        try {
+            val deps = Aether(currentRepos(), localRepo).resolve( artifactId, JavaScopes.RUNTIME)
+            if (deps != null)
+                return deps.map { it.file }
+            else {
+                error("resolving ${artifactId.artifactId} failed: no results")
+            }
+        }
+        catch (e: DependencyResolutionException) {
+            reportError?.invoke("resolving ${artifactId.artifactId} failed: $e") ?: rethrow(e)
         }
         return null
     }
+
+    fun tryAddRepo(annotation: Repository): Boolean {
+        val urlStr = if (annotation.url.isValidParam()) annotation.url else annotation.value ?: return false
+        try {
+            URL(urlStr)
+        } catch (_: MalformedURLException) {
+            return false
+        }
+        repos.add(
+                RemoteRepository(
+                        if (annotation.id.isValidParam()) annotation.id else "cantral",
+                        "default",
+                        urlStr
+                ))
+        return true
+    }
 }

libraries/tools/kotlin-script-util/src/main/kotlin/org/jetbrains/kotlin/script/util/templates/templates.kt

@@ -19,16 +19,17 @@
 package org.jetbrains.kotlin.script.util.templates
 
 import org.jetbrains.kotlin.script.ScriptTemplateDefinition
-import org.jetbrains.kotlin.script.util.AnnotationsBasedResolver
+import org.jetbrains.kotlin.script.util.FilesAndMavenResolver
+import org.jetbrains.kotlin.script.util.LocalFilesResolver
 
-@ScriptTemplateDefinition(scriptFilePattern = ".*\\.kts")
-abstract class StandardScriptTemplate(val args: Array<String>)
+@ScriptTemplateDefinition(resolver = LocalFilesResolver::class, scriptFilePattern = ".*\\.kts")
+abstract class StandardArgsScriptTemplateWithLocalResolving(val args: Array<String>)
 
-@ScriptTemplateDefinition(resolver = AnnotationsBasedResolver::class, scriptFilePattern = ".*\\.kts")
-abstract class StandardScriptTemplateWithAnnotatedResolving(val args: Array<String>)
+@ScriptTemplateDefinition(resolver = FilesAndMavenResolver::class, scriptFilePattern = ".*\\.kts")
+abstract class StandardArgsScriptTemplateWithMavenResolving(val args: Array<String>)
 
-@ScriptTemplateDefinition(scriptFilePattern = ".*\\.kts")
-abstract class ScriptTemplateWithBindings(val bindings: Map<String, Any?>)
+@ScriptTemplateDefinition(resolver = LocalFilesResolver::class, scriptFilePattern = ".*\\.kts")
+abstract class BindingsScriptTemplateWithLocalResolving(val bindings: Map<String, Any?>)
 
-@ScriptTemplateDefinition(resolver = AnnotationsBasedResolver::class, scriptFilePattern = ".*\\.kts")
-abstract class ScriptTemplateWithBindingsAndAnnotatedResolving(val bindings: Map<String, Any?>)
+@ScriptTemplateDefinition(resolver = FilesAndMavenResolver::class, scriptFilePattern = ".*\\.kts")
+abstract class BindingsScriptTemplateWithMavenResolving(val bindings: Map<String, Any?>)

libraries/tools/kotlin-script-util/src/test/kotlin/org/jetbrains/kotlin/script/util/ScriptUtilIT.kt

@@ -31,9 +31,9 @@ import org.jetbrains.kotlin.config.JVMConfigurationKeys
 import org.jetbrains.kotlin.config.addKotlinSourceRoot
 import org.jetbrains.kotlin.script.KotlinScriptDefinition
 import org.jetbrains.kotlin.script.KotlinScriptDefinitionFromAnnotatedTemplate
-import org.jetbrains.kotlin.script.util.templates.ScriptTemplateWithBindings
-import org.jetbrains.kotlin.script.util.templates.StandardScriptTemplate
-import org.jetbrains.kotlin.script.util.templates.StandardScriptTemplateWithAnnotatedResolving
+import org.jetbrains.kotlin.script.util.templates.BindingsScriptTemplateWithLocalResolving
+import org.jetbrains.kotlin.script.util.templates.StandardArgsScriptTemplateWithLocalResolving
+import org.jetbrains.kotlin.script.util.templates.StandardArgsScriptTemplateWithMavenResolving
 import org.jetbrains.kotlin.utils.PathUtil
 import org.jetbrains.kotlin.utils.PathUtil.getResourcePathForClass
 import org.junit.Assert
@@ -63,7 +63,7 @@ done
 
     @Test
     fun testArgsHelloWorld() {
-        val scriptClass = compileScript("args-hello-world.kts", StandardScriptTemplate::class)
+        val scriptClass = compileScript("args-hello-world.kts", StandardArgsScriptTemplateWithLocalResolving::class)
         Assert.assertNotNull(scriptClass)
         val ctor = scriptClass?.getConstructor(Array<String>::class.java)
         Assert.assertNotNull(ctor)
@@ -76,7 +76,7 @@ done
 
     @Test
     fun testBndHelloWorld() {
-        val scriptClass = compileScript("bindings-hello-world.kts", ScriptTemplateWithBindings::class)
+        val scriptClass = compileScript("bindings-hello-world.kts", BindingsScriptTemplateWithLocalResolving::class)
         Assert.assertNotNull(scriptClass)
         val ctor = scriptClass?.getConstructor(Map::class.java)
         Assert.assertNotNull(ctor)
@@ -88,19 +88,12 @@ done
     }
 
     @Test
-    fun testResolveStdHelloWorld() {
-        try {
-            compileScript("args-junit-hello-world.kts", StandardScriptTemplate::class)
-            Assert.fail("Should throw exception")
-        }
-        catch (e: Exception) {
-            val expectedMsg = "Unable to resolve dependency"
-            Assert.assertTrue("Expecting message \"$expectedMsg...\"", e.message?.startsWith(expectedMsg) ?: false)
-        }
+    fun testResolveStdJUnitHelloWorld() {
+        Assert.assertNull(compileScript("args-junit-hello-world.kts", StandardArgsScriptTemplateWithLocalResolving::class))
 
-        val scriptClass = compileScript("args-junit-hello-world.kts", StandardScriptTemplateWithAnnotatedResolving::class)
+        val scriptClass = compileScript("args-junit-hello-world.kts", StandardArgsScriptTemplateWithMavenResolving::class)
         if (scriptClass == null) {
-            val resolver = AnnotationsBasedResolver()
+            val resolver = FilesAndMavenResolver()
             System.err.println(resolver.baseClassPath)
         }
         Assert.assertNotNull(scriptClass)
@@ -149,7 +142,8 @@ done
                     else {
                         // attempt to workaround some maven quirks
                         manifestClassPath(Thread.currentThread().contextClassLoader)?.let {
-                            addJvmClasspathRoots(it)
+                            val files = it.filter { it.name.startsWith("kotlin-") }
+                            addJvmClasspathRoots(files)
                         }
                     }
                 }