Fix failed result index mapping

kordlib · Sep 6, 2020 · 888f678 · 888f678
1 parent 2f791b0
commit 888f678
Show file tree

Hide file tree

Showing 12 changed files with 192 additions and 12 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,3 +1,10 @@
+# 0.3.2
+
+## Fixes
+
+* Fixed an issue where mapping failed results would reset the char index to zero, resulting in a `Too Many Arguments`
+error
+
 # 0.3.1
 
 ## Fixes

diff --git a/...s-runtime/src/main/kotlin/com/gitlab/kordlib/kordx/commands/argument/extension/Default.kt b/...s-runtime/src/main/kotlin/com/gitlab/kordlib/kordx/commands/argument/extension/Default.kt
@@ -14,7 +14,7 @@ fun <T : Any, CONTEXT> Argument<T, CONTEXT>.withDefault(
         default: T
 ): Argument<T, CONTEXT> = object : Argument<T, CONTEXT> by this {
     override suspend fun parse(text: String, fromIndex: Int, context: CONTEXT): ArgumentResult<T> {
-        return this@withDefault.parse(text, fromIndex, context).orElse(default)
+        return this@withDefault.parse(text, fromIndex, context).orElse(fromIndex, default)
     }
 }
 
@@ -27,7 +27,7 @@ fun <T : Any, CONTEXT> Argument<T, CONTEXT>.withDefault(
 ): Argument<T, CONTEXT> = object : Argument<T, CONTEXT> by this {
 
     override suspend fun parse(text: String, fromIndex: Int, context: CONTEXT): ArgumentResult<T> {
-        return this@withDefault.parse(text, fromIndex, context).orElseSupply { default(context) }
+        return this@withDefault.parse(text, fromIndex, context).orElseSupply(fromIndex) { default(context) }
     }
 }
 
@@ -57,6 +57,6 @@ fun <T : Any, CONTEXT> Argument<T?, CONTEXT>.withDefault(
 ): Argument<T, CONTEXT> = object : Argument<T, CONTEXT> by this as Argument<T, CONTEXT> {
 
     override suspend fun parse(text: String, fromIndex: Int, context: CONTEXT): ArgumentResult<T> {
-        return this@withDefault.parse(text, fromIndex, context).orElseSupply { fallback(context) }
+        return this@withDefault.parse(text, fromIndex, context).orElseSupply(fromIndex) { fallback(context) }
     }
 }
diff --git a/...ds-runtime/src/main/kotlin/com/gitlab/kordlib/kordx/commands/argument/extension/Filter.kt b/...ds-runtime/src/main/kotlin/com/gitlab/kordlib/kordx/commands/argument/extension/Filter.kt
@@ -13,7 +13,7 @@ fun <T, CONTEXT> Argument<T, CONTEXT>.filter(
 ): Argument<T, CONTEXT> = object : Argument<T, CONTEXT> by this {
 
     override suspend fun parse(text: String, fromIndex: Int, context: CONTEXT): ArgumentResult<T> {
-        return this@filter.parse(text, fromIndex, context).filter { filter(context, it) }
+        return this@filter.parse(text, fromIndex, context).filter(fromIndex) { filter(context, it) }
     }
 }
 
@@ -29,7 +29,7 @@ inline fun <reified T, CONTEXT> Argument<*, CONTEXT>.filterIsInstance(
         get() = this@filterIsInstance.name
 
     override suspend fun parse(text: String, fromIndex: Int, context: CONTEXT): ArgumentResult<T> {
-        return this@filterIsInstance.parse(text, fromIndex, context).filterIsInstance(failMessage)
+        return this@filterIsInstance.parse(text, fromIndex, context).filterIsInstance(fromIndex, failMessage)
     }
 }
 

diff --git a/...mands-runtime/src/main/kotlin/com/gitlab/kordlib/kordx/commands/argument/extension/Map.kt b/...mands-runtime/src/main/kotlin/com/gitlab/kordlib/kordx/commands/argument/extension/Map.kt
@@ -29,6 +29,6 @@ fun <T, R : Any, CONTEXT> Argument<T, CONTEXT>.tryMap(
 ): Argument<R, CONTEXT> = object : Argument<R, CONTEXT> by this as Argument<R, CONTEXT> {
 
     override suspend fun parse(text: String, fromIndex: Int, context: CONTEXT): ArgumentResult<R> {
-        return this@tryMap.parse(text, fromIndex, context).tryMap { mapper.invoke(context, it) }
+        return this@tryMap.parse(text, fromIndex, context).tryMap(fromIndex) { mapper.invoke(context, it) }
     }
 }
diff --git a/...ds-runtime/src/main/kotlin/com/gitlab/kordlib/kordx/commands/argument/extension/Number.kt b/...ds-runtime/src/main/kotlin/com/gitlab/kordlib/kordx/commands/argument/extension/Number.kt
@@ -17,7 +17,7 @@ fun <T, CONTEXT> Argument<T, CONTEXT>.inRange(
 ): Argument<T, CONTEXT> where T : Number, T : Comparable<T> = object : Argument<T, CONTEXT> by this {
 
     override suspend fun parse(text: String, fromIndex: Int, context: CONTEXT): ArgumentResult<T> {
-        return this@inRange.parse(text, fromIndex, context).filter {
+        return this@inRange.parse(text, fromIndex, context).filter(fromIndex) {
             if (it in range) FilterResult.Pass
             else FilterResult.Fail("expected number in range of [${range.start}..${range.endInclusive}]")
         }

diff --git a/...-runtime/src/main/kotlin/com/gitlab/kordlib/kordx/commands/argument/extension/Optional.kt b/...-runtime/src/main/kotlin/com/gitlab/kordlib/kordx/commands/argument/extension/Optional.kt
@@ -12,7 +12,7 @@ import com.gitlab.kordlib.kordx.commands.argument.result.extension.orElse
 fun <T : Any, CONTEXT> Argument<T, CONTEXT>.optional(): Argument<T?, CONTEXT> = object : Argument<T?, CONTEXT> by this {
 
     override suspend fun parse(text: String, fromIndex: Int, context: CONTEXT): ArgumentResult<T?> {
-        return this@optional.parse(text, fromIndex, context).optional()
+        return this@optional.parse(text, fromIndex, context).optional(fromIndex)
     }
 
 }
@@ -25,7 +25,7 @@ fun <T : Any, CONTEXT> Argument<T, CONTEXT>.optional(
 ): Argument<T, CONTEXT> = object : Argument<T, CONTEXT> by this {
 
     override suspend fun parse(text: String, fromIndex: Int, context: CONTEXT): ArgumentResult<T> {
-        return this@optional.parse(text, fromIndex, context).orElse(default)
+        return this@optional.parse(text, fromIndex, context).orElse(fromIndex, default)
     }
 }
 
@@ -37,6 +37,6 @@ fun <T : Any, CONTEXT> Argument<T, CONTEXT>.optional(
 ): Argument<T, CONTEXT> = object : Argument<T, CONTEXT> by this {
 
     override suspend fun parse(text: String, fromIndex: Int, context: CONTEXT): ArgumentResult<T> {
-        return this@optional.parse(text, fromIndex, context).orElse(default(context))
+        return this@optional.parse(text, fromIndex, context).orElse(fromIndex, default(context))
     }
 }
diff --git a/...me/src/main/kotlin/com/gitlab/kordlib/kordx/commands/argument/result/extension/Default.kt b/...me/src/main/kotlin/com/gitlab/kordlib/kordx/commands/argument/result/extension/Default.kt
@@ -22,7 +22,7 @@ fun <T : Any> ArgumentResult<T?>.orDefault(default: T): ArgumentResult.Success<T
         null -> ArgumentResult.Success(default, newIndex)
         else -> this as ArgumentResult.Success<T>
     }
-    is ArgumentResult.Failure -> ArgumentResult.Success(default, 0)
+    is ArgumentResult.Failure -> ArgumentResult.Success(default, atChar)
 }
 
 /**
@@ -31,6 +31,11 @@ fun <T : Any> ArgumentResult<T?>.orDefault(default: T): ArgumentResult.Success<T
  */
 @Suppress("UNCHECKED_CAST")
 @JvmName("orElseSupplyNullable")
+@Deprecated(
+        "The behavior of this function is incorrect, use the variant with `failIndex` instead",
+        ReplaceWith("orElseSupply(failIndex, fallback)"),
+        level = DeprecationLevel.WARNING,
+)
 inline fun <T : Any> ArgumentResult<T?>.orElseSupply(fallback: () -> T): ArgumentResult.Success<T> = when (this) {
     is ArgumentResult.Success -> when (item) {
         null -> ArgumentResult.Success(fallback(), newIndex)
@@ -39,12 +44,35 @@ inline fun <T : Any> ArgumentResult<T?>.orElseSupply(fallback: () -> T): Argumen
     is ArgumentResult.Failure -> ArgumentResult.Success(fallback(), 0)
 }
 
+/**
+ * Returns this if this is a [ArgumentResult.Success] and [ArgumentResult.Success.item] is not null,
+ * or a [ArgumentResult.Success] with the [fallback] as item otherwise.
+ */
+@Suppress("UNCHECKED_CAST")
+@JvmName("orElseSupplyNullable")
+inline fun <T : Any> ArgumentResult<T?>.orElseSupply(
+        failIndex: Int,
+        fallback: () -> T
+): ArgumentResult.Success<T> = when (this) {
+    is ArgumentResult.Success -> when (item) {
+        null -> ArgumentResult.Success(fallback(), newIndex)
+        else -> this as ArgumentResult.Success<T>
+    }
+    is ArgumentResult.Failure -> ArgumentResult.Success(fallback(), failIndex)
+}
+
+
 
 /**
  * Returns this if this is a [ArgumentResult.Success],
  * or a [ArgumentResult.Success] with the [fallBack] as item otherwise.
  */
 @Suppress("UNCHECKED_CAST")
+@Deprecated(
+        "The behavior of this function is incorrect, use the variant with `failIndex` instead",
+        ReplaceWith("orElse(failIndex, fallback)"),
+        level = DeprecationLevel.WARNING,
+)
 fun <T> ArgumentResult<T>.orElse(fallBack: T): ArgumentResult.Success<T> = when (this) {
     is ArgumentResult.Success -> ArgumentResult.Success(item, newIndex)
     is ArgumentResult.Failure -> ArgumentResult.Success(fallBack, 0)
@@ -55,7 +83,35 @@ fun <T> ArgumentResult<T>.orElse(fallBack: T): ArgumentResult.Success<T> = when
  * or a [ArgumentResult.Success] with the [fallBack] as item otherwise.
  */
 @Suppress("UNCHECKED_CAST")
+fun <T> ArgumentResult<T>.orElse(failIndex: Int, fallBack: T): ArgumentResult.Success<T> = when (this) {
+    is ArgumentResult.Success -> ArgumentResult.Success(item, newIndex)
+    is ArgumentResult.Failure -> ArgumentResult.Success(fallBack, failIndex)
+}
+
+/**
+ * Returns this if this is a [ArgumentResult.Success],
+ * or a [ArgumentResult.Success] with the [fallBack] as item otherwise.
+ */
+@Suppress("UNCHECKED_CAST")
+@Deprecated(
+        "The behavior of this function is incorrect, use the variant with `failIndex` instead",
+        ReplaceWith("orElseSupply(failIndex, fallback)"),
+        level = DeprecationLevel.WARNING,
+)
 inline fun <T> ArgumentResult<T>.orElseSupply(fallBack: () -> T): ArgumentResult.Success<T> = when (this) {
     is ArgumentResult.Success -> ArgumentResult.Success(item, newIndex)
     is ArgumentResult.Failure -> ArgumentResult.Success(fallBack(), 0)
 }
+
+/**
+ * Returns this if this is a [ArgumentResult.Success],
+ * or a [ArgumentResult.Success] with the [fallBack] as item otherwise.
+ */
+@Suppress("UNCHECKED_CAST")
+inline fun <T> ArgumentResult<T>.orElseSupply(
+        failIndex: Int,
+        fallBack: () -> T
+): ArgumentResult.Success<T> = when (this) {
+    is ArgumentResult.Success -> ArgumentResult.Success(item, newIndex)
+    is ArgumentResult.Failure -> ArgumentResult.Success(fallBack(), failIndex)
+}
diff --git a/...ime/src/main/kotlin/com/gitlab/kordlib/kordx/commands/argument/result/extension/Filter.kt b/...ime/src/main/kotlin/com/gitlab/kordlib/kordx/commands/argument/result/extension/Filter.kt
@@ -11,6 +11,11 @@ import com.gitlab.kordlib.kordx.commands.argument.result.ArgumentResult
  * @param [failMessage] the message to supply when the [ArgumentResult.Success.item] is not [T].
  */
 @Suppress("UNCHECKED_CAST")
+@Deprecated(
+        "The behavior of this function is incorrect, use the variant with `failIndex` instead",
+        ReplaceWith("filterIsInstance(failIndex, failMessage)"),
+        level = DeprecationLevel.WARNING,
+)
 inline fun <reified T> ArgumentResult<*>.filterIsInstance(
         failMessage: String
 ): ArgumentResult<T> = when (val result = this) {
@@ -29,6 +34,30 @@ inline fun <reified T> ArgumentResult<*>.filterIsInstance(
  * @param [failMessage] the message to supply when the [ArgumentResult.Success.item] is not [T].
  */
 @Suppress("UNCHECKED_CAST")
+inline fun <reified T> ArgumentResult<*>.filterIsInstance(
+        failIndex: Int,
+        failMessage: String
+): ArgumentResult<T> = when (val result = this) {
+    is ArgumentResult.Success -> when (result.item) {
+        is T -> result as ArgumentResult<T>
+        else -> ArgumentResult.Failure(failMessage, failIndex)
+    }
+    is ArgumentResult.Failure -> this as ArgumentResult<T>
+}
+
+/**
+ * Returns a [ArgumentResult.Success] if
+ * this result is a [ArgumentResult.Success] *and* the [ArgumentResult.Success.item] is [T],
+ * or a [ArgumentResult.Failure] otherwise.
+ *
+ * @param [failMessage] the message to supply when the [ArgumentResult.Success.item] is not [T].
+ */
+@Suppress("UNCHECKED_CAST")
+@Deprecated(
+        "The behavior of this function is incorrect, use the variant with `failIndex` instead",
+        ReplaceWith("filterIsInstance(failIndex, failMessage)"),
+        level = DeprecationLevel.WARNING,
+)
 inline fun <reified T> ArgumentResult<*>.filterIsInstance(
         failMessage: () -> String
 ): ArgumentResult<T> = when (val result = this) {
@@ -39,6 +68,30 @@ inline fun <reified T> ArgumentResult<*>.filterIsInstance(
     is ArgumentResult.Failure -> this as ArgumentResult<T>
 }
 
+/**
+ * Returns a [ArgumentResult.Success] if
+ * this result is a [ArgumentResult.Success] *and* the [ArgumentResult.Success.item] is [T],
+ * or a [ArgumentResult.Failure] otherwise.
+ *
+ * @param [failMessage] the message to supply when the [ArgumentResult.Success.item] is not [T].
+ */
+@Suppress("UNCHECKED_CAST")
+@Deprecated(
+        "The behavior of this function is incorrect, use the variant with `failIndex` instead",
+        ReplaceWith("filterIsInstance(failIndex, failMessage)"),
+        level = DeprecationLevel.WARNING,
+)
+inline fun <reified T> ArgumentResult<*>.filterIsInstance(
+        failIndex: Int,
+        failMessage: () -> String
+): ArgumentResult<T> = when (val result = this) {
+    is ArgumentResult.Success -> when (result.item) {
+        is T -> result as ArgumentResult<T>
+        else -> ArgumentResult.Failure(failMessage(), failIndex)
+    }
+    is ArgumentResult.Failure -> this as ArgumentResult<T>
+}
+
 /**
  * Returns a [ArgumentResult.Success] if
  * this result is a [ArgumentResult.Success] *and* the [ArgumentResult.Success.item] is not null,
@@ -79,6 +132,11 @@ inline fun <T : Any> ArgumentResult<T?>.filterNotNull(failMessage: (T?) -> Strin
  * or a [ArgumentResult.Failure] otherwise.
  */
 @Suppress("UNCHECKED_CAST")
+@Deprecated(
+        "The behavior of this function is incorrect, use the variant with `failIndex` instead",
+        ReplaceWith("filter(failIndex, filter)"),
+        level = DeprecationLevel.WARNING,
+)
 inline fun <T> ArgumentResult<T>.filter(filter: (T) -> FilterResult): ArgumentResult<T> = when (this) {
     is ArgumentResult.Success -> when (val result = filter(item)) {
         FilterResult.Pass -> this
@@ -87,6 +145,20 @@ inline fun <T> ArgumentResult<T>.filter(filter: (T) -> FilterResult): ArgumentRe
     is ArgumentResult.Failure -> this
 }
 
+/**
+ * Returns a [ArgumentResult.Success] if
+ * this result is a [ArgumentResult.Success] *and* the result of [filter] is a [FilterResult.Pass],
+ * or a [ArgumentResult.Failure] otherwise.
+ */
+@Suppress("UNCHECKED_CAST")
+inline fun <T> ArgumentResult<T>.filter(failIndex: Int, filter: (T) -> FilterResult): ArgumentResult<T> = when (this) {
+    is ArgumentResult.Success -> when (val result = filter(item)) {
+        FilterResult.Pass -> this
+        is FilterResult.Fail -> ArgumentResult.Failure(result.reason, failIndex)
+    }
+    is ArgumentResult.Failure -> this
+}
+
 /**
  * The result of a filter action, a [FilterResult] can either [Pass] or [Fail].
  */

diff --git a/...untime/src/main/kotlin/com/gitlab/kordlib/kordx/commands/argument/result/extension/Map.kt b/...untime/src/main/kotlin/com/gitlab/kordlib/kordx/commands/argument/result/extension/Map.kt
@@ -19,6 +19,11 @@ inline fun <T, R> ArgumentResult<T>.map(mapper: (T) -> R): ArgumentResult<R> = w
  * or a [ArgumentResult.Failure] otherwise.
  */
 @Suppress("UNCHECKED_CAST")
+@Deprecated(
+        "The behavior of this function is incorrect, use the variant with `failIndex` instead",
+        ReplaceWith("tryMap(failIndex, mapper)"),
+        level = DeprecationLevel.WARNING,
+)
 inline fun <T, R : Any> ArgumentResult<T>.tryMap(mapper: (T) -> MapResult<R>): ArgumentResult<R> = when (this) {
     is ArgumentResult.Success -> when (val result = mapper(item)) {
         is MapResult.Pass -> ArgumentResult.Success(result.item, newIndex)
@@ -27,6 +32,23 @@ inline fun <T, R : Any> ArgumentResult<T>.tryMap(mapper: (T) -> MapResult<R>): A
     is ArgumentResult.Failure -> this as ArgumentResult<R>
 }
 
+/**
+ * Returns a [ArgumentResult.Success] if
+ * this result is a [ArgumentResult.Success] *and* the result of [mapper] is a [MapResult.Pass],
+ * or a [ArgumentResult.Failure] otherwise.
+ */
+@Suppress("UNCHECKED_CAST")
+inline fun <T, R : Any> ArgumentResult<T>.tryMap(
+        failIndex: Int,
+        mapper: (T) -> MapResult<R>
+): ArgumentResult<R> = when (this) {
+    is ArgumentResult.Success -> when (val result = mapper(item)) {
+        is MapResult.Pass -> ArgumentResult.Success(result.item, newIndex)
+        is MapResult.Fail -> ArgumentResult.Failure(result.reason, failIndex)
+    }
+    is ArgumentResult.Failure -> this as ArgumentResult<R>
+}
+
 /**
  * The result of a mapping action, a [MapResult] can either [Pass] or [Fail].
  */

diff --git a/...e/src/main/kotlin/com/gitlab/kordlib/kordx/commands/argument/result/extension/Optional.kt b/...e/src/main/kotlin/com/gitlab/kordlib/kordx/commands/argument/result/extension/Optional.kt
@@ -5,8 +5,22 @@ import com.gitlab.kordlib.kordx.commands.argument.result.ArgumentResult
 /**
  * Returns this if this is a [ArgumentResult.Success], or a [ArgumentResult.Success] with null as item otherwise.
  */
+@Deprecated(
+        "The behavior of this function is incorrect, use the variant with `failIndex` instead",
+        ReplaceWith("optional(failIndex)"),
+        level = DeprecationLevel.WARNING,
+)
 @Suppress("UNCHECKED_CAST")
 fun <T : Any> ArgumentResult<T>.optional(): ArgumentResult.Success<T?> = when (this) {
     is ArgumentResult.Success -> this as ArgumentResult.Success<T?>
     is ArgumentResult.Failure -> ArgumentResult.Success(null, 0)
 }
+
+/**
+ * Returns this if this is a [ArgumentResult.Success], or a [ArgumentResult.Success] with null as item otherwise.
+ */
+@Suppress("UNCHECKED_CAST")
+fun <T : Any> ArgumentResult<T>.optional(failIndex: Int): ArgumentResult.Success<T?> = when (this) {
+    is ArgumentResult.Success -> this as ArgumentResult.Success<T?>
+    is ArgumentResult.Failure -> ArgumentResult.Success(null, failIndex)
+}
diff --git a/...nds-runtime/src/main/kotlin/com/gitlab/kordlib/kordx/commands/argument/text/Extensions.kt b/...nds-runtime/src/main/kotlin/com/gitlab/kordlib/kordx/commands/argument/text/Extensions.kt
@@ -14,7 +14,7 @@ fun <CONTEXT> Argument<String, CONTEXT>.whitelist(
         vararg whitelist: String, ignoreCase: Boolean = true
 ): Argument<String, CONTEXT> = object : Argument<String, CONTEXT> by this {
     override suspend fun parse(text: String, fromIndex: Int, context: CONTEXT): ArgumentResult<String> {
-        return this@whitelist.parse(text, fromIndex, context).filter {
+        return this@whitelist.parse(text, fromIndex, context).filter(fromIndex) {
             when {
                 ignoreCase -> when {
                     whitelist.any { word -> word.equals(it, true) } -> FilterResult.Pass

diff --git a/...ime/src/test/kotlin/com/gitlab/kordlib/kordx/commands/argument/text/StringArgumentTest.kt b/...ime/src/test/kotlin/com/gitlab/kordlib/kordx/commands/argument/text/StringArgumentTest.kt
@@ -1,11 +1,13 @@
 package com.gitlab.kordlib.kordx.commands.argument.text
 
 import com.gitlab.kordlib.kordx.commands.argument.requireItem
+import com.gitlab.kordlib.kordx.commands.argument.requireSuccess
 import kotlinx.coroutines.ExperimentalCoroutinesApi
 import kotlinx.coroutines.test.runBlockingTest
 import org.junit.jupiter.params.ParameterizedTest
 import org.junit.jupiter.params.provider.Arguments
 import org.junit.jupiter.params.provider.MethodSource
+import kotlin.test.assertEquals
 
 @Suppress("unused")
 @ExperimentalCoroutinesApi
@@ -19,6 +21,13 @@ class StringArgumentTest {
         argument.parse(text, 0, Unit).requireItem(text)
     }
 
+    @ParameterizedTest
+    @MethodSource("sources")
+    fun `correctly consumes all`(text: String) = runBlockingTest {
+        val success = argument.parse(text, 5, Unit).requireSuccess()
+        assertEquals(expected = text.length, actual = success.newIndex)
+    }
+
     companion object {
         @JvmStatic
         fun sources() = listOf(