From b4c830b4cda9351c0f8ef0fc51296b6e21f2968a Mon Sep 17 00:00:00 2001 From: Andreas Volkmann Date: Sun, 18 Apr 2021 15:33:21 +0200 Subject: [PATCH] Refactor cli argument parsing, support single quote assignments --- .../arkenv/feature/cli/CliArgumentParser.kt | 41 +++++++++++++++++++ .../arkenv/feature/cli/CliFeature.kt | 31 ++------------ .../com/apurebase/arkenv/util/StringUtils.kt | 9 +--- .../com/apurebase/arkenv/LookupTests.kt | 4 +- .../arkenv/feature/cli/AssignmentTest.kt | 36 ++++++++++------ .../feature/cli/CliArgumentParserTest.kt | 37 +++++++++++++++++ .../arkenv/feature/cli/CliFeatureTest.kt | 22 ++++++---- .../com/apurebase/arkenv/test/TestUtil.kt | 2 +- 8 files changed, 124 insertions(+), 58 deletions(-) create mode 100644 arkenv/src/main/kotlin/com/apurebase/arkenv/feature/cli/CliArgumentParser.kt create mode 100644 arkenv/src/test/kotlin/com/apurebase/arkenv/feature/cli/CliArgumentParserTest.kt diff --git a/arkenv/src/main/kotlin/com/apurebase/arkenv/feature/cli/CliArgumentParser.kt b/arkenv/src/main/kotlin/com/apurebase/arkenv/feature/cli/CliArgumentParser.kt new file mode 100644 index 0000000..a0283c4 --- /dev/null +++ b/arkenv/src/main/kotlin/com/apurebase/arkenv/feature/cli/CliArgumentParser.kt @@ -0,0 +1,41 @@ +package com.apurebase.arkenv.feature.cli + +/** + * Parses the command line arguments. + */ +internal class CliArgumentParser { + private val allowedSurroundings = listOf("'", "\"") + private val list = mutableListOf() + private var isReading = false + + /** + * Parses the provided [arguments] and returns the accumulated results. + * @param arguments List of raw command line string arguments to parse. + */ + fun parseArguments(arguments: List): List { + arguments.forEach(::parse) + return list + } + + private fun parse(value: String) { + when { + isReading -> list[list.lastIndex] = "${list.last()} $value" + else -> list.add(value) + } + + when { + isReading && value.endsWith(allowedSurroundings) -> { + list[list.lastIndex] = list.last().removeSurrounding(allowedSurroundings) + isReading = false + } + !isReading && value.startsWith(allowedSurroundings) -> isReading = true + } + } + + private fun String.removeSurrounding(list: Iterable): String = + list.fold(this, String::removeSurrounding) + + private fun String.startsWith(list: Iterable): Boolean = list.any(::startsWith) + + private fun String.endsWith(list: Iterable): Boolean = list.any(::endsWith) +} diff --git a/arkenv/src/main/kotlin/com/apurebase/arkenv/feature/cli/CliFeature.kt b/arkenv/src/main/kotlin/com/apurebase/arkenv/feature/cli/CliFeature.kt index c11bcec..e97e757 100644 --- a/arkenv/src/main/kotlin/com/apurebase/arkenv/feature/cli/CliFeature.kt +++ b/arkenv/src/main/kotlin/com/apurebase/arkenv/feature/cli/CliFeature.kt @@ -1,13 +1,11 @@ package com.apurebase.arkenv.feature.cli -import com.apurebase.arkenv.* +import com.apurebase.arkenv.Arkenv import com.apurebase.arkenv.argument.Argument import com.apurebase.arkenv.argument.ArkenvArgument import com.apurebase.arkenv.feature.ArkenvFeature -import com.apurebase.arkenv.util.* -import com.apurebase.arkenv.util.endsWith import com.apurebase.arkenv.util.mapRelaxed -import com.apurebase.arkenv.util.startsWith +import com.apurebase.arkenv.util.putAll import com.apurebase.arkenv.util.toSnakeCase import kotlin.collections.set @@ -22,10 +20,10 @@ class CliFeature : ArkenvFeature { override fun onLoad(arkenv: Arkenv) { args = arkenv.argList args.replaceAll(String::mapRelaxed) - loadCliAssignments().let(arkenv::putAll) - val parsed = parseArguments(args) + val parsed = CliArgumentParser().parseArguments(args) args.clear() args.addAll(parsed) + loadCliAssignments().let(arkenv::putAll) } override fun onParse(arkenv: Arkenv, delegate: ArkenvArgument<*>): String? = @@ -66,25 +64,6 @@ class CliFeature : ArkenvFeature { private fun parseCli(index: Int): String? = args.getOrNull(index + 1) - private fun parseArguments(arguments: List): List { - val list = mutableListOf() - var isReading = false - arguments.forEach { value -> - when { - isReading -> list[list.lastIndex] = "${list.last()} $value" - else -> list.add(value) - } - when { - isReading && value.endsWith(allowedSurroundings) -> { - list[list.lastIndex] = list.last().removeSurrounding(allowedSurroundings) - isReading = false - } - !isReading && value.startsWith(allowedSurroundings) -> isReading = true - } - } - return list - } - private fun findIndex(argument: Argument<*>, arguments: List): Int? = when { argument.isMainArg -> arguments.size - 2 else -> argument @@ -105,6 +84,4 @@ class CliFeature : ArkenvFeature { private fun removeValueArgument(index: Int, isBoolean: Boolean, value: Any?, default: Boolean) { if (!isBoolean && !default && value != null) args.removeAt(index + 1) } - - private val allowedSurroundings = listOf("'", "\"") } diff --git a/arkenv/src/main/kotlin/com/apurebase/arkenv/util/StringUtils.kt b/arkenv/src/main/kotlin/com/apurebase/arkenv/util/StringUtils.kt index a3da34e..387c5a8 100644 --- a/arkenv/src/main/kotlin/com/apurebase/arkenv/util/StringUtils.kt +++ b/arkenv/src/main/kotlin/com/apurebase/arkenv/util/StringUtils.kt @@ -6,20 +6,13 @@ internal fun String.toSnakeCase() = this .toUpperCase() .removePrefixes("_") -internal fun String.removePrefixes(prefix: CharSequence): String = this +private fun String.removePrefixes(prefix: CharSequence): String = this .removePrefix(prefix) .let { if (it.startsWith(prefix)) it.removePrefixes(prefix) else it } -internal fun String.endsWith(list: Iterable): Boolean = list.any(::endsWith) - -internal fun String.startsWith(list: Iterable): Boolean = list.any(::startsWith) - -internal fun String.removeSurrounding(list: Iterable): String = - list.fold(this) { acc, s -> acc.removeSurrounding(s) } - internal fun String.isAdvancedName() = startsWith("--") internal fun String.isSimpleName() = startsWith("-") && !isAdvancedName() diff --git a/arkenv/src/test/kotlin/com/apurebase/arkenv/LookupTests.kt b/arkenv/src/test/kotlin/com/apurebase/arkenv/LookupTests.kt index 86c753a..a9b4553 100644 --- a/arkenv/src/test/kotlin/com/apurebase/arkenv/LookupTests.kt +++ b/arkenv/src/test/kotlin/com/apurebase/arkenv/LookupTests.kt @@ -48,14 +48,14 @@ class LookupTests { val ark = Ark().parse("--other", "name") val key = "left-over" assertThrows { ark[key] } - .expectThat { get { message }.isNotNull().contains(key) } + .expectThat { get { message }.isNotNull() contains key } } @Test fun `input key should be snake-case formatted`() { val expected = "app" MockSystem("CLIENT_DIR" to expected) Ark().parse().expectThat { - get { get("clientDir") }.isEqualTo(expected) + get { get("clientDir") } isEqualTo expected } } diff --git a/arkenv/src/test/kotlin/com/apurebase/arkenv/feature/cli/AssignmentTest.kt b/arkenv/src/test/kotlin/com/apurebase/arkenv/feature/cli/AssignmentTest.kt index 4ae84c6..7bf0768 100644 --- a/arkenv/src/test/kotlin/com/apurebase/arkenv/feature/cli/AssignmentTest.kt +++ b/arkenv/src/test/kotlin/com/apurebase/arkenv/feature/cli/AssignmentTest.kt @@ -1,12 +1,17 @@ package com.apurebase.arkenv.feature.cli import com.apurebase.arkenv.Arkenv -import com.apurebase.arkenv.util.argument import com.apurebase.arkenv.test.expectThat import com.apurebase.arkenv.test.parse +import com.apurebase.arkenv.util.argument import org.junit.jupiter.api.Test +import strikt.api.expectThat import strikt.assertions.isEqualTo +/** + * Verify that command-line assignment style arguments (key=value) are parsed correctly. + * @see CliFeature + */ class AssignmentTest { private class Ark : Arkenv() { @@ -18,29 +23,36 @@ class AssignmentTest { } @Test fun `should parse assignment correctly`() { - Ark().parse("int=4").expectThat { - get { int }.isEqualTo(4) - get { bool }.isEqualTo(true) + Ark().parse("int=4") expectThat { + get { int } isEqualTo 4 + get { bool } isEqualTo true } } @Test fun `should turn bool off`() { - Ark().parse("int=-1", "bool=false").expectThat { - get { int }.isEqualTo(-1) - get { bool }.isEqualTo(false) + Ark().parse("int=-1", "bool=false") expectThat { + get { int } isEqualTo -1 + get { bool } isEqualTo false } } @Test fun `should be able to use complex arg in assignment`() { - Ark().parse("int=0", "complex-arg=false").expectThat { - get { bool }.isEqualTo(false) + Ark().parse("int=0", "complex-arg=false") expectThat { + get { bool } isEqualTo false } } @Test fun `should still allow = as part of other args`() { - Ark().parse("--str", "key=value", "int=1").expectThat { - get { string }.isEqualTo("key=value") - get { int }.isEqualTo(1) + Ark().parse("--str", "key=value", "int=1") expectThat { + get { string } isEqualTo "key=value" + get { int } isEqualTo 1 + } + } + + @Test fun `single quoted assignment`() { + val ark = Ark().parse("int=0", "'str=test", "expected'") + expectThat(ark) { + get { string } isEqualTo "test expected" } } } diff --git a/arkenv/src/test/kotlin/com/apurebase/arkenv/feature/cli/CliArgumentParserTest.kt b/arkenv/src/test/kotlin/com/apurebase/arkenv/feature/cli/CliArgumentParserTest.kt new file mode 100644 index 0000000..9dce9d5 --- /dev/null +++ b/arkenv/src/test/kotlin/com/apurebase/arkenv/feature/cli/CliArgumentParserTest.kt @@ -0,0 +1,37 @@ +package com.apurebase.arkenv.feature.cli + +import org.junit.jupiter.api.DynamicTest +import org.junit.jupiter.api.TestFactory +import strikt.api.expectThat +import strikt.assertions.get +import strikt.assertions.isEqualTo + +/** + * Verify the behavior of the [CliArgumentParser]. + */ +internal class CliArgumentParserTest { + + private data class TestCase(val name: String, val input: List, val expected: String) + + private val testCases = listOf( + TestCase("no quotes", listOf("hello", "world"), "hello"), + TestCase("double quotes", listOf("\"hello", "world\""), "hello world"), + TestCase("single quote inside value", listOf("D'vloper"), "D'vloper"), + TestCase("single quoted", listOf("'test", "expected'"), "test expected"), + TestCase("single quoted, containing singlq quote", listOf("'test", "ex'pected'"), "test ex'pected"), + ) + + @TestFactory + fun parseArguments(): List = testCases.map { (name, input, expected) -> + DynamicTest.dynamicTest(name) { + // Arrange + val parser = CliArgumentParser() + + // Act + val actual = parser.parseArguments(input) + + // Assert + expectThat(actual)[0] isEqualTo expected + } + } +} \ No newline at end of file diff --git a/arkenv/src/test/kotlin/com/apurebase/arkenv/feature/cli/CliFeatureTest.kt b/arkenv/src/test/kotlin/com/apurebase/arkenv/feature/cli/CliFeatureTest.kt index 07fc99d..f9d839e 100644 --- a/arkenv/src/test/kotlin/com/apurebase/arkenv/feature/cli/CliFeatureTest.kt +++ b/arkenv/src/test/kotlin/com/apurebase/arkenv/feature/cli/CliFeatureTest.kt @@ -1,28 +1,34 @@ package com.apurebase.arkenv.feature.cli -import com.apurebase.arkenv.util.get import com.apurebase.arkenv.test.Nullable -import com.apurebase.arkenv.test.expectThat import com.apurebase.arkenv.test.parse +import com.apurebase.arkenv.util.get import org.junit.jupiter.api.Test +import strikt.api.expectThat import strikt.assertions.isEqualTo +/** + * Verify the behavior of the [CliFeature]. + */ internal class CliFeatureTest { @Test fun `resolve undeclared get calls`() { val key = "--undeclared" val expected = "expected" - Nullable().parse(key, expected).expectThat { - get { get(key) }.isEqualTo(expected) - } + val configuration = Nullable().parse(key, expected) + expectThat(configuration).get { get(key) } isEqualTo expected } @Test fun `resolve undeclared assignments`() { val key = "undeclared" val expected = "expected" - Nullable().parse("$key=$expected").expectThat { - get { get(key) }.isEqualTo(expected) - } + val configuration = Nullable().parse("$key=$expected") + expectThat(configuration).get { get(key) } isEqualTo expected } + @Test fun `single quote in value should parse entire string`() { + val expected = "D'vloper" + val configuration = Nullable().parse("-s", expected) + expectThat(configuration).get { str } isEqualTo expected + } } diff --git a/arkenv/src/testFixtures/kotlin/com/apurebase/arkenv/test/TestUtil.kt b/arkenv/src/testFixtures/kotlin/com/apurebase/arkenv/test/TestUtil.kt index 9c73a59..333c5f7 100644 --- a/arkenv/src/testFixtures/kotlin/com/apurebase/arkenv/test/TestUtil.kt +++ b/arkenv/src/testFixtures/kotlin/com/apurebase/arkenv/test/TestUtil.kt @@ -5,7 +5,7 @@ import strikt.api.Assertion import strikt.assertions.isEqualTo import java.io.File -fun T.expectThat(block: Assertion.Builder.() -> Unit) = strikt.api.expectThat(this, block) +infix fun T.expectThat(block: Assertion.Builder.() -> Unit) = strikt.api.expectThat(this, block) infix fun T.expectIsEqual(expected: T) = expectThat { isEqualTo(expected) }