Skip to content

Commit 9f1f093

Browse files
committed
refactor(args): validate explicit boolean flag values (review feedback)
Address PR review: - hoist the recognised true/false token sets to module-level Sets instead of allocating an array on every boolean-flag parse - accept common spellings (true/1/yes/on, false/0/no/off; case- and whitespace-insensitive) and throw a clear error for an unrecognised explicit value (e.g. `--flag=maybe`, `--flag=`) so a typo cannot silently enable a flag, consistent with numeric-flag validation - expand tests to cover no/off, case/whitespace, true-like values, and the rejection path
1 parent b7c8b77 commit 9f1f093

2 files changed

Lines changed: 39 additions & 9 deletions

File tree

src/args.ts

Lines changed: 20 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,10 @@
11
import type { GlobalFlags } from './types/flags';
22
import type { OptionDef } from './command';
33

4+
/** Recognised spellings for an explicit boolean flag value, e.g. `--flag=false`. */
5+
const BOOLEAN_TRUE_VALUES = new Set(['true', '1', 'yes', 'on']);
6+
const BOOLEAN_FALSE_VALUES = new Set(['false', '0', 'no', 'off']);
7+
48
function kebabToCamel(str: string): string {
59
return str.replace(/-([a-z])/g, (_, c: string) => c.toUpperCase());
610
}
@@ -115,11 +119,22 @@ export function parseFlags(argv: string[], options: OptionDef[]): GlobalFlags {
115119

116120
if (schema.booleans.has(camelKey)) {
117121
// A bare boolean flag (`--flag`) is true. Honour an explicit value
118-
// such as `--flag=false` / `--flag=0` instead of silently forcing the
119-
// flag to true and discarding what the user typed.
120-
(flags as Record<string, unknown>)[camelKey] =
121-
value === undefined ||
122-
!['false', '0', 'no', 'off'].includes(value.trim().toLowerCase());
122+
// (`--flag=false`, `--flag=0`, ...) and reject an unrecognised one so a
123+
// typo cannot silently enable the flag, mirroring numeric-flag handling.
124+
if (value === undefined) {
125+
(flags as Record<string, unknown>)[camelKey] = true;
126+
} else {
127+
const normalized = value.trim().toLowerCase();
128+
if (BOOLEAN_TRUE_VALUES.has(normalized)) {
129+
(flags as Record<string, unknown>)[camelKey] = true;
130+
} else if (BOOLEAN_FALSE_VALUES.has(normalized)) {
131+
(flags as Record<string, unknown>)[camelKey] = false;
132+
} else {
133+
throw new Error(
134+
`Flag --${key} requires a boolean value (e.g. true/false), got "${value}".`,
135+
);
136+
}
137+
}
123138
i++;
124139
continue;
125140
}

test/args.test.ts

Lines changed: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -31,10 +31,25 @@ describe('parseFlags', () => {
3131
expect(parseFlags(['--verbose'], OPTIONS).verbose).toBe(true);
3232
});
3333

34-
it('honours an explicit value on a boolean flag instead of forcing true', () => {
34+
it('honours explicit false-like values on a boolean flag', () => {
3535
// Regression: `--flag=false` / `--flag=0` were silently set to true.
36-
expect(parseFlags(['--verbose=false'], OPTIONS).verbose).toBe(false);
37-
expect(parseFlags(['--verbose=0'], OPTIONS).verbose).toBe(false);
38-
expect(parseFlags(['--verbose=true'], OPTIONS).verbose).toBe(true);
36+
for (const v of ['false', '0', 'no', 'off', ' OFF ', 'False']) {
37+
expect(parseFlags([`--verbose=${v}`], OPTIONS).verbose).toBe(false);
38+
}
39+
});
40+
41+
it('honours explicit true-like values on a boolean flag', () => {
42+
for (const v of ['true', '1', 'yes', 'on', 'TRUE']) {
43+
expect(parseFlags([`--verbose=${v}`], OPTIONS).verbose).toBe(true);
44+
}
45+
});
46+
47+
it('rejects an unrecognised explicit boolean value', () => {
48+
expect(() => parseFlags(['--verbose=maybe'], OPTIONS)).toThrow(
49+
'Flag --verbose requires a boolean value',
50+
);
51+
expect(() => parseFlags(['--verbose='], OPTIONS)).toThrow(
52+
'Flag --verbose requires a boolean value',
53+
);
3954
});
4055
});

0 commit comments

Comments
 (0)