fix(ext/node): Support returning tokens and option defaults in node:util.parseArgs (#23192)

Fixes #23179.
Fixes #22454.

Enables passing `{tokens: true}` to `parseArgs` and setting default
values for options.

With this PR, the observable framework works with deno out of the box
(no unstable flags needed).

The existing code was basically copied straight from node, so this PR
mostly just updates that (out of date) vendored code. Also fixes some
issues with error exports (before this PR, in certain error cases we
were attempting to construct error classes that weren't actually in
scope).

The last change (in the second commit) adds a small hack so that we
actually exercise the `test-parse-args.js` node_compat test, previously
it was reported as passing though it should have failed. That test now
passes.

---------

Co-authored-by: Bartek Iwańczuk <biwanczuk@gmail.com>
This commit is contained in:
Nathan Whitaker 2024-04-02 16:20:48 -07:00 committed by Satya Rohith
parent f4e4e1baaf
commit 77f2a2c3f2
No known key found for this signature in database
GPG key ID: B2705CF40523EB05
5 changed files with 305 additions and 143 deletions

View file

@ -17,6 +17,8 @@
* ERR_INVALID_PACKAGE_CONFIG // package.json stuff, probably useless
*/
import { primordials } from "ext:core/mod.js";
const { JSONStringify } = primordials;
import { format, inspect } from "ext:deno_node/internal/util/inspect.mjs";
import { codes } from "ext:deno_node/internal/error_codes.ts";
import {
@ -2629,6 +2631,11 @@ codes.ERR_OUT_OF_RANGE = ERR_OUT_OF_RANGE;
codes.ERR_SOCKET_BAD_PORT = ERR_SOCKET_BAD_PORT;
codes.ERR_BUFFER_OUT_OF_BOUNDS = ERR_BUFFER_OUT_OF_BOUNDS;
codes.ERR_UNKNOWN_ENCODING = ERR_UNKNOWN_ENCODING;
codes.ERR_PARSE_ARGS_INVALID_OPTION_VALUE = ERR_PARSE_ARGS_INVALID_OPTION_VALUE;
codes.ERR_PARSE_ARGS_UNEXPECTED_POSITIONAL =
ERR_PARSE_ARGS_UNEXPECTED_POSITIONAL;
codes.ERR_PARSE_ARGS_UNKNOWN_OPTION = ERR_PARSE_ARGS_UNKNOWN_OPTION;
// TODO(kt3k): assign all error classes here.
/**
@ -2846,6 +2853,7 @@ export default {
ERR_OUT_OF_RANGE,
ERR_PACKAGE_IMPORT_NOT_DEFINED,
ERR_PACKAGE_PATH_NOT_EXPORTED,
ERR_PARSE_ARGS_INVALID_OPTION_VALUE,
ERR_QUICCLIENTSESSION_FAILED,
ERR_QUICCLIENTSESSION_FAILED_SETSOCKET,
ERR_QUICSESSION_DESTROYED,

View file

@ -5,6 +5,7 @@ import { primordials } from "ext:core/mod.js";
const {
ArrayPrototypeForEach,
ArrayPrototypeIncludes,
ArrayPrototypeMap,
ArrayPrototypePushApply,
ArrayPrototypeShift,
ArrayPrototypeSlice,
@ -15,13 +16,16 @@ const {
StringPrototypeCharAt,
StringPrototypeIndexOf,
StringPrototypeSlice,
StringPrototypeStartsWith,
} = primordials;
import {
validateArray,
validateBoolean,
validateBooleanArray,
validateObject,
validateString,
validateStringArray,
validateUnion,
} from "ext:deno_node/internal/validators.mjs";
@ -36,6 +40,7 @@ import {
isShortOptionGroup,
objectGetOwn,
optionsGetOwn,
useDefaultValueOption,
} from "ext:deno_node/internal/util/parse_args/utils.js";
import { codes } from "ext:deno_node/internal/error_codes.ts";
@ -68,20 +73,16 @@ function getMainArgs() {
/**
* In strict mode, throw for possible usage errors like --foo --bar
*
* @param {string} longOption - long option name e.g. 'foo'
* @param {string|undefined} optionValue - value from user args
* @param {string} shortOrLong - option used, with dashes e.g. `-l` or `--long`
* @param {boolean} strict - show errors, from parseArgs({ strict })
* @param {object} token - from tokens as available from parseArgs
*/
function checkOptionLikeValue(longOption, optionValue, shortOrLong, strict) {
if (strict && isOptionLikeValue(optionValue)) {
function checkOptionLikeValue(token) {
if (!token.inlineValue && isOptionLikeValue(token.value)) {
// Only show short example if user used short option.
const example = (shortOrLong.length === 2)
? `'--${longOption}=-XYZ' or '${shortOrLong}-XYZ'`
: `'--${longOption}=-XYZ'`;
const errorMessage = `Option '${shortOrLong}' argument is ambiguous.
Did you forget to specify the option argument for '${shortOrLong}'?
const example = StringPrototypeStartsWith(token.rawName, "--")
? `'${token.rawName}=-XYZ'`
: `'--${token.name}=-XYZ' or '${token.rawName}-XYZ'`;
const errorMessage = `Option '${token.rawName}' argument is ambiguous.
Did you forget to specify the option argument for '${token.rawName}'?
To specify an option argument starting with a dash use ${example}.`;
throw new ERR_PARSE_ARGS_INVALID_OPTION_VALUE(errorMessage);
}
@ -89,39 +90,27 @@ To specify an option argument starting with a dash use ${example}.`;
/**
* In strict mode, throw for usage errors.
*
* @param {string} longOption - long option name e.g. 'foo'
* @param {string|undefined} optionValue - value from user args
* @param {object} options - option configs, from parseArgs({ options })
* @param {string} shortOrLong - option used, with dashes e.g. `-l` or `--long`
* @param {boolean} strict - show errors, from parseArgs({ strict })
* @param {boolean} allowPositionals - from parseArgs({ allowPositionals })
* @param {object} config - from config passed to parseArgs
* @param {object} token - from tokens as available from parseArgs
*/
function checkOptionUsage(
longOption,
optionValue,
options,
shortOrLong,
strict,
allowPositionals,
) {
// Strict and options are used from local context.
if (!strict) return;
if (!ObjectHasOwn(options, longOption)) {
throw new ERR_PARSE_ARGS_UNKNOWN_OPTION(shortOrLong, allowPositionals);
function checkOptionUsage(config, token) {
if (!ObjectHasOwn(config.options, token.name)) {
throw new ERR_PARSE_ARGS_UNKNOWN_OPTION(
token.rawName,
config.allowPositionals,
);
}
const short = optionsGetOwn(options, longOption, "short");
const shortAndLong = short ? `-${short}, --${longOption}` : `--${longOption}`;
const type = optionsGetOwn(options, longOption, "type");
if (type === "string" && typeof optionValue !== "string") {
const short = optionsGetOwn(config.options, token.name, "short");
const shortAndLong = `${short ? `-${short}, ` : ""}--${token.name}`;
const type = optionsGetOwn(config.options, token.name, "type");
if (type === "string" && typeof token.value !== "string") {
throw new ERR_PARSE_ARGS_INVALID_OPTION_VALUE(
`Option '${shortAndLong} <value>' argument missing`,
);
}
// (Idiomatic test for undefined||null, expecting undefined.)
if (type === "boolean" && optionValue != null) {
if (type === "boolean" && token.value != null) {
throw new ERR_PARSE_ARGS_INVALID_OPTION_VALUE(
`Option '${shortAndLong}' does not take an argument`,
);
@ -130,7 +119,6 @@ function checkOptionUsage(
/**
* Store the option value in `values`.
*
* @param {string} longOption - long option name e.g. 'foo'
* @param {string|undefined} optionValue - value from user args
* @param {object} options - option configs, from parseArgs({ options })
@ -160,71 +148,56 @@ function storeOption(longOption, optionValue, options, values) {
}
}
export const parseArgs = (config = { __proto__: null }) => {
const args = objectGetOwn(config, "args") ?? getMainArgs();
const strict = objectGetOwn(config, "strict") ?? true;
const allowPositionals = objectGetOwn(config, "allowPositionals") ?? !strict;
const options = objectGetOwn(config, "options") ?? { __proto__: null };
/**
* Store the default option value in `values`.
* @param {string} longOption - long option name e.g. 'foo'
* @param {string
* | boolean
* | string[]
* | boolean[]} optionValue - default value from option config
* @param {object} values - option values returned in `values` by parseArgs
*/
function storeDefaultOption(longOption, optionValue, values) {
if (longOption === "__proto__") {
return; // No. Just no.
}
// Validate input configuration.
validateArray(args, "args");
validateBoolean(strict, "strict");
validateBoolean(allowPositionals, "allowPositionals");
validateObject(options, "options");
ArrayPrototypeForEach(
ObjectEntries(options),
({ 0: longOption, 1: optionConfig }) => {
validateObject(optionConfig, `options.${longOption}`);
values[longOption] = optionValue;
}
// type is required
validateUnion(
objectGetOwn(optionConfig, "type"),
`options.${longOption}.type`,
["string", "boolean"],
);
if (ObjectHasOwn(optionConfig, "short")) {
const shortOption = optionConfig.short;
validateString(shortOption, `options.${longOption}.short`);
if (shortOption.length !== 1) {
throw new ERR_INVALID_ARG_VALUE(
`options.${longOption}.short`,
shortOption,
"must be a single character",
);
}
}
if (ObjectHasOwn(optionConfig, "multiple")) {
validateBoolean(
optionConfig.multiple,
`options.${longOption}.multiple`,
);
}
},
);
const result = {
values: { __proto__: null },
positionals: [],
};
/**
* Process args and turn into identified tokens:
* - option (along with value, if any)
* - positional
* - option-terminator
* @param {string[]} args - from parseArgs({ args }) or mainArgs
* @param {object} options - option configs, from parseArgs({ options })
*/
function argsToTokens(args, options) {
const tokens = [];
let index = -1;
let groupCount = 0;
const remainingArgs = ArrayPrototypeSlice(args);
while (remainingArgs.length > 0) {
const arg = ArrayPrototypeShift(remainingArgs);
const nextArg = remainingArgs[0];
if (groupCount > 0) {
groupCount--;
} else {
index++;
}
// Check if `arg` is an options terminator.
// Guideline 10 in https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap12.html
if (arg === "--") {
if (!allowPositionals && remainingArgs.length > 0) {
throw new ERR_PARSE_ARGS_UNEXPECTED_POSITIONAL(nextArg);
}
// Everything after a bare '--' is considered a positional argument.
ArrayPrototypePush(tokens, { kind: "option-terminator", index });
ArrayPrototypePushApply(
result.positionals,
remainingArgs,
tokens,
ArrayPrototypeMap(remainingArgs, (arg) => {
return { kind: "positional", index: ++index, value: arg };
}),
);
break; // Finished processing args, leave while loop.
}
@ -233,24 +206,28 @@ export const parseArgs = (config = { __proto__: null }) => {
// e.g. '-f'
const shortOption = StringPrototypeCharAt(arg, 1);
const longOption = findLongOptionForShort(shortOption, options);
let optionValue;
let value;
let inlineValue;
if (
optionsGetOwn(options, longOption, "type") === "string" &&
isOptionValue(nextArg)
) {
// e.g. '-f', 'bar'
optionValue = ArrayPrototypeShift(remainingArgs);
checkOptionLikeValue(longOption, optionValue, arg, strict);
value = ArrayPrototypeShift(remainingArgs);
inlineValue = false;
}
checkOptionUsage(
longOption,
optionValue,
options,
arg,
strict,
allowPositionals,
ArrayPrototypePush(
tokens,
{
kind: "option",
name: longOption,
rawName: arg,
index,
value,
inlineValue,
},
);
storeOption(longOption, optionValue, options, result.values);
if (value != null) ++index;
continue;
}
@ -274,6 +251,7 @@ export const parseArgs = (config = { __proto__: null }) => {
}
}
ArrayPrototypeUnshiftApply(remainingArgs, expanded);
groupCount = expanded.length;
continue;
}
@ -281,67 +259,178 @@ export const parseArgs = (config = { __proto__: null }) => {
// e.g. -fFILE
const shortOption = StringPrototypeCharAt(arg, 1);
const longOption = findLongOptionForShort(shortOption, options);
const optionValue = StringPrototypeSlice(arg, 2);
checkOptionUsage(
longOption,
optionValue,
options,
`-${shortOption}`,
strict,
allowPositionals,
const value = StringPrototypeSlice(arg, 2);
ArrayPrototypePush(
tokens,
{
kind: "option",
name: longOption,
rawName: `-${shortOption}`,
index,
value,
inlineValue: true,
},
);
storeOption(longOption, optionValue, options, result.values);
continue;
}
if (isLoneLongOption(arg)) {
// e.g. '--foo'
const longOption = StringPrototypeSlice(arg, 2);
let optionValue;
let value;
let inlineValue;
if (
optionsGetOwn(options, longOption, "type") === "string" &&
isOptionValue(nextArg)
) {
// e.g. '--foo', 'bar'
optionValue = ArrayPrototypeShift(remainingArgs);
checkOptionLikeValue(longOption, optionValue, arg, strict);
value = ArrayPrototypeShift(remainingArgs);
inlineValue = false;
}
checkOptionUsage(
longOption,
optionValue,
options,
arg,
strict,
allowPositionals,
ArrayPrototypePush(
tokens,
{
kind: "option",
name: longOption,
rawName: arg,
index,
value,
inlineValue,
},
);
storeOption(longOption, optionValue, options, result.values);
if (value != null) ++index;
continue;
}
if (isLongOptionAndValue(arg)) {
// e.g. --foo=bar
const index = StringPrototypeIndexOf(arg, "=");
const longOption = StringPrototypeSlice(arg, 2, index);
const optionValue = StringPrototypeSlice(arg, index + 1);
checkOptionUsage(
longOption,
optionValue,
options,
`--${longOption}`,
strict,
allowPositionals,
const equalIndex = StringPrototypeIndexOf(arg, "=");
const longOption = StringPrototypeSlice(arg, 2, equalIndex);
const value = StringPrototypeSlice(arg, equalIndex + 1);
ArrayPrototypePush(
tokens,
{
kind: "option",
name: longOption,
rawName: `--${longOption}`,
index,
value,
inlineValue: true,
},
);
storeOption(longOption, optionValue, options, result.values);
continue;
}
// Anything left is a positional
if (!allowPositionals) {
throw new ERR_PARSE_ARGS_UNEXPECTED_POSITIONAL(arg);
}
ArrayPrototypePush(result.positionals, arg);
ArrayPrototypePush(tokens, { kind: "positional", index, value: arg });
}
return tokens;
}
export const parseArgs = (config = { __proto__: null }) => {
const args = objectGetOwn(config, "args") ?? getMainArgs();
const strict = objectGetOwn(config, "strict") ?? true;
const allowPositionals = objectGetOwn(config, "allowPositionals") ?? !strict;
const returnTokens = objectGetOwn(config, "tokens") ?? false;
const options = objectGetOwn(config, "options") ?? { __proto__: null };
// Bundle these up for passing to strict-mode checks.
const parseConfig = { args, strict, options, allowPositionals };
// Validate input configuration.
validateArray(args, "args");
validateBoolean(strict, "strict");
validateBoolean(allowPositionals, "allowPositionals");
validateBoolean(returnTokens, "tokens");
validateObject(options, "options");
ArrayPrototypeForEach(
ObjectEntries(options),
({ 0: longOption, 1: optionConfig }) => {
validateObject(optionConfig, `options.${longOption}`);
// type is required
const optionType = objectGetOwn(optionConfig, "type");
validateUnion(optionType, `options.${longOption}.type`, [
"string",
"boolean",
]);
if (ObjectHasOwn(optionConfig, "short")) {
const shortOption = optionConfig.short;
validateString(shortOption, `options.${longOption}.short`);
if (shortOption.length !== 1) {
throw new ERR_INVALID_ARG_VALUE(
`options.${longOption}.short`,
shortOption,
"must be a single character",
);
}
}
const multipleOption = objectGetOwn(optionConfig, "multiple");
if (ObjectHasOwn(optionConfig, "multiple")) {
validateBoolean(multipleOption, `options.${longOption}.multiple`);
}
const defaultValue = objectGetOwn(optionConfig, "default");
if (defaultValue !== undefined) {
let validator;
switch (optionType) {
case "string":
validator = multipleOption ? validateStringArray : validateString;
break;
case "boolean":
validator = multipleOption ? validateBooleanArray : validateBoolean;
break;
}
validator(defaultValue, `options.${longOption}.default`);
}
},
);
// Phase 1: identify tokens
const tokens = argsToTokens(args, options);
// Phase 2: process tokens into parsed option values and positionals
const result = {
values: { __proto__: null },
positionals: [],
};
if (returnTokens) {
result.tokens = tokens;
}
ArrayPrototypeForEach(tokens, (token) => {
if (token.kind === "option") {
if (strict) {
checkOptionUsage(parseConfig, token);
checkOptionLikeValue(token);
}
storeOption(token.name, token.value, options, result.values);
} else if (token.kind === "positional") {
if (!allowPositionals) {
throw new ERR_PARSE_ARGS_UNEXPECTED_POSITIONAL(token.value);
}
ArrayPrototypePush(result.positionals, token.value);
}
});
// Phase 3: fill in default values for missing args
ArrayPrototypeForEach(
ObjectEntries(options),
({ 0: longOption, 1: optionConfig }) => {
const mustSetDefault = useDefaultValueOption(
longOption,
optionConfig,
result.values,
);
if (mustSetDefault) {
storeDefaultOption(
longOption,
objectGetOwn(optionConfig, "default"),
result.values,
);
}
},
);
return result;
};

View file

@ -173,6 +173,18 @@ function findLongOptionForShort(shortOption, options) {
return longOptionEntry?.[0] ?? shortOption;
}
/**
* Check if the given option includes a default value
* and that option has not been set by the input args.
* @param {string} longOption - long option name e.g. 'foo'
* @param {object} optionConfig - the option configuration properties
* @param {object} values - option values returned in `values` by parseArgs
*/
function useDefaultValueOption(longOption, optionConfig, values) {
return objectGetOwn(optionConfig, "default") !== undefined &&
values[longOption] === undefined;
}
export {
findLongOptionForShort,
isLoneLongOption,
@ -184,4 +196,5 @@ export {
isShortOptionGroup,
objectGetOwn,
optionsGetOwn,
useDefaultValueOption,
};

View file

@ -288,9 +288,51 @@ const validateArray = hideStackFrames(
},
);
/**
* @callback validateStringArray
* @param {*} value
* @param {string} name
* @returns {asserts value is string[]}
*/
/** @type {validateStringArray} */
const validateStringArray = hideStackFrames((value, name) => {
validateArray(value, name);
for (let i = 0; i < value.length; ++i) {
// Don't use validateString here for performance reasons, as
// we would generate intermediate strings for the name.
if (typeof value[i] !== "string") {
throw new codes.ERR_INVALID_ARG_TYPE(`${name}[${i}]`, "string", value[i]);
}
}
});
/**
* @callback validateBooleanArray
* @param {*} value
* @param {string} name
* @returns {asserts value is boolean[]}
*/
/** @type {validateBooleanArray} */
const validateBooleanArray = hideStackFrames((value, name) => {
validateArray(value, name);
for (let i = 0; i < value.length; ++i) {
// Don't use validateBoolean here for performance reasons, as
// we would generate intermediate strings for the name.
if (value[i] !== true && value[i] !== false) {
throw new codes.ERR_INVALID_ARG_TYPE(
`${name}[${i}]`,
"boolean",
value[i],
);
}
}
});
function validateUnion(value, name, union) {
if (!ArrayPrototypeIncludes(union, value)) {
throw new ERR_INVALID_ARG_TYPE(
throw new codes.ERR_INVALID_ARG_TYPE(
name,
`('${ArrayPrototypeJoin(union, "|")}')`,
value,
@ -305,6 +347,7 @@ export default {
validateAbortSignal,
validateArray,
validateBoolean,
validateBooleanArray,
validateBuffer,
validateFunction,
validateInt32,
@ -314,6 +357,7 @@ export default {
validateOneOf,
validatePort,
validateString,
validateStringArray,
validateUint32,
validateUnion,
};
@ -324,6 +368,7 @@ export {
validateAbortSignal,
validateArray,
validateBoolean,
validateBooleanArray,
validateBuffer,
validateFunction,
validateInt32,
@ -333,6 +378,7 @@ export {
validateOneOf,
validatePort,
validateString,
validateStringArray,
validateUint32,
validateUnion,
};

View file

@ -76,18 +76,24 @@ async function runTest(t: Deno.TestContext, path: string): Promise<void> {
// contain actual JS objects, not strings :)).
envVars["NODE_TEST_KNOWN_GLOBALS"] = "0";
}
// TODO(nathanwhit): once we match node's behavior on executing
// `node:test` tests when we run a file, we can remove this
const usesNodeTest = testSource.includes("node:test");
const args = [
"run",
usesNodeTest ? "test" : "run",
"-A",
"--quiet",
//"--unsafely-ignore-certificate-errors",
"--unstable-unsafe-proto",
"--unstable-bare-node-builtins",
"--v8-flags=" + v8Flags.join(),
"runner.ts",
testCase,
];
if (usesNodeTest) {
// deno test typechecks by default + we want to pass script args
args.push("--no-check", "runner.ts", "--", testCase);
} else {
args.push("runner.ts", testCase);
}
// Pipe stdout in order to output each test result as Deno.test output
// That way the tests will respect the `--quiet` option when provided