Skip to content

Commit 936d89e

Browse files
authored
Fix error reporting of incomplete short args (#162)
This was accidentally broken in #102 and #112, and wasn't covered by tests. Noticed when trying to update Ammonite to the latest version of MainArgs in com-lihaoyi/Ammonite#1549 Restored the special casing for tracking/handling incomplete arguments and added some unit test cases
1 parent 80ef899 commit 936d89e

File tree

3 files changed

+48
-13
lines changed

3 files changed

+48
-13
lines changed

mainargs/src/TokenGrouping.scala

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,7 @@ object TokenGrouping {
5858
var i = 0
5959
var currentMap = current
6060
var failure = false
61+
var incomplete: Option[ArgSig] = None
6162

6263
while (i < chars.length) {
6364
val c = chars(i)
@@ -79,6 +80,7 @@ object TokenGrouping {
7980
rest2 match {
8081
case Nil =>
8182
// If there is no next token, it is an error
83+
incomplete = Some(a)
8284
failure = true
8385
case next :: remaining =>
8486
currentMap = Util.appendMap(currentMap, a, next)
@@ -95,7 +97,7 @@ object TokenGrouping {
9597

9698
}
9799

98-
if (failure) None else Some((rest2, currentMap))
100+
if (failure) Left(incomplete) else Right((rest2, currentMap))
99101
}
100102

101103
def lookupArgMap(k: String, m: Map[String, ArgSig]): Option[(ArgSig, mainargs.TokensReader[_])] = {
@@ -111,8 +113,10 @@ object TokenGrouping {
111113
// special handling for combined short args of the style "-xvf" or "-j10"
112114
if (head.startsWith("-") && head.lift(1).exists(c => c != '-')){
113115
parseCombinedShortTokens(current, head, rest) match{
114-
case None => complete(remaining, current)
115-
case Some((rest2, currentMap)) => rec(rest2, currentMap)
116+
case Left(Some(incompleteArg)) =>
117+
Result.Failure.MismatchedArguments(Nil, Nil, Nil, incomplete = Some(incompleteArg))
118+
case Left(None) => complete(remaining, current)
119+
case Right((rest2, currentMap)) => rec(rest2, currentMap)
116120
}
117121

118122
} else if (head.startsWith("-") && head.exists(_ != '-')) {

mainargs/test/src/CoreTests.scala

Lines changed: 38 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,11 @@ object CoreBase {
1515
i: Int,
1616
@arg(doc = "Pass in a custom `s` to override it")
1717
s: String = "lols"
18-
) = s * i
18+
)
19+
= s * i
20+
@main
21+
def baz(arg: Int) = arg
22+
1923
@main
2024
def ex() = throw MyException
2125

@@ -44,7 +48,10 @@ class CoreTests(allowPositional: Boolean) extends TestSuite {
4448
| qux
4549
| Qux is a function that does stuff
4650
| -i <int>
47-
| -s <str> Pass in a custom `s` to override it
51+
| -s <str> Pass in a custom `s` to override it
52+
|
53+
| baz
54+
| --arg <int>
4855
|
4956
| ex
5057
|""".stripMargin
@@ -56,27 +63,31 @@ class CoreTests(allowPositional: Boolean) extends TestSuite {
5663

5764
assert(
5865
names ==
59-
List("foo", "bar", "qux", "ex")
66+
List("foo", "bar", "qux", "baz", "ex")
6067
)
6168
val evaledArgs = check.mains.value.map(_.flattenedArgSigs.map {
62-
case (ArgSig(name, s, docs, None, parser, _, _), _) => (s, docs, None, parser)
69+
case (ArgSig(name, s, docs, None, parser, _, _), _) => (name, s, docs, None, parser)
6370
case (ArgSig(name, s, docs, Some(default), parser, _, _), _) =>
64-
(s, docs, Some(default(CoreBase)), parser)
71+
(name, s, docs, Some(default(CoreBase)), parser)
6572
})
6673

6774
assert(
6875
evaledArgs == List(
6976
List(),
70-
List((Some('i'), None, None, TokensReader.IntRead)),
77+
List((None, Some('i'), None, None, TokensReader.IntRead)),
7178
List(
72-
(Some('i'), None, None, TokensReader.IntRead),
79+
(None, Some('i'), None, None, TokensReader.IntRead),
7380
(
81+
None,
7482
Some('s'),
7583
Some("Pass in a custom `s` to override it"),
7684
Some("lols"),
7785
TokensReader.StringRead
7886
)
7987
),
88+
List(
89+
(Some("arg"), None, None, None, TokensReader.IntRead),
90+
),
8091
List()
8192
)
8293
)
@@ -127,6 +138,26 @@ class CoreTests(allowPositional: Boolean) extends TestSuite {
127138
None
128139
) =>
129140
}
141+
test("incomplete") {
142+
// Make sure both long args and short args properly report
143+
// incomplete arguments as distinct from other mismatches
144+
test - assertMatch(check.parseInvoke(List("qux", "-s"))) {
145+
case Result.Failure.MismatchedArguments(
146+
Nil,
147+
Nil,
148+
Nil,
149+
Some(_)
150+
) =>
151+
}
152+
test - assertMatch(check.parseInvoke(List("baz", "--arg"))) {
153+
case Result.Failure.MismatchedArguments(
154+
Nil,
155+
Nil,
156+
Nil,
157+
Some(_)
158+
) =>
159+
}
160+
}
130161
}
131162

132163
test("tooManyParams") - check(

mainargs/test/src/FlagTests.scala

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -112,10 +112,10 @@ object FlagTests extends TestSuite {
112112
test - check(
113113
List("bool", "-ab"),
114114
Result.Failure.MismatchedArguments(
115-
Vector(new ArgSig(None, Some('b'), None, None, TokensReader.BooleanRead, false, false)),
116-
List("-ab"),
117115
Nil,
118-
None
116+
Nil,
117+
Nil,
118+
Some(new ArgSig(None, Some('b'), None, None, TokensReader.BooleanRead, false, false))
119119
)
120120
)
121121

0 commit comments

Comments
 (0)