mirror of
https://github.com/apple/pkl.git
synced 2026-04-22 08:18:32 +02:00
Fix stream double-consumption in CommandSpecParser (#1448)
The `choices` stream was consumed eagerly for metavar construction, then captured in a lambda for later validation—which promptly fell over with `IllegalStateException`. Materialise to a `List` straightaway.
This commit is contained in:
committed by
Jen Basch
parent
853ac26e44
commit
9315b8410d
@@ -32,7 +32,6 @@ import java.util.Set;
|
|||||||
import java.util.function.BiFunction;
|
import java.util.function.BiFunction;
|
||||||
import java.util.function.Function;
|
import java.util.function.Function;
|
||||||
import java.util.function.Supplier;
|
import java.util.function.Supplier;
|
||||||
import java.util.stream.Collectors;
|
|
||||||
import org.graalvm.collections.EconomicMap;
|
import org.graalvm.collections.EconomicMap;
|
||||||
import org.pkl.core.CommandSpec;
|
import org.pkl.core.CommandSpec;
|
||||||
import org.pkl.core.CommandSpec.Argument;
|
import org.pkl.core.CommandSpec.Argument;
|
||||||
@@ -771,18 +770,18 @@ public final class CommandSpecParser {
|
|||||||
return true;
|
return true;
|
||||||
} else if (typeNode
|
} else if (typeNode
|
||||||
instanceof TypeNode.UnionOfStringLiteralsTypeNode unionOfStringLiteralsTypeNode) {
|
instanceof TypeNode.UnionOfStringLiteralsTypeNode unionOfStringLiteralsTypeNode) {
|
||||||
var choices = unionOfStringLiteralsTypeNode.getStringLiterals().stream().sorted();
|
var choices = unionOfStringLiteralsTypeNode.getStringLiterals().stream().sorted().toList();
|
||||||
if (each == null)
|
if (each == null)
|
||||||
each =
|
each =
|
||||||
(rawValue, workingDirUri) -> {
|
(rawValue, workingDirUri) -> {
|
||||||
if (!unionOfStringLiteralsTypeNode.getStringLiterals().contains(rawValue)) {
|
if (!unionOfStringLiteralsTypeNode.getStringLiterals().contains(rawValue)) {
|
||||||
throw BadValue.invalidChoice(rawValue, choices.toList());
|
throw BadValue.invalidChoice(rawValue, choices);
|
||||||
}
|
}
|
||||||
return rawValue;
|
return rawValue;
|
||||||
};
|
};
|
||||||
if (all == null) all = this::allChooseLast;
|
if (all == null) all = this::allChooseLast;
|
||||||
if (multiple == null) multiple = false;
|
if (multiple == null) multiple = false;
|
||||||
if (metavar == null) metavar = "[" + choices.collect(Collectors.joining(", ")) + "]";
|
if (metavar == null) metavar = "[" + String.join(", ", choices) + "]";
|
||||||
if (completionCandidates == null)
|
if (completionCandidates == null)
|
||||||
completionCandidates = new Fixed(unionOfStringLiteralsTypeNode.getStringLiterals());
|
completionCandidates = new Fixed(unionOfStringLiteralsTypeNode.getStringLiterals());
|
||||||
return true;
|
return true;
|
||||||
|
|||||||
@@ -787,4 +787,31 @@ class CommandSpecParserTest {
|
|||||||
.contains("Option `foo` with annotation `@CountedFlag` has invalid type `String`.")
|
.contains("Option `foo` with annotation `@CountedFlag` has invalid type `String`.")
|
||||||
assertThat(exc.message).contains("Expected type: `Int`")
|
assertThat(exc.message).contains("Expected type: `Int`")
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
fun `union typed option validates invalid choice without stream error`() {
|
||||||
|
val moduleUri =
|
||||||
|
writePklFile(
|
||||||
|
"cmd.pkl",
|
||||||
|
renderOptions +
|
||||||
|
"""
|
||||||
|
class Options {
|
||||||
|
format: "json" | "yaml" | "toml"
|
||||||
|
}
|
||||||
|
"""
|
||||||
|
.trimIndent(),
|
||||||
|
)
|
||||||
|
|
||||||
|
val spec = parse(moduleUri)
|
||||||
|
val flag = spec.options.first() as CommandSpec.Flag
|
||||||
|
|
||||||
|
assertThat(flag.metavar()).isEqualTo("[json, toml, yaml]")
|
||||||
|
|
||||||
|
val apply =
|
||||||
|
assertThrows<CommandSpec.Option.BadValue> {
|
||||||
|
flag.transformEach().apply("xml", URI("file:///tmp"))
|
||||||
|
}
|
||||||
|
assertThat(apply.message).contains("invalid choice")
|
||||||
|
assertThat(apply.message).contains("xml")
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user