Change SubCommandDescriptor calling from callSuspend to callSuspendBy

- Support optional argument now.

backend/mirai-console/src/main/kotlin/net/mamoe/mirai/console/command/description/CommandArgumentContext.kt

@@ -20,6 +20,7 @@ import net.mamoe.mirai.console.permission.PermissionId
 import net.mamoe.mirai.console.permission.PermitteeId
 import net.mamoe.mirai.console.util.ConsoleExperimentalApi
 import net.mamoe.mirai.contact.*
+import net.mamoe.mirai.message.data.MessageContent
 import kotlin.internal.LowPriorityInOverloadResolution
 import kotlin.reflect.KClass
 import kotlin.reflect.full.isSubclassOf
@@ -84,6 +85,8 @@ public interface CommandArgumentContext {
 
         PermissionId::class with PermissionIdArgumentParser
         PermitteeId::class with PermitteeIdArgumentParser
+
+        MessageContent::class with RawContentArgumentParser
     })
 }
 

backend/mirai-console/src/main/kotlin/net/mamoe/mirai/console/command/description/CommandArgumentParserBuiltins.kt

@@ -19,10 +19,7 @@ import net.mamoe.mirai.console.permission.PermitteeId
 import net.mamoe.mirai.contact.*
 import net.mamoe.mirai.getFriendOrNull
 import net.mamoe.mirai.getGroupOrNull
-import net.mamoe.mirai.message.data.At
-import net.mamoe.mirai.message.data.MessageContent
-import net.mamoe.mirai.message.data.SingleMessage
-import net.mamoe.mirai.message.data.content
+import net.mamoe.mirai.message.data.*
 
 
 /**
@@ -86,9 +83,9 @@ public object StringArgumentParser : InternalCommandArgumentParserExtensions<Str
 public object BooleanArgumentParser : InternalCommandArgumentParserExtensions<Boolean> {
     public override fun parse(raw: String, sender: CommandSender): Boolean = raw.trim().let { str ->
         str.equals("true", ignoreCase = true)
-                || str.equals("yes", ignoreCase = true)
-                || str.equals("enabled", ignoreCase = true)
-                || str.equals("on", ignoreCase = true)
+            || str.equals("yes", ignoreCase = true)
+            || str.equals("enabled", ignoreCase = true)
+            || str.equals("on", ignoreCase = true)
     }
 }
 
@@ -331,6 +328,12 @@ public object PermitteeIdArgumentParser : CommandArgumentParser<PermitteeId> {
     }
 }
 
+/** 直接返回原始参数 [MessageContent] */
+public object RawContentArgumentParser : CommandArgumentParser<MessageContent> {
+    override fun parse(raw: String, sender: CommandSender): MessageContent = PlainText(raw)
+    override fun parse(raw: MessageContent, sender: CommandSender): MessageContent = raw
+}
+
 internal interface InternalCommandArgumentParserExtensions<T : Any> : CommandArgumentParser<T> {
     fun String.parseToLongOrFail(): Long = toLongOrNull() ?: illegalArgument("无法解析 $this 为整数")
 
@@ -365,10 +368,10 @@ internal interface InternalCommandArgumentParserExtensions<T : Any> : CommandArg
         } else {
             var index = 1
             illegalArgument("无法找到成员 $idOrCard。 多个成员满足搜索结果或匹配度不足: \n\n" +
-                    candidates.joinToString("\n", limit = 6) {
-                        val percentage = (it.second * 100).toDecimalPlace(0)
-                        "#${index++}(${percentage}%)${it.first.nameCardOrNick.truncate(10)}(${it.first.id})" // #1 15.4%
-                    }
+                candidates.joinToString("\n", limit = 6) {
+                    val percentage = (it.second * 100).toDecimalPlace(0)
+                    "#${index++}(${percentage}%)${it.first.nameCardOrNick.truncate(10)}(${it.first.id})" // #1 15.4%
+                }
             )
         }
     }

backend/mirai-console/src/main/kotlin/net/mamoe/mirai/console/internal/command/CompositeCommand.CommandParam.kt

@@ -14,20 +14,25 @@ package net.mamoe.mirai.console.internal.command
 import net.mamoe.mirai.console.command.CompositeCommand
 import net.mamoe.mirai.console.command.description.CommandArgumentParser
 import java.lang.reflect.Parameter
+import kotlin.contracts.ExperimentalContracts
+import kotlin.contracts.contract
 import kotlin.reflect.KClass
+import kotlin.reflect.KParameter
 
+/*
 internal fun Parameter.toCommandParam(): CommandParameter<*> {
     val name = getAnnotation(CompositeCommand.Name::class.java)
     return CommandParameter(
         name?.value ?: this.name
         ?: throw IllegalArgumentException("Cannot construct CommandParam from a unnamed param"),
-        this.type.kotlin
+        this.type.kotlin,
+        null
     )
 }
+*/
 
 /**
  * 指令形式参数.
- * @see toCommandParam
  */
 internal data class CommandParameter<T : Any>(
     /**
@@ -37,9 +42,12 @@ internal data class CommandParameter<T : Any>(
     /**
      * 参数类型. 将从 [CompositeCommand.context] 中寻找 [CommandArgumentParser] 解析.
      */
-    val type: KClass<T> // exact type
+    val type: KClass<T>, // exact type
+    val parameter: KParameter, // source parameter
 ) {
-    constructor(name: String, type: KClass<T>, parser: CommandArgumentParser<T>) : this(name, type) {
+    constructor(name: String, type: KClass<T>, parameter: KParameter, parser: CommandArgumentParser<T>) : this(
+        name, type, parameter
+    ) {
         this._overrideParser = parser
     }
 

backend/mirai-console/src/main/kotlin/net/mamoe/mirai/console/internal/command/CompositeCommandInternal.kt

@@ -21,7 +21,8 @@ import net.mamoe.mirai.message.data.*
 import kotlin.reflect.KAnnotatedElement
 import kotlin.reflect.KClass
 import kotlin.reflect.KFunction
-import kotlin.reflect.full.callSuspend
+import kotlin.reflect.KParameter
+import kotlin.reflect.full.callSuspendBy
 import kotlin.reflect.full.declaredFunctions
 import kotlin.reflect.full.findAnnotation
 import kotlin.reflect.full.isSubclassOf
@@ -131,8 +132,9 @@ internal abstract class AbstractReflectionCommand
         val params: Array<CommandParameter<*>>,
         val description: String,
         val permission: Permission,
-        val onCommand: suspend (sender: CommandSender, parsedArgs: Array<out Any>) -> Boolean,
+        val onCommand: suspend (sender: CommandSender, parsedArgs: Map<KParameter, Any?>) -> Boolean,
         val context: CommandArgumentContext,
+        val argumentBuilder: (sender: CommandSender) -> MutableMap<KParameter, Any?>,
     ) {
         val usage: String = createUsage(this@AbstractReflectionCommand)
 
@@ -151,21 +153,40 @@ internal abstract class AbstractReflectionCommand
             }
         }
 
+        private fun KParameter.isOptional(): Boolean {
+            return isOptional || this.type.isMarkedNullable
+        }
+
+        val minimalArgumentsSize = params.count {
+            !it.parameter.isOptional()
+        }
+
         @JvmField
         internal val bakedSubNames: Array<Array<String>> = names.map { it.bakeSubName() }.toTypedArray()
-        private fun parseArgs(sender: CommandSender, rawArgs: MessageChain, offset: Int): Array<out Any>? {
-            if (rawArgs.size < offset + this.params.size)
+        private fun parseArgs(sender: CommandSender, rawArgs: MessageChain, offset: Int): MutableMap<KParameter, Any?>? {
+            if (rawArgs.size < offset + minimalArgumentsSize)
                 return null
             //require(rawArgs.size >= offset + this.params.size) { "No enough args. Required ${params.size}, but given ${rawArgs.size - offset}" }
-
-            return Array(this.params.size) { index ->
-                val param = params[index]
-                val rawArg = rawArgs[offset + index]
-                when (rawArg) {
-                    is PlainText -> context[param.type]?.parse(rawArg.content, sender)
-                    is MessageContent -> context[param.type]?.parse(rawArg, sender)
-                    else -> throw IllegalArgumentException("Illegal Message kind: ${rawArg.kClassQualifiedNameOrTip}")
-                } ?: error("Cannot find a parser for $rawArg")
+            return argumentBuilder(sender).also { result ->
+                params.forEachIndexed { index, parameter ->
+                    val rawArg = rawArgs.getOrNull(offset + index)
+                    result[parameter.parameter] = when (rawArg) {
+                        null -> {
+                            val p = parameter.parameter
+                            when {
+                                p.isOptional -> return@forEachIndexed
+                                p.type.isMarkedNullable -> {
+                                    result[parameter.parameter] = null
+                                    return@forEachIndexed
+                                }
+                                else -> null
+                            }
+                        }
+                        is PlainText -> context[parameter.type]?.parse(rawArg.content, sender)
+                        is MessageContent -> context[parameter.type]?.parse(rawArg, sender)
+                        else -> throw IllegalArgumentException("Illegal Message kind: ${rawArg.kClassQualifiedNameOrTip}")
+                    } ?: error("Cannot find a parser for $rawArg")
+                }
             }
         }
     }
@@ -250,6 +271,10 @@ internal fun AbstractReflectionCommand.SubCommandDescriptor.createUsage(baseComm
         appendLine()
     }.trimEnd()
 
+internal fun <T1, R1, R2> ((T1) -> R1).then(then: (T1, R1) -> R2): ((T1) -> R2) {
+    return { a -> then.invoke(a, (this@then(a))) }
+}
+
 internal fun AbstractReflectionCommand.createSubCommand(
     function: KFunction<*>,
     context: CommandArgumentContext,
@@ -273,12 +298,17 @@ internal fun AbstractReflectionCommand.createSubCommand(
     check(!function.returnType.isMarkedNullable) {
         error("Return type of sub command ${function.name} must not be marked nullable in Kotlin, and must be marked with @NotNull or @NonNull explicitly in Java. (at ${this::class.qualifiedNameOrTip}.${function.name})")
     }
-
+    var argumentBuilder: (sender: CommandSender) -> MutableMap<KParameter, Any?> = { HashMap() }
     val parameters = function.parameters.toMutableList()
 
-    if (notStatic) parameters.removeAt(0) // instance
+    if (notStatic) {
+        val type = parameters.removeAt(0) // instance
+        argumentBuilder = argumentBuilder.then { _, map ->
+            map[type] = this@createSubCommand
+            map
+        }
+    }
 
-    var hasSenderParam = false
     check(parameters.isNotEmpty()) {
         "Parameters of sub command ${function.name} must not be empty. (Must have CommandSender as its receiver or first parameter or absent, followed by naturally typed params) (at ${this::class.qualifiedNameOrTip}.${function.name})"
     }
@@ -291,8 +321,11 @@ internal fun AbstractReflectionCommand.createSubCommand(
 
     (parameters.first()).let { receiver ->
         if ((receiver.type.classifier as? KClass<*>)?.isSubclassOf(CommandSender::class) == true) {
-            hasSenderParam = true
-            parameters.removeAt(0)
+            val senderType = parameters.removeAt(0)
+            argumentBuilder = argumentBuilder.then { sender, map ->
+                map[senderType] = sender
+                map
+            }
         }
     }
 
@@ -313,37 +346,32 @@ internal fun AbstractReflectionCommand.createSubCommand(
     //map parameter
     val params = parameters.map { param ->
 
-        if (param.isOptional) error("optional parameters are not yet supported. (at ${this::class.qualifiedNameOrTip}.${function.name}.$param)")
+        // if (param.isOptional) error("optional parameters are not yet supported. (at ${this::class.qualifiedNameOrTip}.${function.name}.$param)")
 
         val paramName = param.findAnnotation<CompositeCommand.Name>()?.value ?: param.name ?: "unknown"
         CommandParameter(
             paramName,
             (param.type.classifier as? KClass<*>)
-                ?: throw IllegalArgumentException("unsolved type reference from param " + param.name + ". (at ${this::class.qualifiedNameOrTip}.${function.name}.$param)")
+                ?: throw IllegalArgumentException("unsolved type reference from param " + param.name + ". (at ${this::class.qualifiedNameOrTip}.${function.name}.$param)"),
+            param
         )
     }.toTypedArray()
 
+    // TODO: 2020/09/19 检查 optional/nullable 是否都在最后
+
     return SubCommandDescriptor(
         commandName,
         params,
         subDescription, // overridePermission?.value
         permission,//overridePermission?.value?.let { PermissionService.INSTANCE[PermissionId.parseFromString(it)] } ?: permission,
-        onCommand = { sender: CommandSender, args: Array<out Any> ->
-            val result = if (notStatic) {
-                if (hasSenderParam) {
-                    function.isSuspend
-                    function.callSuspend(this, sender, *args)
-                } else function.callSuspend(this, *args)
-            } else {
-                if (hasSenderParam) {
-                    function.callSuspend(sender, *args)
-                } else function.callSuspend(*args)
-            }
+        onCommand = { sender: CommandSender, args: Map<KParameter, Any?> ->
+            val result = function.callSuspendBy(args)
 
             checkNotNull(result) { "sub command return value is null (at ${this::class.qualifiedName}.${function.name})" }
 
             result as? Boolean ?: true // Unit, void is considered as true.
         },
-        context = context
+        context = context,
+        argumentBuilder = argumentBuilder
     )
 }

backend/mirai-console/src/test/kotlin/net/mamoe/mirai/console/command/TestCommand.kt

@@ -238,8 +238,34 @@ internal class TestCommand {
             }
         }
     }
+
+    @Test
+    fun `test optional argument command`() {
+        runBlocking {
+            val optionCommand = object : CompositeCommand(
+                ConsoleCommandOwner,
+                "testOptional"
+            ) {
+                @SubCommand
+                fun optional(arg1: String, arg2: String = "Here is optional", arg3: String?) {
+                    println(arg1)
+                    println(arg2)
+                    println(arg3)
+//                    println(arg3)
+                    Testing.ok(Unit)
+                }
+            }
+            optionCommand.withRegistration {
+                withTesting<Unit> {
+                    assertSuccess(sender.executeCommand("/testOptional optional 1"))
+                }
+            }
+        }
+    }
 }
 
 internal fun assertSuccess(result: CommandExecuteResult) {
-    assertTrue(result.isSuccess(), result.toString())
+    if (result.isFailure()) {
+        throw result.exception ?: AssertionError(result.toString())
+    }
 }