cli: add --check syntax mode and Node's CLI error contract - #35375
Closed
cirospaciari wants to merge 2 commits into
Closed
cli: add --check syntax mode and Node's CLI error contract#35375cirospaciari wants to merge 2 commits into
cirospaciari wants to merge 2 commits into
Conversation
Adds the Node.js CLI surface that Node v26.3.0's test-cli-* files exercise.
--check / -c parses the entry point (or piped stdin) without executing it.
CommonJS sources are checked inside the module wrapper, so a top-level return
is accepted exactly where require() would accept it, including a wrapper
replaced through require('module').wrapper by a --require preload (preloads
still run first). .mjs files and --input-type=module are checked as ES
modules. Syntax errors print JSC's parser message prefixed by the file path
(or [stdin]) and exit 1; a missing target prints "Error: Cannot find module
'<path>'"; --check together with --eval exits 9.
--input-type is recognized and currently honored by --check.
-e, --eval, -p, --print, --inspect-port and --debug-port without a value now
print "<execPath>: <flag> requires an argument" and exit 9, matching Node.
--inspect-port / --debug-port set the default [host:]port used when the
debugger is activated by --inspect/--inspect-wait/--inspect-brk without its
own target; they do not activate the debugger on their own.
The Node permission-model flags are rejected rather than ignored:
--allow-fs-read / --allow-fs-write without --permission error with
"--permission is required", and --permission itself is rejected as
unsupported. Bun does not implement the permission model, and running without
a requested sandbox is worse than failing.
NODE_OPTIONS now rejects the values Node rejects (--version, -v, --help, -h,
--eval, -e, --print, -p, -pe, --check, -c, --interactive, -i, --v8-options,
--test, --, --expose-internals in either spelling) with Node's message and
exit code 9. The remaining NODE_OPTIONS entries are still not applied.
-c means --check for `bun` and `bun run`; --config keeps its long form
everywhere and every other subcommand keeps -c for --config.
--config is declared with an optional value, and clap's optional-value path
drops an attached value and never consumes the next token, so on the runtime
commands neither `bun -c=bunfig.toml run x` nor `bun -c bunfig.toml run x`
ever loaded the named config: the first was bare -c with the path dropped,
the second treated bunfig.toml as the entry point. What bare -c did do is ask
for the default ./bunfig.toml, which `bun run` does not load on its own;
spell that --config now. The two repo tests that passed a path through -c=
are updated accordingly.
Vendors the Node v26.3.0 tests that now pass: test-cli-bad-options.js,
test-cli-node-options-disallowed.js, test-cli-syntax-eval.js,
test-cli-syntax-piped-bad.js, test-cli-syntax-piped-good.js, and sequential
test-cli-syntax-{bad,file-not-found,good,require}.js. test/cli/run/run-eval.test.ts
gains Bun-side coverage for all of the above.
Collaborator
Contributor
|
Found 2 issues this PR may fix:
🤖 Generated with Claude Code |
Contributor
|
This PR may be a duplicate of:
🤖 Generated with Claude Code |
-p turns on print mode and may also carry the script, matching Node's registration of --print as a boolean plus a `--print <arg>` alias for `--print --eval <arg>`. `bun -pe 42`, `bun -p -e 42`, `bun -p 42` and bare `bun -p` (prints undefined) all work now. Bun's parser has no alias expansion, so -e/-p opt into Node's value-binding rules through two new Values variants implemented once in node_style_value, used by both the long and short option paths. Mirroring node_options-inl.h: - a separate following argument starting with '-' is never the value, it is a missing value instead (`bun -e -p` exits 9) - a separate following argument may escape that with a leading backslash, which is stripped (`bun -p "\-42"` prints -42); the '=' form does not unescape, so `--eval=\-42` keeps the backslash - an empty '=' value is a missing argument for --eval but no value at all for --print, which is a boolean upstream - an empty following argument is not consumed as --print's script Empty and absent are now distinct: `provided` records that -e/-p was passed at all, so `bun -e ""` runs an empty program instead of printing help. That replaces the argv-shape shim in Command::start, and makes `bun --check -e ""` report the --check/--eval conflict the way Node does. Missing-value errors echo the argument as written, so `--eval=` reports `--eval= requires an argument`. `--no-<x>` where `<x>` is an option that takes a value is now rejected with Node's wording and exit code instead of being ignored. An unknown `--no-<x>` is still ignored: Bun tolerates unrecognized flags on purpose so the Node options it does not implement stay harmless, and rejecting them broke --no-warnings, --no-extra-info-on-fatal-exception and friends. --no-warnings is a declared flag now and actually suppresses process warnings, flipping the same switch NODE_NO_WARNINGS=1 already did.
Member
Author
|
Superseded by #35391, which combines this branch with the other five stacked on The branch |
Member
Author
|
🤖 From the combined PR #35391: two files trace to the CLI error contract on your branch. 1. Exit 9 is your Node-compatible CLI error code. Confirmed member-caused: 181 pass / 1 fail on 2. The test re-spawns itself with Worth deciding deliberately, since a permission-model branch is now in flight: does Everything else from your branch is green on #35391. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stacked on #34660 — review that first. This branch is based on
claude/callback-throw-uncaught, notmain.What this does
Adds the Node.js CLI surface the
test-cli-*files in Node's v26.3.0 test suite exercise, and vendors the ones that now pass:--check/-csyntax-check mode. Parses the entry point (or piped stdin) without executing it. CommonJS sources are checked inside the module wrapper, so a top-levelreturnis accepted exactly whererequire()would accept it — including a wrapper replaced throughrequire('module').wrapperby a--requirepreload, which still runs first..mjsand--input-type=moduleare checked as ES modules. Syntax errors print JSC's parser message prefixed by the file path (or[stdin]) and exit 1; a missing target printsError: Cannot find module '<path>';--checktogether with--evalexits 9.--input-typeis recognized and currently honored by--check.-e,--eval,-p,--print,--inspect-port,--debug-portwithout a value now print<execPath>: <flag> requires an argumentand exit 9, the waynodedoes.--inspect-port/--debug-port. They set the default[host:]portused when the debugger is activated by--inspect/--inspect-wait/--inspect-brkwithout its own target. They do not activate the debugger on their own, matching Node.--allow-fs-read/--allow-fs-writewithout--permissionerror with--permission is required(exit 1);--permissionitself is rejected as unsupported. Bun does not implement the permission model, and silently running without a requested sandbox is worse than failing.-e/-pfollow Node's value-binding rules.-pturns on print mode and may also carry the script, sobun -pe 42,bun -p -e 42,bun -p 42and barebun -p(printsundefined) all work. A separate following argument starting with-is never taken as the value (bun -e -pexits 9), a leading backslash escapes that (bun -p "\-42"prints-42), and--eval=reports itself verbatim. Empty and absent are now distinct, sobun -e ""runs an empty program instead of printing help — that replaces an argv-shape shim inCommand::start.--no-<x>where<x>is a known option that takes a value is rejected with Node's wording and exit code, instead of being silently ignored.--no-warningsis a real flag now: it suppresses process warnings, the same switchNODE_NO_WARNINGS=1already flipped.NODE_OPTIONSrejects the values Node rejects.--version,-v,--help,-h,--eval,-e,--print,-p,-pe,--check,-c,--interactive,-i,--v8-options,--test,--, and--expose-internals(either spelling) print<execPath>: <opt> is not allowed in NODE_OPTIONSand exit 9. Bun still does not apply the remainingNODE_OPTIONSentries — this covers only the error contract.The
-cshorthand-cnow means--checkforbunandbun run.--configkeeps its long form everywhere, and every other subcommand (bun install,bun outdated, …) keeps-cfor--config.--configis declared<PATH>?(optional value), and clap's optional-value path discards an attached value and never consumes the next token, so onmaintoday neither spelling passes a path on the runtime commands:bun -c=bunfig.toml run xparses as bare-cwith the path droppedbun -c bunfig.toml run xtreatsbunfig.tomlas the entry point ("Bun cannot run toml files directly")What bare
-cdid do is request the default./bunfig.toml, whichbun rundoes not load on its own. That capability is unchanged, it is just spelled--confignow. The two repo tests that passed a path through-c=are updated:bun-run-bunfig.test.tsto--config=bunfig.toml(the file is always present there, so it now really loads the config it names), and the tsconfig-extends case inbun-run.test.tsto a bare--config, which is what it was actually relying on.Params-table shape
BASE_PARAMS_is split intoBASE_HEAD_PARAMS/BASE_TAIL_PARAMSso the runtime commands can get a--configwithout the-calias without duplicating the other base params. This is the only reason for the split. It does not touchRUNTIME_PARAMS_, where new flags normally get added.Tests
Vendors 9 upstream files from Node v26.3.0, byte-identical, all passing one process per file against a debug build, invoked the way
scripts/runner.node.mjsinvokes them:parallel/test-cli-bad-options.jsparallel/test-cli-node-options-disallowed.jsparallel/test-cli-syntax-eval.jsparallel/test-cli-syntax-piped-bad.jsparallel/test-cli-syntax-piped-good.jssequential/test-cli-syntax-bad.jssequential/test-cli-syntax-file-not-found.jssequential/test-cli-syntax-good.jssequential/test-cli-syntax-require.jstest/cli/run/run-eval.test.tsgains Bun-side coverage for--check(bad syntax, no execution, wrapper-onlyreturn, stdin,--input-type=module, missing file,--check+--eval), the missing-argument exit-9 contract, the permission-flag rejections, and theNODE_OPTIONSdeny list. No changes totest/expectations.txt.Why unknown
--no-<x>is still ignoredNode rejects
--no-i-dont-existwithbad option:. Bun does not, and this PR deliberately keeps it that way: Bun ignores unrecognized flags on purpose so the many Node options it does not implement stay harmless. An earlier revision of this branch did reject them, and it immediately broke--no-warningsin two of the vendored tests;--no-extra-info-on-fatal-exceptionand--no-global-search-pathswould have gone the same way. Only the unambiguous case is rejected — negating an option Bun knows takes a value — which cannot false-positive on a flag Bun has never heard of. There is a regression test for the tolerated set.Overlap with #32622
#32622 ("Node v26 CLI compatibility", open since June) covers much of the same ground and should be triaged against this PR rather than merged alongside it. It is based on a June
main, so its commits no longer apply cleanly; this PR is a fresh implementation against the campaign base, using #32622's design as the reference for--check, theNODE_OPTIONSdeny list, the node-style argument errors, and the-p/-evalue-binding rules.What #32622 has that this PR does not:
--printdesign that prints promises the way Node does (Promise { 42 }), which also removes theentry_point_resultcapture inRun::startsrc/js/internal/eval_print.tshelper and the associatedZigGlobalObject/JSCommonJSModulechangesWhat this PR has that #32622 does not:
--no-<flag>negation diagnosticssequential/test-cli-syntax-bad.jsvendored--config's-csplit against today's table layoutPick one; do not land both.
Still failing, and why
Files from the same upstream group that this PR does not land:
test-cli-eval.js— the-p/-ebinding rules it exercises are all implemented here, but one assertion comparesconsole.logoutput of an array of strings against Node's spelling ([ '--use-strict', … ]). Bun's inspect quotes strings with double quotes. That is a repo-wide formatting choice, not a CLI bug, so the file stays out.test-cli-node-options.js— needsNODE_OPTIONSto actually be applied (-rpreloads,--stack-trace-limit, the V8 option set) and inherited by workers.test-cli-options-as-flags.js— needsinternal/options.getOptionsAsFlagsFromBindingand--experimental-config-file.test-cli-options-negation.js— the negation diagnostics it asserts are implemented, but its first assertion needs theDEP0005new Buffer()deprecation warning, which Bun does not emit. Emitting it faithfully also needs Node'sisInsideNodeModulessuppression, or every dependency still callingnew Buffer()starts printing warnings. The file also wants--no-warnings --warningsto re-enable warnings, which needs order-sensitive boolean negation the parser does not track.test-cli-print-promise.mjs— Bun's--printawaits a promise and prints the settled value; Node prints the promise itself (Promise { 42 }). Deliberate divergence, not changed here.test-cli-node-cli-manpage-env-vars.mjs,test-cli-node-cli-manpage-options.mjs,test-cli-node-options-docs.js— these readdoc/api/cli.md,doc/node.1, andsrc/node_options.ccfrom the Node repository root. They check Node's own documentation for internal consistency and cannot apply to Bun.