Fix annotation construction with array literals

Turns out the issue happens to be that ArrayValue uses a list of values which needs to be translated to an array of the percise type before it is used by callBy

This also addresses handling of arguments after a vararg in an annotation

compiler/cli/cli-common/src/org/jetbrains/kotlin/utils/parametersMap.kt

@@ -63,8 +63,8 @@ private interface ArgsConverter<T> {
     }
 
     fun tryConvertSingle(parameter: KParameter, arg: NamedArgument<T>): Result
-    fun tryConvertVararg(parameter: KParameter, firstArg: NamedArgument<T>, restArgsIt: Iterator<NamedArgument<T>>): Result
+    fun tryConvertVararg(parameter: KParameter, firstArg: NamedArgument<T>, restArgs: Sequence<NamedArgument<T>>): Result
-    fun tryConvertTail(parameter: KParameter, firstArg: NamedArgument<T>, restArgsIt: Iterator<NamedArgument<T>>): Result
+    fun tryConvertTail(parameter: KParameter, firstArg: NamedArgument<T>, restArgs: Sequence<NamedArgument<T>>): Result
 }
 
 private enum class ArgsTraversalState { UNNAMED, NAMED, TAIL }
@@ -77,7 +77,7 @@ private fun <T> tryCreateCallableMapping(
     val res = mutableMapOf<KParameter, Any?>()
     var state = ArgsTraversalState.UNNAMED
     val unboundParams = callable.parameters.toMutableList()
-    val argIt = args.iterator()
+    val argIt = LookAheadIterator(args.iterator())
     while (argIt.hasNext()) {
         if (unboundParams.isEmpty()) return null // failed to match: no param left for the arg
         val arg = argIt.next()
@@ -102,7 +102,11 @@ private fun <T> tryCreateCallableMapping(
                     res[par] = cvtRes.v
                 } else if (par.type.jvmErasure.java.isArray) {
                     // try vararg
-                    val cvtVRes = converter.tryConvertVararg(par, arg, argIt)
+
+                    // Collect all the arguments that do not have a name
+                    val unnamed = argIt.sequenceUntil { it.name != null }
+
+                    val cvtVRes = converter.tryConvertVararg(par, arg, unnamed)
                     if (cvtVRes is ArgsConverter.Result.Success) {
                         res[par] = cvtVRes.v
                     } else return null // failed to match: no suitable param for unnamed arg
@@ -121,7 +125,7 @@ private fun <T> tryCreateCallableMapping(
             ArgsTraversalState.TAIL -> {
                 assert(arg.name == null)
                 val par = unboundParams.removeAt(unboundParams.lastIndex)
-                val cvtVRes = converter.tryConvertTail(par, arg, argIt)
+                val cvtVRes = converter.tryConvertTail(par, arg, argIt.asSequence())
                 if (cvtVRes is ArgsConverter.Result.Success) {
                     if (argIt.hasNext()) return null // failed to match: not all tail args are consumed
                     res[par] = cvtVRes.v
@@ -162,7 +166,7 @@ private class StringArgsConverter : ArgsConverter<String> {
     override fun tryConvertVararg(
         parameter: KParameter,
         firstArg: NamedArgument<String>,
-        restArgsIt: Iterator<NamedArgument<String>>
+        restArgs: Sequence<NamedArgument<String>>
     ): ArgsConverter.Result {
         fun convertPrimitivesArray(type: KType, args: Sequence<String?>): Any? =
             when (type.classifier) {
@@ -179,7 +183,7 @@ private class StringArgsConverter : ArgsConverter<String> {
 
         val parameterType = parameter.type
         if (parameterType.jvmErasure.java.isArray) {
-            val argsSequence = sequenceOf(firstArg.value) + restArgsIt.asSequence().map { it.value }
+            val argsSequence = sequenceOf(firstArg.value) + restArgs.map { it.value }
             val primArrayArgCandidate = convertPrimitivesArray(parameterType, argsSequence)
             if (primArrayArgCandidate != null)
                 return ArgsConverter.Result.Success(primArrayArgCandidate)
@@ -195,9 +199,9 @@ private class StringArgsConverter : ArgsConverter<String> {
     override fun tryConvertTail(
         parameter: KParameter,
         firstArg: NamedArgument<String>,
-        restArgsIt: Iterator<NamedArgument<String>>
+        restArgs: Sequence<NamedArgument<String>>
     ): ArgsConverter.Result =
-        tryConvertVararg(parameter, firstArg, restArgsIt)
+        tryConvertVararg(parameter, firstArg, restArgs)
 }
 
 private class AnyArgsConverter : ArgsConverter<Any> {
@@ -218,18 +222,33 @@ private class AnyArgsConverter : ArgsConverter<Any> {
                 else -> null
             }
 
-        if (value::class.isSubclassOf(parameter.type.jvmErasure)) return ArgsConverter.Result.Success(value)
+        fun evaluateValue(arg: Any): Any? {
+            if (arg::class.isSubclassOf(parameter.type.jvmErasure)) return arg
+            return convertPrimitivesArray(parameter.type, arg)
+        }
 
-        return convertPrimitivesArray(parameter.type, value)?.let { ArgsConverter.Result.Success(it) }
+        evaluateValue(value)?.let { return ArgsConverter.Result.Success(it) }
-            ?: ArgsConverter.Result.Failure
+
+        // Handle the scenario where [arg::class] is an Array<Any>
+        // but it's values could all still be valid
+        val parameterKClass = parameter.type.classifier as? KClass<*>
+        val arrayComponentType = parameterKClass?.java?.takeIf { it.isArray}?.componentType?.kotlin
+
+        if (value is Array<*> && arrayComponentType != null) {
+            // TODO: Idea! Maybe we should check if the values in the array are compatible with [arrayComponentType]
+            //  if they aren't perhaps we should fail silently
+            convertAnyArray(arrayComponentType, value.asSequence())?.let(::evaluateValue)?.let { return ArgsConverter.Result.Success(it) }
+        }
+
+        return ArgsConverter.Result.Failure
     }
 
     override fun tryConvertVararg(
-        parameter: KParameter, firstArg: NamedArgument<Any>, restArgsIt: Iterator<NamedArgument<Any>>
+        parameter: KParameter, firstArg: NamedArgument<Any>, restArgs: Sequence<NamedArgument<Any>>
     ): ArgsConverter.Result {
         val parameterType = parameter.type
         if (parameterType.jvmErasure.java.isArray) {
-            val argsSequence = sequenceOf(firstArg.value) + restArgsIt.asSequence().map { it.value }
+            val argsSequence = sequenceOf(firstArg.value) + restArgs.map { it.value }
             val arrayElementType = parameterType.arguments.firstOrNull()?.type
             val arrayArgCandidate = convertAnyArray(arrayElementType?.classifier, argsSequence)
             if (arrayArgCandidate != null)
@@ -242,7 +261,7 @@ private class AnyArgsConverter : ArgsConverter<Any> {
     override fun tryConvertTail(
         parameter: KParameter,
         firstArg: NamedArgument<Any>,
-        restArgsIt: Iterator<NamedArgument<Any>>
+        restArgs: Sequence<NamedArgument<Any>>
     ): ArgsConverter.Result =
         tryConvertSingle(parameter, firstArg)
 }
@@ -266,3 +285,38 @@ private fun <T> convertAnyArrayImpl(classifier: KClassifier?, args: Sequence<T?>
     }
     return result
 }
+
+/*
+ An iterator that allows us to read the next value without consuming it.
+ */
+private class LookAheadIterator<T>(private val iterator: Iterator<T>) : Iterator<T> {
+    private var currentLookAhead: T? = null
+
+    override fun hasNext(): Boolean {
+        return currentLookAhead != null || iterator.hasNext()
+    }
+
+    override fun next(): T {
+        currentLookAhead?.let { value ->
+            currentLookAhead = null
+            return value
+        }
+
+        return iterator.next()
+    }
+
+    fun nextWithoutConsuming(): T {
+        return currentLookAhead ?: iterator.next().also { currentLookAhead = it }
+    }
+}
+
+/*
+ Will return a sequence with the values of the iterator until the predicate evaluates to true.
+ */
+private fun <T> LookAheadIterator<T>.sequenceUntil(predicate: (T) -> Boolean): Sequence<T> = sequence {
+    while (hasNext()) {
+        if (predicate(nextWithoutConsuming()))
+            break
+        yield(next())
+    }
+}

plugins/scripting/scripting-compiler-impl/src/org/jetbrains/kotlin/scripting/resolve/scriptAnnotationsPreprocessing.kt

@@ -14,10 +14,10 @@ import org.jetbrains.kotlin.name.Name
 import org.jetbrains.kotlin.psi.*
 import org.jetbrains.kotlin.resolve.BindingTrace
 import org.jetbrains.kotlin.resolve.BindingTraceContext
-import org.jetbrains.kotlin.resolve.CollectionLiteralResolver
+import org.jetbrains.kotlin.resolve.ArrayFqNames.ARRAY_OF_FUNCTION
-import org.jetbrains.kotlin.resolve.CollectionLiteralResolver.Companion.ARRAY_OF_FUNCTION
+import org.jetbrains.kotlin.resolve.ArrayFqNames.PRIMITIVE_TYPE_TO_ARRAY
-import org.jetbrains.kotlin.resolve.CollectionLiteralResolver.Companion.PRIMITIVE_TYPE_TO_ARRAY
 import org.jetbrains.kotlin.resolve.constants.ArrayValue
+import org.jetbrains.kotlin.resolve.constants.ConstantValue
 import org.jetbrains.kotlin.resolve.constants.ConstantValueFactory
 import org.jetbrains.kotlin.resolve.constants.evaluate.ConstantExpressionEvaluator
 import org.jetbrains.kotlin.storage.LockBasedStorageManager
@@ -69,7 +69,7 @@ internal fun constructAnnotation(psi: KtAnnotationEntry, targetClass: KClass<out
 
         // 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()
-        argName to result?.value
+        argName to result?.toRuntimeValue()
     }
     val mappedArguments: Map<KParameter, Any?> =
         tryCreateCallableMappingFromNamedArgs(targetClass.constructors.first(), valueArguments)
@@ -92,6 +92,11 @@ internal fun ConstantExpressionEvaluator.evaluateToConstantArrayValue(
     return ConstantValueFactory.createArrayValue(constants, TypeUtils.NO_EXPECTED_TYPE)
 }
 
+private fun ConstantValue<*>.toRuntimeValue(): Any? = when (this) {
+    is ArrayValue -> value.map { it.toRuntimeValue() }.toTypedArray()
+    else -> value
+}
+
 // NOTE: this class is used for error reporting. But in order to pass plugin verification, it should derive directly from java's Annotation
 // and implement annotationType method (see #KT-16621 for details).
 // TODO: instead of the workaround described above, consider using a sum-type for returning errors from constructAnnotation

plugins/scripting/scripting-compiler/testData/compiler/constructAnnotations/SimpleTestAnnotation.kts

@@ -0,0 +1,2 @@
+
+@file:TestAnnotation("option")

plugins/scripting/scripting-compiler/testData/compiler/constructAnnotations/TestAnnotationEmptyVarArg.kts

@@ -0,0 +1,2 @@
+
+@file:TestAnnotation()

plugins/scripting/scripting-compiler/testData/compiler/constructAnnotations/TestAnnotationWithArrayLiteral.kts

@@ -0,0 +1,2 @@
+
+@file:TestAnnotation(options = ["option"])

plugins/scripting/scripting-compiler/testData/compiler/constructAnnotations/TestAnnotationWithArrayOfFunction.kts

@@ -0,0 +1,2 @@
+
+@file:TestAnnotation(options = arrayOf("option"))

plugins/scripting/scripting-compiler/testData/compiler/constructAnnotations/TestAnnotationWithEmptyArrayFunction.kts

@@ -0,0 +1,2 @@
+
+@file:TestAnnotation(options = emptyArray())

plugins/scripting/scripting-compiler/testData/compiler/constructAnnotations/TestAnnotationWithVarArgAndArray.kts

@@ -0,0 +1,2 @@
+
+@file:AnnotationWithVarArgAndArray("option", moreOptions = ["otherOption"])

plugins/scripting/scripting-compiler/tests/org/jetbrains/kotlin/scripting/compiler/test/ConstructAnnotationTest.kt

@@ -0,0 +1,147 @@
+/*
+ * Copyright 2010-2020 JetBrains s.r.o. and Kotlin Programming Language contributors.
+ * Use of this source code is governed by the Apache 2.0 license that can be found in the license/LICENSE.txt file.
+ */
+
+package org.jetbrains.kotlin.scripting.compiler.test
+
+import com.intellij.openapi.Disposable
+import junit.framework.TestCase
+import org.jetbrains.kotlin.cli.common.config.addKotlinSourceRoot
+import org.jetbrains.kotlin.cli.jvm.compiler.EnvironmentConfigFiles
+import org.jetbrains.kotlin.cli.jvm.compiler.KotlinCoreEnvironment
+import org.jetbrains.kotlin.script.loadScriptingPlugin
+import org.jetbrains.kotlin.scripting.compiler.plugin.TestDisposable
+import org.jetbrains.kotlin.scripting.compiler.plugin.impl.ScriptDiagnosticsMessageCollector
+import org.jetbrains.kotlin.scripting.compiler.plugin.impl.createCompilationContextFromEnvironment
+import org.jetbrains.kotlin.scripting.compiler.plugin.impl.getScriptKtFile
+import org.jetbrains.kotlin.scripting.resolve.InvalidScriptResolverAnnotation
+import org.jetbrains.kotlin.scripting.resolve.getScriptCollectedData
+import org.jetbrains.kotlin.test.ConfigurationKind
+import org.jetbrains.kotlin.test.KotlinTestUtils
+import org.jetbrains.kotlin.test.TestJdkKind
+import java.io.File
+import kotlin.reflect.KClass
+import kotlin.script.experimental.api.*
+import kotlin.script.experimental.host.toScriptSource
+import kotlin.script.experimental.jvm.jvm
+
+private const val testDataPath = "plugins/scripting/scripting-compiler/testData/compiler/constructAnnotations"
+
+@Target(AnnotationTarget.FILE)
+@Repeatable
+@Retention(AnnotationRetention.SOURCE)
+private annotation class TestAnnotation(vararg val options: String)
+
+@Target(AnnotationTarget.FILE)
+@Repeatable
+@Retention(AnnotationRetention.SOURCE)
+private annotation class AnnotationWithVarArgAndArray(vararg val options: String, val moreOptions: Array<String>)
+
+class ConstructAnnotationTest : TestCase() {
+    private val testRootDisposable: Disposable = TestDisposable()
+
+    fun testAnnotationEmptyVarArg() {
+        val annotations = annotations("TestAnnotationEmptyVarArg.kts", TestAnnotation::class)
+            .valueOrThrow()
+            .filterIsInstance(TestAnnotation::class.java)
+
+        assertEquals(annotations.count(), 1)
+        assert(annotations.first().options.isEmpty())
+    }
+
+    fun testBasicVarArgTestAnnotation() {
+        val annotations = annotations("SimpleTestAnnotation.kts", TestAnnotation::class)
+            .valueOrThrow()
+            .filterIsInstance(TestAnnotation::class.java)
+
+        assertEquals(annotations.count(), 1)
+        assertEquals(annotations.first().options.toList(), listOf("option"))
+    }
+
+    fun testAnnotationWithArrayLiteral() {
+        val annotations = annotations("TestAnnotationWithArrayLiteral.kts", TestAnnotation::class)
+            .valueOrThrow()
+            .filterIsInstance(TestAnnotation::class.java)
+
+        assertEquals(annotations.count(), 1)
+        assertEquals(annotations.first().options.toList(), listOf("option"))
+    }
+
+    fun testAnnotationWithArrayOfFunction() {
+        val annotations = annotations("TestAnnotationWithArrayOfFunction.kts", TestAnnotation::class)
+            .valueOrThrow()
+            .filterIsInstance(TestAnnotation::class.java)
+
+        assertEquals(annotations.count(), 1)
+        assertEquals(annotations.first().options.toList(), listOf("option"))
+    }
+
+    fun testAnnotationWithEmptyArrayFunction() {
+        val annotations = annotations("TestAnnotationWithEmptyArrayFunction.kts", TestAnnotation::class)
+            .valueOrThrow()
+            .filterIsInstance(TestAnnotation::class.java)
+
+        assertEquals(annotations.count(), 1)
+        assert(annotations.first().options.isEmpty())
+    }
+
+    fun testArrayAfterVarArgInAnnotation() {
+        val annotations = annotations("TestAnnotationWithVarArgAndArray.kts", AnnotationWithVarArgAndArray::class)
+            .valueOrThrow()
+            .filterIsInstance(AnnotationWithVarArgAndArray::class.java)
+
+        assertEquals(annotations.count(), 1)
+        assertEquals(annotations.first().options.toList(), listOf("option"))
+        assertEquals(annotations.first().moreOptions.toList(), listOf("otherOption"))
+    }
+
+    private fun annotations(filename: String, vararg classes: KClass<out Annotation>): ResultWithDiagnostics<List<Annotation>> {
+        val file = File(testDataPath, filename)
+        val compilationConfiguration = KotlinTestUtils.newConfiguration(ConfigurationKind.NO_KOTLIN_REFLECT, TestJdkKind.MOCK_JDK).apply {
+            addKotlinSourceRoot(file.path)
+            loadScriptingPlugin(this)
+        }
+        val configuration = ScriptCompilationConfiguration {
+            defaultImports(*classes)
+            jvm {
+                refineConfiguration {
+                    onAnnotations(*classes) {
+                        it.compilationConfiguration.asSuccess()
+                    }
+                }
+            }
+        }
+
+        val messageCollector = ScriptDiagnosticsMessageCollector(null)
+        val environment = KotlinCoreEnvironment.createForTests(
+            testRootDisposable, compilationConfiguration, EnvironmentConfigFiles.JVM_CONFIG_FILES
+        )
+        val context = createCompilationContextFromEnvironment(configuration, environment, messageCollector)
+        val source = file.toScriptSource()
+        val ktFile = getScriptKtFile(
+            source,
+            configuration,
+            context.environment.project,
+            messageCollector
+        ).valueOr { return it }
+
+        if (messageCollector.hasErrors()) {
+            return makeFailureResult(messageCollector.diagnostics)
+        }
+
+        val data = getScriptCollectedData(ktFile, configuration, environment.project, null)
+        val annotations = data[ScriptCollectedData.foundAnnotations] ?: emptyList()
+
+        annotations
+            .filterIsInstance(InvalidScriptResolverAnnotation::class.java)
+            .takeIf { it.isNotEmpty() }
+            ?.let { invalid ->
+                val reports = invalid.map { "Failed to resolve annotation of type ${it.name} due to ${it.error}".asErrorDiagnostics() }
+                return makeFailureResult(reports)
+            }
+
+        return annotations.asSuccess()
+    }
+
+}