-
-
Notifications
You must be signed in to change notification settings - Fork 30
fix(cli): suggest --help when unknown or unexpected CLI argument is passed (Closes #400) #405
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -46,7 +46,9 @@ try { | |
| } catch (error) { | ||
| const message = error instanceof Error ? error.message : String(error); | ||
| console.error(chalk.red(`Error: ${message}`)); | ||
| console.error(chalk.gray("Run `cve-lite --help` to see supported options.")); | ||
| if (!message.includes("--help")) { | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This works, but it ties the catch block to the text content of errors thrown in |
||
| console.error(chalk.gray("Run `cve-lite --help` to see supported options.")); | ||
| } | ||
| process.exit(1); | ||
| } | ||
|
|
||
|
|
@@ -105,11 +107,11 @@ if (parsedArgs) { | |
| } | ||
|
|
||
| if ((options.offline || options.offlineDb) && options.osvUrl) { | ||
| throw new Error("--offline/--offline-db cannot be used with --osv-url"); | ||
| throw new Error("--offline/--offline-db cannot be used with --osv-url. Choose offline mode (local DB) or online mode (custom OSV endpoint), not both."); | ||
| } | ||
|
|
||
| if (options.noCache && (options.offline || options.offlineDb)) { | ||
| throw new Error("--no-cache cannot be used with --offline or --offline-db"); | ||
| throw new Error("--no-cache cannot be used with --offline or --offline-db. In offline mode the local advisory DB is used directly; --no-cache only applies to online scans."); | ||
| } | ||
|
|
||
| if (options.osvUrl) { | ||
|
|
@@ -121,11 +123,11 @@ if (parsedArgs) { | |
| } | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The improved flag-conflict messages here don't have test coverage yet. The existing |
||
|
|
||
| if (options.fix && options.json) { | ||
| throw new Error("--fix cannot be used with --json"); | ||
| throw new Error("--fix cannot be used with --json. Use --fix to apply fixes interactively, or --json to output scan results as JSON — not both at once."); | ||
| } | ||
|
|
||
| if (options.report && options.json) { | ||
| throw new Error("--report cannot be used with --json"); | ||
| throw new Error("--report cannot be used with --json. Use --report to generate an HTML report, or --json to output scan results as JSON — not both at once."); | ||
| } | ||
|
|
||
| let advisorySourceLine: string; | ||
|
|
@@ -150,7 +152,11 @@ if (parsedArgs) { | |
| advisorySource.cleanup(); | ||
| } catch (error) { | ||
| if (options.offline || options.offlineDb) { | ||
| throw new Error(`Offline advisory database is not available: ${error instanceof Error ? error.message : String(error)}`); | ||
| const reason = error instanceof Error ? error.message : String(error); | ||
| const hint = options.offlineDb | ||
| ? `To build it, run: cve-lite advisories sync --output ${options.offlineDb}` | ||
| : 'To build it, run: cve-lite advisories sync'; | ||
| throw new Error(`Offline advisory database is not available: ${reason}\n${hint}`); | ||
| } | ||
| throw error; | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,38 @@ | ||
| import { jest } from "@jest/globals"; | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
| import { parseArgs } from "../src/cli/args.js"; | ||
|
|
||
| describe("parseArgs", () => { | ||
| it("includes --help hint in unknown option error", () => { | ||
| expect(() => parseArgs(["--verbosse"])).toThrow( | ||
| "Unknown option: --verbosse\nRun `cve-lite --help` to see supported options." | ||
| ); | ||
| }); | ||
|
|
||
| it("includes --help hint in unexpected argument error", () => { | ||
| expect(() => parseArgs([".", "extra-arg"])).toThrow( | ||
| "Unexpected argument: extra-arg\nRun `cve-lite --help` to see supported options." | ||
| ); | ||
| }); | ||
|
|
||
| it("parses valid arguments without error", () => { | ||
| expect(() => parseArgs(["--json", "--verbose"])).not.toThrow(); | ||
| }); | ||
|
|
||
| it("includes --help hint for unknown option in advisories-sync command", () => { | ||
| expect(() => parseArgs(["advisories", "sync", "--bad-flag"])).toThrow( | ||
| "Unknown option: --bad-flag\nRun `cve-lite --help` to see supported options." | ||
| ); | ||
| }); | ||
|
|
||
| it("includes --help hint for unexpected argument in advisories-sync command", () => { | ||
| expect(() => parseArgs(["advisories", "sync", "unexpected"])).toThrow( | ||
| "Unexpected argument: unexpected\nRun `cve-lite --help` to see supported options." | ||
| ); | ||
| }); | ||
|
|
||
| it("includes --help hint for unknown option in install-skill command", () => { | ||
| expect(() => parseArgs(["install-skill", "--bad-flag"])).toThrow( | ||
| "Unknown option: --bad-flag\nRun `cve-lite --help` to see supported options." | ||
| ); | ||
| }); | ||
| }); | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The backticks around
cve-lite --helparen't escaped here, so the template literal terminates early —tsc --noEmitproduces 12 errors across all six lines (19, 29, 30, 79, 81 too).Also worth pulling the hint into a constant since it's repeated six times: