From 05c25ba9fe3d8eff16207fc36a94e6e5d996b9d1 Mon Sep 17 00:00:00 2001 From: Willie Ruemmele Date: Thu, 18 Jul 2024 09:52:14 -0600 Subject: [PATCH 1/5] feat: parse error messages to suggest correct flag value usage --- src/sfCommand.ts | 24 +++++++++++ test/unit/sfCommand.test.ts | 80 +++++++++++++++++++++++++++++++++++++ 2 files changed, 104 insertions(+) diff --git a/src/sfCommand.ts b/src/sfCommand.ts index 6257e336..1f5c5b6b 100644 --- a/src/sfCommand.ts +++ b/src/sfCommand.ts @@ -376,6 +376,30 @@ export abstract class SfCommand extends Command { const sfCommandError = SfCommandError.from(error, this.statics.name, this.warnings); process.exitCode = sfCommandError.exitCode; + // no var args (strict = true || undefined), and unexpected arguments when parsing + if ( + this.statics.strict !== false && + sfCommandError.exitCode === 2 && + error.message.includes('Unexpected argument') + ) { + // @ts-expect-error error's causes aren't typed, this is what's returned from flag parsing errors + // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access + const output = + (sfCommandError.cause?.parse?.output?.raw as Array<{ flag: string; input: string; type: 'flag' | 'arg' }>) ?? + []; + + // find the extra arguments causing issues + const extras = output + .filter((f) => f.type === 'arg') + .flatMap((f) => f.input) + .join(' '); + // find the flag before the 'args' block that's valid, to append the args with its value as a suggestion + const target = output.find((flag, index) => flag.type === 'flag' && output[index + 1]?.type === 'arg'); + + sfCommandError.actions ??= []; + sfCommandError.actions.push(`--${target?.flag} "${target?.input} ${extras}"`); + } + if (this.jsonEnabled()) { this.logJson(sfCommandError.toJson()); } else { diff --git a/test/unit/sfCommand.test.ts b/test/unit/sfCommand.test.ts index 8369c00f..19aa49e6 100644 --- a/test/unit/sfCommand.test.ts +++ b/test/unit/sfCommand.test.ts @@ -110,6 +110,23 @@ class NonJsonCommand extends SfCommand { } } +class SuggestionCommand extends SfCommand { + public static enableJsonFlag = false; + public static readonly flags = { + first: Flags.string({ + default: 'My first flag', + required: true, + }), + second: Flags.string({ + default: 'My second', + required: true, + }), + }; + public async run(): Promise { + await this.parse(SuggestionCommand); + } +} + describe('jsonEnabled', () => { afterEach(() => { delete process.env.SF_CONTENT_TYPE; @@ -375,6 +392,69 @@ describe('error standardization', () => { } }); + it('should log correct suggestion when user doesnt wrap with quotes', async () => { + const logToStderrStub = $$.SANDBOX.stub(SfCommand.prototype, 'logToStderr'); + try { + await SuggestionCommand.run(['--first', 'my', 'alias', 'with', 'spaces', '--second', 'my second value']); + expect(false, 'error should have been thrown').to.be.true; + } catch (e: unknown) { + expect(e).to.be.instanceOf(SfCommandError); + const err = e as SfCommand.Error; + + // Ensure the error was logged to the console + expect(logToStderrStub.callCount).to.equal(1); + expect(logToStderrStub.firstCall.firstArg).to.contain(err.message); + + // Ensure the error has expected properties + expect(err).to.have.property('actions'); + expect(err.actions).to.deep.equal(['--first "my alias with spaces"']); + expect(err).to.have.property('exitCode', 2); + expect(err).to.have.property('context', 'SuggestionCommand'); + expect(err).to.have.property('data', undefined); + expect(err).to.have.property('cause'); + expect(err).to.have.property('code', '2'); + expect(err).to.have.property('status', 2); + expect(err).to.have.property('stack').and.be.ok; + expect(err).to.have.property('skipOclifErrorHandling', true); + expect(err).to.have.deep.property('oclif', { exit: 2 }); + + // Ensure a sfCommandError event was emitted with the expected data + expect(sfCommandErrorData[0]).to.equal(err); + expect(sfCommandErrorData[1]).to.equal('suggestioncommand'); + } + }); + it('should log correct suggestion when user doesnt wrap with quotes without flag order', async () => { + const logToStderrStub = $$.SANDBOX.stub(SfCommand.prototype, 'logToStderr'); + try { + await SuggestionCommand.run(['--second', 'my second value', '--first', 'my', 'alias', 'with', 'spaces']); + expect(false, 'error should have been thrown').to.be.true; + } catch (e: unknown) { + expect(e).to.be.instanceOf(SfCommandError); + const err = e as SfCommand.Error; + + // Ensure the error was logged to the console + expect(logToStderrStub.callCount).to.equal(1); + expect(logToStderrStub.firstCall.firstArg).to.contain(err.message); + + // Ensure the error has expected properties + expect(err).to.have.property('actions'); + expect(err.actions).to.deep.equal(['--first "my alias with spaces"']); + expect(err).to.have.property('exitCode', 2); + expect(err).to.have.property('context', 'SuggestionCommand'); + expect(err).to.have.property('data', undefined); + expect(err).to.have.property('cause'); + expect(err).to.have.property('code', '2'); + expect(err).to.have.property('status', 2); + expect(err).to.have.property('stack').and.be.ok; + expect(err).to.have.property('skipOclifErrorHandling', true); + expect(err).to.have.deep.property('oclif', { exit: 2 }); + + // Ensure a sfCommandError event was emitted with the expected data + expect(sfCommandErrorData[0]).to.equal(err); + expect(sfCommandErrorData[1]).to.equal('suggestioncommand'); + } + }); + it('should log correct error when command throws an SfError --json', async () => { const logJsonStub = $$.SANDBOX.stub(SfCommand.prototype, 'logJson'); try { From d9f7a6d7c19944c696291a506abfe7f6c5ede8f7 Mon Sep 17 00:00:00 2001 From: Willie Ruemmele Date: Thu, 18 Jul 2024 09:53:30 -0600 Subject: [PATCH 2/5] style: fix linting --- src/sfCommand.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/sfCommand.ts b/src/sfCommand.ts index 1f5c5b6b..37180178 100644 --- a/src/sfCommand.ts +++ b/src/sfCommand.ts @@ -382,9 +382,9 @@ export abstract class SfCommand extends Command { sfCommandError.exitCode === 2 && error.message.includes('Unexpected argument') ) { - // @ts-expect-error error's causes aren't typed, this is what's returned from flag parsing errors - // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access const output = + // @ts-expect-error error's causes aren't typed, this is what's returned from flag parsing errors + // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access (sfCommandError.cause?.parse?.output?.raw as Array<{ flag: string; input: string; type: 'flag' | 'arg' }>) ?? []; From ad49d559876e97ba2aaaafa808ff9f85d5225201 Mon Sep 17 00:00:00 2001 From: Willie Ruemmele Date: Fri, 19 Jul 2024 14:31:58 -0600 Subject: [PATCH 3/5] chore: move to SfCommandError, find multiple errors correctly --- src/SfCommandError.ts | 36 ++++++++++++++++++++++++++++++++++++ src/sfCommand.ts | 17 +---------------- test/unit/sfCommand.test.ts | 4 ++-- 3 files changed, 39 insertions(+), 18 deletions(-) diff --git a/src/SfCommandError.ts b/src/SfCommandError.ts index c3e995be..505d464a 100644 --- a/src/SfCommandError.ts +++ b/src/SfCommandError.ts @@ -95,4 +95,40 @@ export class SfCommandError extends SfError { result: this.result, }; } + + public appendErrorSuggestions(): void { + const output = + // @ts-expect-error error's causes aren't typed, this is what's returned from flag parsing errors + // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access + (this.cause?.parse?.output?.raw as Array<{ flag: string; input: string; type: 'flag' | 'arg' }>) ?? []; + + /* + if there's a group of args, and additional args separated, we could have multiple suggestions + --first my first --second my second => + try this: + --first "my first" + --second "my second" + */ + + // find the flag before the 'args' block that's valid, to append the args with its value as a suggestion + // const target = output.find((flag, index) => flag.type === 'flag' && output[index + 1]?.type === 'arg'); + + const catcher: Array<{ flag: string; args: string[] }> = []; + output.forEach((k, i) => { + let argCounter = i + 1; + if (k.type === 'flag' && output[argCounter].type === 'arg') { + const args: string[] = []; + // add the flag name, and first correctly parsed value to the suggestion + + while (output[argCounter]?.type === 'arg') { + args.push(output[argCounter].input); + argCounter++; + } + catcher.push({ flag: k.flag, args: [k.input, ...args] }); + } + }); + + this.actions ??= []; + this.actions.push(...catcher.map((cause) => `--${cause.flag} "${cause.args.join(' ')}"`)); + } } diff --git a/src/sfCommand.ts b/src/sfCommand.ts index 37180178..72a6a09b 100644 --- a/src/sfCommand.ts +++ b/src/sfCommand.ts @@ -382,22 +382,7 @@ export abstract class SfCommand extends Command { sfCommandError.exitCode === 2 && error.message.includes('Unexpected argument') ) { - const output = - // @ts-expect-error error's causes aren't typed, this is what's returned from flag parsing errors - // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access - (sfCommandError.cause?.parse?.output?.raw as Array<{ flag: string; input: string; type: 'flag' | 'arg' }>) ?? - []; - - // find the extra arguments causing issues - const extras = output - .filter((f) => f.type === 'arg') - .flatMap((f) => f.input) - .join(' '); - // find the flag before the 'args' block that's valid, to append the args with its value as a suggestion - const target = output.find((flag, index) => flag.type === 'flag' && output[index + 1]?.type === 'arg'); - - sfCommandError.actions ??= []; - sfCommandError.actions.push(`--${target?.flag} "${target?.input} ${extras}"`); + sfCommandError.appendErrorSuggestions(); } if (this.jsonEnabled()) { diff --git a/test/unit/sfCommand.test.ts b/test/unit/sfCommand.test.ts index 19aa49e6..944e9dca 100644 --- a/test/unit/sfCommand.test.ts +++ b/test/unit/sfCommand.test.ts @@ -395,7 +395,7 @@ describe('error standardization', () => { it('should log correct suggestion when user doesnt wrap with quotes', async () => { const logToStderrStub = $$.SANDBOX.stub(SfCommand.prototype, 'logToStderr'); try { - await SuggestionCommand.run(['--first', 'my', 'alias', 'with', 'spaces', '--second', 'my second value']); + await SuggestionCommand.run(['--first', 'my', 'alias', 'with', 'spaces', '--second', 'my second', 'value']); expect(false, 'error should have been thrown').to.be.true; } catch (e: unknown) { expect(e).to.be.instanceOf(SfCommandError); @@ -407,7 +407,7 @@ describe('error standardization', () => { // Ensure the error has expected properties expect(err).to.have.property('actions'); - expect(err.actions).to.deep.equal(['--first "my alias with spaces"']); + expect(err.actions).to.deep.equal(['--first "my alias with spaces"', '--second "my second value"']); expect(err).to.have.property('exitCode', 2); expect(err).to.have.property('context', 'SuggestionCommand'); expect(err).to.have.property('data', undefined); From f708a6904acb49b5d7da74704a92ef07dc3f488a Mon Sep 17 00:00:00 2001 From: Willie Ruemmele Date: Fri, 19 Jul 2024 14:42:04 -0600 Subject: [PATCH 4/5] chore: cleanup --- src/SfCommandError.ts | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/src/SfCommandError.ts b/src/SfCommandError.ts index 505d464a..a0413286 100644 --- a/src/SfCommandError.ts +++ b/src/SfCommandError.ts @@ -110,25 +110,20 @@ export class SfCommandError extends SfError { --second "my second" */ - // find the flag before the 'args' block that's valid, to append the args with its value as a suggestion - // const target = output.find((flag, index) => flag.type === 'flag' && output[index + 1]?.type === 'arg'); - - const catcher: Array<{ flag: string; args: string[] }> = []; + const aggregator: Array<{ flag: string; args: string[] }> = []; output.forEach((k, i) => { let argCounter = i + 1; if (k.type === 'flag' && output[argCounter].type === 'arg') { const args: string[] = []; - // add the flag name, and first correctly parsed value to the suggestion - while (output[argCounter]?.type === 'arg') { args.push(output[argCounter].input); argCounter++; } - catcher.push({ flag: k.flag, args: [k.input, ...args] }); + aggregator.push({ flag: k.flag, args: [k.input, ...args] }); } }); this.actions ??= []; - this.actions.push(...catcher.map((cause) => `--${cause.flag} "${cause.args.join(' ')}"`)); + this.actions.push(...aggregator.map((cause) => `--${cause.flag} "${cause.args.join(' ')}"`)); } } From 12db42d13fef4fc964c09906c0584b4df947755f Mon Sep 17 00:00:00 2001 From: Willie Ruemmele Date: Fri, 19 Jul 2024 14:48:02 -0600 Subject: [PATCH 5/5] chore: optional type --- src/SfCommandError.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/SfCommandError.ts b/src/SfCommandError.ts index a0413286..0f94e7fc 100644 --- a/src/SfCommandError.ts +++ b/src/SfCommandError.ts @@ -113,7 +113,7 @@ export class SfCommandError extends SfError { const aggregator: Array<{ flag: string; args: string[] }> = []; output.forEach((k, i) => { let argCounter = i + 1; - if (k.type === 'flag' && output[argCounter].type === 'arg') { + if (k.type === 'flag' && output[argCounter]?.type === 'arg') { const args: string[] = []; while (output[argCounter]?.type === 'arg') { args.push(output[argCounter].input);