Reasoning

Currently there's no real way to modify what BooleanParser interprets as true and what as false. The goal of these basic parser classes is to be open. The class currently as is does not provide any way to modify it's behaviour (in particular what is true and what false)

The "Manager" suffix is ambiguous to say the least. Changing it to "Container" seems like a good alternative as it's not ambiguous

I.e.:

class A
val linker = SyntaxLinkerImpl(syntaxElement(A::class.java))
linker.link(A())

Currently the code above throws an exception (java.lang.UnsupportedOperationException: The given input type [A@5479e3f] isn't supported)

Reasoning

Provide more in-depth information without needing to parse the exception's message.

Goal

Provide access to:

1. the reassigned SyntaxElement
2. the original value
3. the new (reassigned) value

Related issue: #29

Pros:

• no ambiguity if a given syntax is ordered (currently parsers kind of have to guess if a syntax element is meant to be ordered [i.e. allowed to be parsed the way OrderedParserRegistry parses])

Cons:

• ?

Name propositions:

• IndexComponent
• OrderComponent
• PositionComponent

SyntaxComponent must be an instance of SyntaxComponent's id property (Class). This should be clarified in the docs and the default SyntaxElement implementation (SyntaxElementImpl) should fail-fast in case this condition isn't fulfilled.

fun String.extension(name: String, num: Int) = Pair(name, num)

CallableBoundMethod(String::extension, 123456)

The code above won't throw an exception until you invoke the method. It should let the user know beforehand that it's not going to work anyway

Reasoning

When creating a syntaxContainer it's fairly easy to confuse syntaxElement with element. The first will produce a result different than what's expected (the created element won't be added to the SyntaxContainer). It can and probably will lead to unexpected bugs.

Solution

A extension function syntaxElement with receiver of SyntaxContainer. That way even if someone uses syntaxElement it will work exactly the same as element and thus won't create any problems

Reasoning

It can be annoying to need to write { autoIndex() } on every syntax element. The library should be as straightforward and simple as possible. This certainly is an annoyance and makes the API harder to use

Related to #15

Probably by creating a new SyntaxComponent hard (if at all possible) it is possible but potentially may break type safety.

Reasoning

The name is currently too long. You can import ArgumentMatchResult itself but that's not the preferred usage. It should be renamed to ArgumentMatcher.Result to make it more concise.

Currently given a function with (e.g.) 4 parameters JavaDefaultValueSyntaxGenerator requires the defaultValues parameter to have 4 values. In a scenario when you want to have a default value only for the 1st parameter you will still need to provide the other 3 too (probably using NO_DEFAULT_VALUE). This shouldn't be required as it only creates unnecessary boilerplate

Implementation outline

sealed UnknownSyntaxElementException class with .Indexed and .Named subclasses.

Reasoning

• someone could possibly want to get the unknown's syntax element name (or index)
• standardized exception message across parsers (which is good)
• ability to catch a exception only when it is a unknown syntax element exception (instead of catching IllegalArgumentException and parsing the message)

Hi,

hope find you well with this cold call.

I am an author of mocking framework for Kotlin

I see you are using mockito-kotlin.

I just want you to be aware that there is solution that fully supports Kotlin and ask to try it in your new/current projects.

I can help you if you answer to this issue.

Thanks and please star it

Parser (registries) could link syntax straight out of the box thus eliminating the need for SyntaxLinkers. Due to the linking being done in the parser itself it should be more accurate than SyntaxLinkers as parsers know for certain which syntax element is mapped to which key.

Another possibility is for parser registries to return a LinkedMap<K, V> (implementing Map<K, V>, Map<SyntaxElement, V>) and the SyntaxLinker could be just left as it is though it probably won't be needed

Possible solutions

1. Remove SyntaxLinker and replace it with parser registries.

Pros:

• probably better accuracy during linking
• simpler linking (as it wouldn't require anything - syntax would be linked out of the box)

Cons:

• someone may need to link to syntax some external data in which case it would be harder to achieve

2. Keep SyntaxLinker. Create a SyntaxLinkedMap<K, V> : Map<K, V>, Map<SyntaxElement, V> and make parser registries implement them

Pros:

• probably better accuracy during linking
• simpler linking (as it wouldn't require anything - syntax would be linked out of the box) but if anyone needs to, there's the SyntaxLinker approach as well

Cons:

• ?

It's useless and only bloats the code

Related. plus, minus, plusAssign and minusAssign can be just used as methods (i.e. both of the following usages are valid: manager + NullSyntaxGenerator, manager.plus(NullSyntaxGenerator)). Given that why would we want to keep both the operator methods and the normal ones?

Why is matcher in NamedParserRegistry a public var? Doesn't this break encapsulation? If someone wants to use a different ArgumentMatcher it should be possible by passing it through the constructor.

Is ArgumentReader actually useful?
TBD

Couldn't we actually use just a plain String?
TBD

Maybe should ArgumentReader implement CharSequence to allow both?
While in theory it could:
a) it seems insufficient (doesn't expose mark, reset, removeMark)
b) it doesn't exactly comply to CharSequence.getLength which might make it confusing for Parsers

Thoughts
It could be removed but only if Parsers would be used only on Strings (currently the implementation allows Reader and InputStream as an underlying data source for ArgumentReaders). Then the lack of mark, reset, and removeMark could be replaced with substring.

This annotation shouldn't have a default value (symbolizing no name). If you don't want your param to have a name then just don't use this annotation

All parser registries (Simple-, Named- and MixedParserRegistry) don't call parsers when input is empty. This might seem good but it's not because:

• it's not impossible that someone will create a parser returning something even if the input is empty and in that case that someone won't know why isn't his parser returning anything,
• some parsers return syntax's default value when the input is empty but currently that's not possible which might cause some deeper problems

To do:

• SimplePR
• NamedPR
• MixedPR

It's kind of complex now because of all the fuss with instanceParameters and extensionReceiverParameters

Reference. CallableBoundMethod itself won't be able to accept functions with external instances (e.g. CBM(String::substring, "Some example string") won't be possible)

Should it be removed and MixedPRImpl implement instead OrderedPR and NamedPR?

Pros:

• TBD

Cons:

• TBD

Reasoning

Some of the currently public classes are of no help to the user and may only unintentionally confuse him. The classes are not expected to be used outside of the API any way

Class list

• SingleSyntaxElementIterator
• SyntaxElementImpl
• SyntaxContainerImpl
• SyntaxLinkedMapImpl

Currently Binding doesn't really have too much of a purpose. There needs some work to be done so it either is more useful or it'll be removed

Options

1. Create a extension function

Something like BoundMethod.invoke(ArgumentReader, Parser): Any?

Pros:

• breaks compatibility in a easy to fix way

Cons:

• ?

The following code will fail. This is a serious problem that must be fixed ASAP.

val dsl = SyntaxElementDSL(Any::class.java)
val anotherDsl = SyntaxElementDSL(Any::class.java)

dsl.name = "dsl1 name"

assertEquals(null, anotherDsl.name)

Currently when you need to access MethodBinder.generator from Java code you need to use it through the binder instance (MethodBinder.INSTANCE.getGenerator). This should be static as it would reduce the code noise and not making it static doesn't have any adventages

Consider the following code:

open class A
class B : A()

val bInstance = B()
val linker = SyntaxLinkerImpl(syntaxElement(A::class.java))

linker.link(bInstance)

It throws an exception (java.lang.UnsupportedOperationException: The given input type [B@27082746 (null)] isn't supported) though it should map bInstance to syntaxElement(A::class.java)

Reasoning

It's a nuisance for anyone using an array or list if there's no built-in parser for them. These types are used commonly enough to make the cost worth it

Goals

1. Custom starting and closing characters (Strings?). By default just [ and ]. Do not allow mixing (e.g. if chars would be [ to ] and ( to ) do not allow for [1,2,3)

2. Allow to have content of anything as long as there's a parser able to parse that anything (pass a parser to constructor)

3. Edge cases:

   Input	Output
   "[]"	empty list (array)
   "[,]"	make the element parser parse "" (twice)
   ""	exception / default value
   "["	exception
   empty input	empty list (array)

Some extensions to simplify adding parsers. More less like the ones below

fun <T : ParserRegistry> T.withParsers(vararg parsers: Parser): T {
    for (parser in parsers)
        registerParser(parser)

    return this
}

typealias Name = String
fun <T : NamedParserRegistry> T.withNamedParsers(vararg parsers: Pair<Parser, Name>): T {
    for ((parser, name) in parsers)
        registerParser(parser, name)

    return this
}

typealias Position = Int
fun <T : OrderedParserRegistry> T.withOrderedParsers(vararg parsers: Pair<Parser, Position>): T {
    for ((parser, pos) in parsers)
        registerParser(parser, pos)

    return this
}

fun <T : OrderedParserRegistry> T.withOrderedParsers(vararg parsers: Parser, offset: Position = 0): T {
    for ((i, parser) in parsers.withIndex())
        registerParser(parser, i + offset)

    return this
}

Usage:

val parser = SimpleParserRegistry().withParsers(SomeParser(), SomeOtherParser())
val namedParser = NamedParserRegistryImpl().withNamedParsers(SomeParser() to "something", SomeOtherParser() to "other")
val orderedParser = SimpleParserRegistry().withOrderedParsers(SomeParser() to 1, SomeOtherParser() to 2)
val orderedParser2 = SimpleParserRegistry().withOrderedParsers(SomeParser(), SomeOtherParser())

SyntaxLinker currently outputs an ordered array of the given input ((Any) -> Array<Any?>). Instead it should output a map with syntax elements as keys and the linked input as values. Outputting a map would be more meaningful and will be obvious what was mapped to which array element. Example:

fun example(a: Int, b: String) {}

val syntax = MethodBinder.bind(::example).syntax
val linker = SyntaxLinkerImpl(syntax)

linker.link(listOf("String", 12)) // returns [12, "String"]

// Proposed output
linker.link(listOf("String", 12)) // -simplified representation- [SyntaxElement(int) -> 12, SyntaxElement(String) -> "String")

EmptyArgumentReader is an ArgumentReader though it's empty. Any ArgumentReader shouldn't throw exceptions when calling read or skip when count equals 0. Though currently EmptyArgumentReader does violate this unwritten contract which needs to be fixed.

Reasoning

A simple extension function to map a parser's output would be beneficial as it would be shorter than overriding it. Approximate signature of the function: fun <T> Parser.map(outputType: Class<T>, transform: Any? -> T) where Any? is the mapped parser's output value

Related issue: #38

Reasoning

It would be nice to have support for functions of which parameters can be both named and ordered.

Main goal

The expected behavior is something like this

Given:

fun function(@Name("name") something: String) {}
val parser = MixedParserRegistry()
val somethingParser = SequenceParser()
parser.registerParser(somethingParser, "name")
parser.registerParser(somethingParser, 0)

Consider these scenarios
--blabla:123 --name:value otherValue -> Exception thrown (reassign of value)
--blabla:123 --name:value -> return value
--blabla:123 value -> return value

Given syntax like this: Container(Element(index = 0), Element(name = "a", index = 1), Element(name = "b", index = 2)) the parser should be able to correctly parse input.
Test cases:

1. Input: --b=b ordered a
   Output: [a="a", b="b", null=["ordered"]]

Additional goals

1. When SyntaxElement is parsed by position (e.g. input: importantStuff with importantStuff named myVal and ordered 0) it'd be nice to output importantStuff by myVal ([myVal="importantStuff"])
   Test cases:
   1. Syntax: Element(index = 0, name = "name")
      Input: 123
      Output: [name="123"]
2. More specific exceptions when value is reassigned than just IllegalArgumentException

Consider the following scenario

class X {
    fun function(a: String, b: String) { println("a: $a, b: $b") }

    fun main(x: Array<String>) {
        val method = MethodBinder.bind(this::function)
        val linker = SyntaxLinkerImpl(method.syntax)
        method.invoke(linker.link(listOf("a", "b"))
    }
}

The expected output is a: a, b: b but instead the syntax linker throws IllegalStateException: Can't pass two values to one argument @ SyntaxLinkerImpl.linkMap(SyntaxLinkerImpl.kt:71).

Cause: resolveArg searches by value's type and then by index. That approach has problems - as can be seen in this issue - when there are at least 2 arguments without a name and the same types. If the args are searched by index first and then by value's type this problem will be solved.

Reasoning

Recently (07cc026), a ValueRedefinitionException has been added and it's already used by MixedParserRegistryImpl. This change is good for consistency and would make it easier for anyone who needs to know the redefined value.^[1]

^[1]: as ValueRedefinitionException has valueName and index properties for redefinition of a element with name and redefinition of a element with index, respectively while name is preferred over index

Current system allows for null values as defaults (in SyntaxElement) only in theory. In practice it's currently nearly impossible to do this.

Core:

• Parser - class
• Parser.parseReturnSyntaxLinkedMap
• ParserRegistry - class
• SimpleParserRegistry - class
• NamedParserRegistry - class
• ArgumentMatcher - class
• OrderedParserRegistry - class _{(methods, warn that pos >= 0)}
• DefaultValueComponent - class
• ValueRedefinitionException - class, Named, Indexed

Binder:

• JavaDefaultValueSyntaxGenerator - class, constructor
• KotlinDefaultValueSynaxGenerator - class
• MethodBinder - class, _{generator, bindMethod(Any, String, Array<Any?>), bindMethod(KCallable, Any?), bindMethod(Any, Method, Array<Any?>)}
• CallableBoundMethod - constructor
• SyntaxGenerator - class, generate
• SyntaxGeneratorManager - class
• SyntaxLinker - class, link?
• Name.kt
• Index.kt

Reasoning

This is already implemented for IndexComponent (#38, #40) so why not implement it for NameComponent as well? It should be useful as it can make it easier to track down bugs and mistakes in copy-pasting or some other stuff

It might cause unexpected and undesired results.
E.g. (time frame):

1. user creates a function with a single unnamed parameter (i.e. without @Name annotation)
2. Syntax generation occurs
3. user sets up MixedParserRegistryImpl
4. user calls MixedParserRI' parse function
5. user expects the parser to parse input by id but the parser searches by id only if the method hasn't got a name. The user doesn't know that ParamNameSyntaxGen generates NameComponent even without placing any annotations whatsoever on parameters

Consider the following code

        val pattern = Pattern.compile("a")
        val reader = " abc".asReader()

        reader.mark()
        reader.read(1)
        reader.matchPattern(pattern)
        reader.reset()

        assertEquals(" abc", reader.readUntilChar('\r'))

The expected output is abc, right? The problem is that currently it will output bc and therefore this code will fail.

val parser = SequenceParser()
parser.parse(EmptyArgumentReader, EmptySyntaxContainer)

The code above throws an IllegalArgumentException (doesn't check for available amount of text, just reads)

Related to #2

Currently the defaultValue and especially required variables are confusing. They should be changed to a easier to understand and somewhat more meaningful names.