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:
Kushal Pisavadia
2026-02-27 10:04:32 +00:00
committed by GitHub
parent cac3e483b5
commit 64ea7951db
2 changed files with 30 additions and 4 deletions

View File

@@ -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;

View File

@@ -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")
}
} }