fix(cli): suggest --help when unknown or unexpected CLI argument is passed (Closes #400)#405
fix(cli): suggest --help when unknown or unexpected CLI argument is passed (Closes #400)#405dagangtj wants to merge 2 commits into
Conversation
…assed When parseArgs encounters an unknown option or unexpected argument, the error message previously only stated what went wrong without telling the user they can run --help to see valid options. This change: - Appends 'Run cve-lite --help to see supported options.' to every Unknown option and Unexpected argument error thrown by args.ts - Updates index.ts to avoid printing the hint a second time when the message already contains it - Adds dedicated unit tests covering all three CLI entry points Closes OWASP#400
…B unavailability Issue OWASP#401: Flag incompatibility errors now explain what each flag does and guide the user on which to remove: - --fix vs --json: explains interactive fixes vs JSON output - --offline/--offline-db vs --osv-url: explains local DB vs custom endpoint - --no-cache vs --offline: explains cache behavior in offline mode - --report vs --json: explains HTML report vs JSON output Issue OWASP#402: Offline advisory database unavailable error now includes the sync command hint: - Without --offline-db: 'To build it, run: cve-lite advisories sync' - With --offline-db: includes --output <path> in the hint Closes OWASP#401 Closes OWASP#402
| if (arg.startsWith("--output=")) { options.output = arg.slice("--output=".length); continue; } | ||
| if (arg.startsWith("-")) throw new Error(`Unknown option: ${arg}`); | ||
| throw new Error(`Unexpected argument: ${arg}`); | ||
| if (arg.startsWith("-")) throw new Error(`Unknown option: ${arg}\nRun `cve-lite --help` to see supported options.`); |
There was a problem hiding this comment.
The backticks around cve-lite --help aren't escaped here, so the template literal terminates early — tsc --noEmit produces 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:
const HELP_HINT = `\\nRun \`cve-lite --help\` to see supported options.`;
// then: throw new Error(`Unknown option: ${arg}${HELP_HINT}`);| 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")) { |
There was a problem hiding this comment.
This works, but it ties the catch block to the text content of errors thrown in args.ts. If the wording ever changes the deduplication silently breaks. Cleaner to keep args.ts throwing plain messages and always append the hint here unconditionally.
| @@ -0,0 +1,38 @@ | |||
| import { jest } from "@jest/globals"; | |||
There was a problem hiding this comment.
jest is imported but never used — safe to remove. Also, there's already a describe("parseArgs") block in helpers.test.ts — could you fold these in there to keep everything in one place?
| @@ -121,11 +123,11 @@ if (parsedArgs) { | |||
| } | |||
There was a problem hiding this comment.
The improved flag-conflict messages here don't have test coverage yet. The existing cli-integration.test.ts already mocks these option combinations, so it'd be a natural home for assertions on the new message text.
|
@dagangtj please review comments. |
|
@dagangtj Can you please review the conflicts and resolve the comments? |
sonukapoor
left a comment
sonukapoor
left a comment
There was a problem hiding this comment.
Hey, thanks for this — the core idea is exactly right and the test coverage across all three CLI entry points is thorough.
Two things to fix before this can land:
Syntax error in the error messages (all 6 sites in args.ts). The backticks around cve-lite --help aren't escaped, which terminates the template literal early and will cause a compile error. Should be:
throw new Error(`Unknown option: ${arg}\nRun \`cve-lite --help\` to see supported options.`);The existing code in index.ts already uses this escaped form — just follow that pattern.
The src/index.ts changes need to be dropped. The flag conflict error messages (--fix cannot be used with --json, etc.) were moved to src/cli/validate.ts back in v1.18.0, so those edits won't land where you expect them. The offline advisory hint improvement was also already merged separately in a recent release. The only index.ts change worth keeping is the double-hint guard (if (!message.includes("--help"))).
Once you rebase against main and apply those two fixes, this should be straightforward to merge.
|
Also — the branch has merge conflicts with main. Once you've made the fixes above, please rebase against main before pushing so it's ready to merge. |
2 participants
Summary
Fixes #400 — when a user passes an unknown flag or unexpected argument, the error now includes a pointer to --help.
Changes
Acceptance Criteria
Verification
Unit tests added for all three CLI entry points:
Closes #400