diff --git a/.changeset/bright-flags-stand.md b/.changeset/bright-flags-stand.md new file mode 100644 index 0000000000..131bcf4340 --- /dev/null +++ b/.changeset/bright-flags-stand.md @@ -0,0 +1,5 @@ +--- +"effect": patch +--- + +Fix CLI parsing so command-local flags can override globals without breaking global flags before subcommands. diff --git a/packages/effect/src/unstable/cli/Command.ts b/packages/effect/src/unstable/cli/Command.ts index 31641af9b9..d08e30dd69 100644 --- a/packages/effect/src/unstable/cli/Command.ts +++ b/packages/effect/src/unstable/cli/Command.ts @@ -1535,7 +1535,11 @@ export const runWith = ( // 2. Extract global flag tokens const allFlagParams = allFlags.flatMap((f) => Param.extractSingleParams(f.flag)) const globalRegistry = Parser.createFlagRegistry(allFlagParams.filter(Param.isFlagParam)) - const { flagMap, remainder, errors: globalFlagErrors } = Parser.consumeKnownFlags(tokens, globalRegistry) + const { flagMap, remainder, errors: globalFlagErrors } = Parser.consumeGlobalFlags( + tokens, + command, + globalRegistry + ) const emptyArgs: Param.ParsedArgs = { flags: flagMap, arguments: [] } // 3. Parse command arguments from remaining tokens diff --git a/packages/effect/src/unstable/cli/internal/parser.ts b/packages/effect/src/unstable/cli/internal/parser.ts index 7c78181be0..40abd776e8 100644 --- a/packages/effect/src/unstable/cli/internal/parser.ts +++ b/packages/effect/src/unstable/cli/internal/parser.ts @@ -329,6 +329,17 @@ type ConsumedFlagValue = readonly _tag: "Error" readonly error: CliError.InvalidValue } +type ConsumedFlagValueWithTokens = + | { + readonly _tag: "Value" + readonly value: string | undefined + readonly tokens: ReadonlyArray + } + | { + readonly _tag: "Error" + readonly error: CliError.InvalidValue + readonly tokens: ReadonlyArray + } const isFlagToken = (t: Token): t is FlagToken => t._tag === "LongOption" || t._tag === "ShortOption" @@ -407,60 +418,88 @@ const consumeFlagValue = ( spec: FlagParam, negated = false ): ConsumedFlagValue => { + const consumed = consumeFlagValueWithTokens(cursor, token, spec, negated) + switch (consumed._tag) { + case "Value": + return { + _tag: "Value", + value: consumed.value + } + case "Error": + return { + _tag: "Error", + error: consumed.error + } + } +} + +const consumeFlagValueWithTokens = ( + cursor: TokenCursor, + token: FlagToken, + spec: FlagParam, + negated = false +): ConsumedFlagValueWithTokens => { // Inline value has highest priority if (negated) { if (token.value !== undefined) { return { _tag: "Error", - error: invalidNegatedFlagValue(token, spec, token.value) + error: invalidNegatedFlagValue(token, spec, token.value), + tokens: [] } } const literal = asBooleanLiteral(cursor.peek()) if (literal !== undefined) { - cursor.take() + const literalToken = cursor.take() return { _tag: "Error", - error: invalidNegatedFlagValue(token, spec, literal) + error: invalidNegatedFlagValue(token, spec, literal), + tokens: literalToken === undefined ? [] : [literalToken] } } return { _tag: "Value", - value: "false" + value: "false", + tokens: [] } } if (token.value !== undefined) { return { _tag: "Value", - value: token.value + value: token.value, + tokens: [] } } // Boolean flags: check for explicit literal or default to "true" if (Primitive.isBoolean(spec.primitiveType)) { const literal = asBooleanLiteral(cursor.peek()) - if (literal !== undefined) cursor.take() + const literalToken = literal !== undefined ? cursor.take() : undefined return { _tag: "Value", - value: literal ?? "true" + value: literal ?? "true", + tokens: literalToken === undefined ? [] : [literalToken] } } // Non-boolean: try to consume next Value token const next = cursor.peek() if (next?._tag === "Value") { - cursor.take() + const valueToken = cursor.take() return { _tag: "Value", - value: next.value + value: next.value, + tokens: valueToken === undefined ? [] : [valueToken] } } return { _tag: "Value", - value: undefined + value: undefined, + tokens: [] } } @@ -504,6 +543,161 @@ export const consumeKnownFlags = ( return { flagMap, remainder, errors } } +const extractFlagParams = (command: Command.Any): ReadonlyArray => { + const commandImpl = toImpl(command) + const singles = commandImpl.config.flags.flatMap(Param.extractSingleParams) + return singles.filter(Param.isFlagParam) +} + +const extractContextFlagParams = (command: Command.Any): ReadonlyArray => { + const commandImpl = toImpl(command) + const singles = commandImpl.contextConfig.flags.flatMap(Param.extractSingleParams) + return singles.filter(Param.isFlagParam) +} + +const resolveFromRegistries = ( + token: FlagToken, + registries: ReadonlyArray +): ResolvedFlag | undefined => { + for (const registry of registries) { + const resolved = resolveFlag(token, registry) + if (resolved !== undefined) { + return resolved + } + } + return undefined +} + +const preserveFlag = ( + remainder: Array, + cursor: TokenCursor, + token: FlagToken, + resolved: ResolvedFlag +): void => { + remainder.push(token) + const consumed = consumeFlagValueWithTokens(cursor, token, resolved.param, resolved.negated) + remainder.push(...consumed.tokens) +} + +const localFlagWouldPrecedeSubcommand = ( + token: FlagToken, + remainingTokens: ReadonlyArray, + resolved: ResolvedFlag, + subIndex: Map>, + registries: ReadonlyArray +): boolean => { + const cursor = makeCursor(remainingTokens) + consumeFlagValueWithTokens(cursor, token, resolved.param, resolved.negated) + for (let token = cursor.take(); token; token = cursor.take()) { + if (isFlagToken(token)) { + const known = resolveFromRegistries(token, registries) + if (known !== undefined) { + consumeFlagValueWithTokens(cursor, token, known.param, known.negated) + } + continue + } + + if (token._tag === "Value") { + return subIndex.has(token.value) + } + } + return false +} + +/** + * Consumes global flags while walking the command tree. + * + * Command-local flags take precedence over global flags at the selected command + * level. This lets commands reuse a global flag name, for example a subcommand + * with its own `--version ` flag overriding the built-in global + * `--version` action. + * @internal + */ +export const consumeGlobalFlags = ( + tokens: ReadonlyArray, + command: Command.Any, + registry: FlagRegistry +): { flagMap: FlagMap; remainder: ReadonlyArray; errors: ReadonlyArray } => { + const flagMap = createEmptyFlagMap(registry.params) + const errors: Array = [] + + const consumeLevel = ( + tokens: ReadonlyArray, + command: Command.Any, + ignoredRegistries: ReadonlyArray + ): ReadonlyArray => { + const localRegistry = createFlagRegistry(extractFlagParams(command)) + const inheritedRegistry = createFlagRegistry(extractContextFlagParams(command)) + const subIndex = buildSubcommandIndex(command.subcommands) + const cursor = makeCursor(tokens) + const remainder: Array = [] + let awaitingFirstValue = true + + for (let token = cursor.take(); token; token = cursor.take()) { + if (isFlagToken(token)) { + const ignored = resolveFromRegistries(token, ignoredRegistries) + if (ignored !== undefined) { + preserveFlag(remainder, cursor, token, ignored) + continue + } + + const inherited = resolveFlag(token, inheritedRegistry) + if (inherited !== undefined) { + preserveFlag(remainder, cursor, token, inherited) + continue + } + + const local = resolveFlag(token, localRegistry) + const global = resolveFlag(token, registry) + if (local !== undefined) { + if ( + global === undefined || !awaitingFirstValue || + !localFlagWouldPrecedeSubcommand(token, cursor.rest(), local, subIndex, [ + localRegistry, + inheritedRegistry, + registry + ]) + ) { + preserveFlag(remainder, cursor, token, local) + continue + } + } + + if (global !== undefined) { + const consumed = consumeFlagValueWithTokens(cursor, token, global.param, global.negated) + if (consumed._tag === "Error") { + errors.push(consumed.error) + continue + } + if (consumed.value !== undefined) { + flagMap[global.param.name].push(consumed.value) + } + continue + } + + remainder.push(token) + continue + } + + if (token._tag === "Value" && awaitingFirstValue) { + const sub = subIndex.get(token.value) + if (sub !== undefined) { + remainder.push(token) + remainder.push(...consumeLevel(cursor.rest(), sub, [...ignoredRegistries, inheritedRegistry])) + return remainder + } + awaitingFirstValue = false + } + + remainder.push(token) + } + + return remainder + } + + return { flagMap, remainder: consumeLevel(tokens, command, []), errors } +} + /* ========================================================================== */ /* Error Creation */ /* ========================================================================== */ diff --git a/packages/effect/test/unstable/cli/Command.test.ts b/packages/effect/test/unstable/cli/Command.test.ts index a6d83ff9d8..a3e3077cc3 100644 --- a/packages/effect/test/unstable/cli/Command.test.ts +++ b/packages/effect/test/unstable/cli/Command.test.ts @@ -1422,6 +1422,233 @@ describe("Command", () => { assert.strictEqual(actions.length, 0) }).pipe(Effect.provide(TestLayer))) + it.effect("should let global flags before subcommands override non-shared parent locals", () => + Effect.gen(function*() { + const captured: Array = [] + let childInvoked = false + const child = Command.make("child", {}, () => + Effect.sync(() => { + childInvoked = true + })) + const command = Command.make("tool", { + version: Flag.boolean("version") + }, (config) => + Effect.sync(() => { + captured.push(config.version) + })).pipe(Command.withSubcommands([child])) + + const runCommand = Command.runWith(command, { version: "1.0.0" }) + yield* runCommand(["--version", "child"]) + + assert.deepStrictEqual(captured, []) + assert.isFalse(childInvoked) + + const output = yield* TestConsole.logLines + assert.isTrue(output.some((line) => String(line).includes("1.0.0"))) + }).pipe(Effect.provide(TestLayer))) + + it.effect("should keep shared flags before subcommands over globals", () => + Effect.gen(function*() { + const captured: Array = [] + const root = Command.make("tool", {}, () => Effect.void).pipe( + Command.withSharedFlags({ + version: Flag.boolean("version") + }) + ) + const child = Command.make("child", {}, () => + Effect.gen(function*() { + const parent = yield* root + captured.push(parent.version) + })) + const command = root.pipe(Command.withSubcommands([child])) + + const runCommand = Command.runWith(command, { version: "1.0.0" }) + yield* runCommand(["--version", "child"]) + + assert.deepStrictEqual(captured, [true]) + + const output = yield* TestConsole.logLines + assert.isFalse(output.some((line) => String(line).includes("1.0.0"))) + + yield* runCommand(["child", "--version"]) + + assert.deepStrictEqual(captured, [true, true]) + }).pipe(Effect.provide(TestLayer))) + + it.effect("should keep root local flags over globals when no subcommand is selected", () => + Effect.gen(function*() { + const captured: Array = [] + const child = Command.make("child", {}, () => Effect.void) + const command = Command.make("tool", { + version: Flag.string("version") + }, (config) => + Effect.sync(() => { + captured.push(config.version) + })).pipe(Command.withSubcommands([child])) + + const runCommand = Command.runWith(command, { version: "1.0.0" }) + yield* runCommand(["--version", "canary"]) + + assert.deepStrictEqual(captured, ["canary"]) + + const output = yield* TestConsole.logLines + assert.isFalse(output.some((line) => String(line).includes("1.0.0"))) + }).pipe(Effect.provide(TestLayer))) + + it.effect("should skip intervening local flag values before selecting subcommands", () => + Effect.gen(function*() { + const captured: Array<{ name: string; version: boolean }> = [] + let childInvoked = false + const child = Command.make("child", {}, () => + Effect.sync(() => { + childInvoked = true + })) + const command = Command.make("tool", { + name: Flag.string("name"), + version: Flag.boolean("version") + }, (config) => + Effect.sync(() => { + captured.push(config) + })).pipe(Command.withSubcommands([child])) + + const runCommand = Command.runWith(command, { version: "1.0.0" }) + yield* runCommand(["--version", "--name", "child"]) + + assert.deepStrictEqual(captured, [{ name: "child", version: true }]) + assert.isFalse(childInvoked) + + const output = yield* TestConsole.logLines + assert.isFalse(output.some((line) => String(line).includes("1.0.0"))) + }).pipe(Effect.provide(TestLayer))) + + it.effect("should stop looking for subcommands after the first positional argument", () => + Effect.gen(function*() { + const captured: Array<{ files: ReadonlyArray; version: boolean }> = [] + let childInvoked = false + const child = Command.make("child", {}, () => + Effect.sync(() => { + childInvoked = true + })) + const command = Command.make("tool", { + files: Argument.string("file").pipe(Argument.variadic()), + version: Flag.boolean("version") + }, (config) => + Effect.sync(() => { + captured.push(config) + })).pipe(Command.withSubcommands([child])) + + const runCommand = Command.runWith(command, { version: "1.0.0" }) + yield* runCommand(["--version", "file", "child"]) + + assert.deepStrictEqual(captured, [{ files: ["file", "child"], version: true }]) + assert.isFalse(childInvoked) + + const output = yield* TestConsole.logLines + assert.isFalse(output.some((line) => String(line).includes("1.0.0"))) + }).pipe(Effect.provide(TestLayer))) + + it.effect("should still select subcommands after skipped flag values", () => + Effect.gen(function*() { + const captured: Array<{ name: string; version: boolean }> = [] + let childInvoked = false + const child = Command.make("child", {}, () => + Effect.sync(() => { + childInvoked = true + })) + const command = Command.make("tool", { + name: Flag.string("name"), + version: Flag.boolean("version") + }, (config) => + Effect.sync(() => { + captured.push(config) + })).pipe(Command.withSubcommands([child])) + + const runCommand = Command.runWith(command, { version: "1.0.0" }) + yield* runCommand(["--version", "--name", "value", "child"]) + + assert.deepStrictEqual(captured, []) + assert.isFalse(childInvoked) + + const output = yield* TestConsole.logLines + assert.isTrue(output.some((line) => String(line).includes("1.0.0"))) + }).pipe(Effect.provide(TestLayer))) + + it.effect("should let local flags override global flags on the selected command", () => + Effect.gen(function*() { + const captured: Array = [] + const release = Command.make("release", { + version: Flag.string("version") + }, (config) => + Effect.sync(() => { + captured.push(config.version) + })) + const packageCommand = Command.make("package").pipe(Command.withSubcommands([release])) + const command = Command.make("tool").pipe(Command.withSubcommands([packageCommand])) + + const runCommand = Command.runWith(command, { version: "1.0.0" }) + yield* runCommand(["package", "release", "--version", "canary"]) + + assert.deepStrictEqual(captured, ["canary"]) + + const output = yield* TestConsole.logLines + assert.isFalse(output.some((line) => String(line).includes("1.0.0"))) + }).pipe(Effect.provide(TestLayer))) + + it.effect("should let local short aliases override global short aliases on the selected command", () => + Effect.gen(function*() { + const Output = GlobalFlag.setting("output")({ + flag: Flag.choice("output", ["pretty", "json", "yaml"] as const).pipe( + Flag.withAlias("o"), + Flag.withDefault("pretty") + ) + }) + const captured: Array<"summary" | "json" | "csv"> = [] + const report = Command.make("report", { + output: Flag.choice("output", ["summary", "json", "csv"] as const).pipe( + Flag.withAlias("o"), + Flag.withDefault("summary") + ) + }, (config) => + Effect.sync(() => { + captured.push(config.output) + })) + const command = Command.make("tool").pipe( + Command.withSubcommands([report]), + Command.withGlobalFlags([Output]) + ) + + const runCommand = Command.runWith(command, { version: "1.0.0" }) + yield* runCommand(["report", "-o", "csv"]) + + assert.deepStrictEqual(captured, ["csv"]) + }).pipe(Effect.provide(TestLayer))) + + it.effect("should let local flags override scoped global flags from another command branch", () => + Effect.gen(function*() { + const Region = GlobalFlag.setting("region")({ + flag: Flag.choice("region", ["us", "eu"] as const).pipe(Flag.withDefault("us")) + }) + const captured: Array = [] + let deployInvoked = false + const deploy = Command.make("deploy", {}, () => + Effect.sync(() => { + deployInvoked = true + })).pipe(Command.withGlobalFlags([Region])) + const status = Command.make("status", { + region: Flag.string("region") + }, (config) => + Effect.sync(() => { + captured.push(config.region) + })) + const command = Command.make("app").pipe(Command.withSubcommands([deploy, status])) + + const runCommand = Command.runWith(command, { version: "1.0.0" }) + yield* runCommand(["status", "--region", "local"]) + + assert.deepStrictEqual(captured, ["local"]) + assert.isFalse(deployInvoked) + }).pipe(Effect.provide(TestLayer))) + it.effect("should print help when invoked with no arguments", () => Effect.gen(function*() { yield* Cli.run([]).pipe(Effect.catchTag("ShowHelp", () => Effect.void))