From 7e24ad394f9ca4c1b841f02e5d4641c15af5c676 Mon Sep 17 00:00:00 2001
From: hiroki osame <hiroki.osame@gmail.com>
Date: Sat, 29 Jan 2022 12:42:11 -0500
Subject: [PATCH] feat: support eof in parameters (#7)

---
 README.md                             |  32 ++++++++
 examples/npm/commands/run-script.ts   |  18 +++++
 examples/npm/index.ts                 |   2 +
 src/cli.ts                            | 101 +++++++++++++++++-------
 src/render-help/generate-help.ts      |  14 +++-
 tests/__snapshots__/help.spec.ts.snap |  27 +++++++
 tests/arguments.spec.ts               | 108 ++++++++++++++++++++++++++
 tests/help.spec.ts                    |  52 +++++++++++++
 8 files changed, 327 insertions(+), 27 deletions(-)
 create mode 100644 examples/npm/commands/run-script.ts

diff --git a/README.md b/README.md
index c6d02a0..4313987 100644
--- a/README.md
+++ b/README.md
@@ -155,6 +155,38 @@ argv._.optionalParameter // => "b" (string | undefined)
 argv._.optionalSpread // => ["c", "d"] (string[])
 ```
 
+#### End-of-flags
+End-of-flags (`--`) (aka _end-of-options_) allows users to pass in a subset of arguments. This is useful for passing in arguments that should be parsed separately from the rest of the arguments or passing in arguments that look like flags.
+
+An example of this is [`npm run`](https://docs.npmjs.com/cli/v8/commands/npm-run-script):
+```sh
+$ npm run <script> -- <script arguments>
+```
+The `--` indicates that all arguments afterwards should be passed into the _script_ rather than _npm_.
+
+All end-of-flag arguments will be accessible from `argv._['--']`.
+
+Additionally, you can specify `--` in the `parameters` array to parse end-of-flags arguments.
+
+Example:
+
+```ts
+const argv = cli({
+    name: 'npm-run',
+    parameters: [
+        '<script>',
+        '--',
+        '[arguments...]'
+    ]
+})
+
+// $ npm-run echo -- hello world
+
+argv._.script // => "echo" (string)
+argv._.arguments // => ["hello", "world] (string[])
+```
+
+
 ### Flags
 Flags (aka Options) are key-value pairs passed into the script in the format `--flag-name <value>`.
 
diff --git a/examples/npm/commands/run-script.ts b/examples/npm/commands/run-script.ts
new file mode 100644
index 0000000..193bf77
--- /dev/null
+++ b/examples/npm/commands/run-script.ts
@@ -0,0 +1,18 @@
+import { command } from '../../../src';
+
+export const runScript = command({
+	name: 'run-script',
+
+	alias: ['run', 'rum', 'urn'],
+
+	parameters: ['<command>', '--', '[args...]'],
+
+	help: {
+		description: 'Run a script',
+	},
+}, (argv) => {
+	console.log('run', {
+		command: argv._.command,
+		args: argv._.args,
+	});
+});
diff --git a/examples/npm/index.ts b/examples/npm/index.ts
index 40a5d5b..b522df0 100644
--- a/examples/npm/index.ts
+++ b/examples/npm/index.ts
@@ -7,12 +7,14 @@
 
 import { cli } from '../../src';
 import { install } from './commands/install';
+import { runScript } from './commands/run-script';
 
 const argv = cli({
 	name: 'npm',
 
 	commands: [
 		install,
+		runScript,
 	],
 });
 
diff --git a/src/cli.ts b/src/cli.ts
index 8ee5d5c..70a7dd9 100644
--- a/src/cli.ts
+++ b/src/cli.ts
@@ -18,12 +18,14 @@ const { stringify } = JSON;
 
 const specialCharactersPattern = /[|\\{}()[\]^$+*?.]/;
 
+type ParsedParameter = {
+	name: string;
+	required: boolean;
+	spread: boolean;
+};
+
 function parseParameters(parameters: string[]) {
-	const parsedParameters: {
-		name: string;
-		required: boolean;
-		spread: boolean;
-	}[] = [];
+	const parsedParameters: ParsedParameter[] = [];
 
 	let hasOptional: string | undefined;
 	let hasSpread: string | undefined;
@@ -78,6 +80,38 @@ function parseParameters(parameters: string[]) {
 	return parsedParameters;
 }
 
+function mapParametersToArguments(
+	mapping: Record<string, string | string[]>,
+	parameters: ParsedParameter[],
+	cliArguments: string[],
+	showHelp: () => void,
+) {
+	for (let i = 0; i < parameters.length; i += 1) {
+		const { name, required, spread } = parameters[i];
+		const camelCaseName = camelCase(name);
+		if (camelCaseName in mapping) {
+			throw new Error(`Invalid parameter: ${stringify(name)} is used more than once.`);
+		}
+
+		const value = spread ? cliArguments.slice(i) : cliArguments[i];
+
+		if (spread) {
+			i = parameters.length;
+		}
+
+		if (
+			required
+			&& (!value || (spread && value.length === 0))
+		) {
+			console.error(`Error: Missing required parameter ${stringify(name)}\n`);
+			showHelp();
+			return process.exit(1);
+		}
+
+		mapping[camelCaseName] = value;
+	}
+}
+
 function helpEnabled(help: false | undefined | HelpOptions): help is (HelpOptions | undefined) {
 	return (
 		// Default
@@ -165,28 +199,43 @@ function cliBase<
 	}
 
 	if (options.parameters) {
-		const parsedParameters = parseParameters(options.parameters);
-		const _ = parsed._ as (typeof parsed._ & Record<string, string | string[]>);
-
-		for (let i = 0; i < parsedParameters.length; i += 1) {
-			const { name, required, spread } = parsedParameters[i];
-			const value = spread ? parsed._.slice(i) : parsed._[i];
-
-			if (spread) {
-				i = parsedParameters.length;
-			}
-
-			if (
-				required
-				&& (!value || (spread && value.length === 0))
-			) {
-				console.error(`Error: Missing required parameter ${stringify(name)}\n`);
-				showHelp();
-				return process.exit(1);
-			}
-
-			_[camelCase(name)] = value;
+		let { parameters } = options;
+		let cliArguments = parsed._ as string[];
+		const hasEof = parameters.indexOf('--');
+		const eofParameters = parameters.slice(hasEof + 1);
+		const mapping: Record<string, string | string[]> = Object.create(null);
+
+		if (hasEof > -1 && eofParameters.length > 0) {
+			parameters = parameters.slice(0, hasEof);
+
+			const eofArguments = parsed._['--'];
+			cliArguments = cliArguments.slice(0, -eofArguments.length || undefined);
+
+			mapParametersToArguments(
+				mapping,
+				parseParameters(parameters),
+				cliArguments,
+				showHelp,
+			);
+			mapParametersToArguments(
+				mapping,
+				parseParameters(eofParameters),
+				eofArguments,
+				showHelp,
+			);
+		} else {
+			mapParametersToArguments(
+				mapping,
+				parseParameters(parameters),
+				cliArguments,
+				showHelp,
+			);
 		}
+
+		Object.assign(
+			parsed._,
+			mapping,
+		);
 	}
 
 	const parsedWithApi = {
diff --git a/src/render-help/generate-help.ts b/src/render-help/generate-help.ts
index d91eac0..ab4f37f 100644
--- a/src/render-help/generate-help.ts
+++ b/src/render-help/generate-help.ts
@@ -98,7 +98,19 @@ function getUsage(options: Options) {
 			options.parameters
 			&& options.parameters.length > 0
 		) {
-			usage.push(options.parameters.join(' '));
+			const { parameters } = options;
+			const hasEof = parameters.indexOf('--');
+			const hasRequiredParametersAfterEof = hasEof > -1 && parameters.slice(hasEof + 1).some(parameter => parameter.startsWith('<'));
+			usage.push(
+				parameters
+					.map((parameter) => {
+						if (parameter !== '--') {
+							return parameter;
+						}
+						return hasRequiredParametersAfterEof ? '--' : '[--]';
+					})
+					.join(' '),
+			);
 		}
 
 		if (usage.length > 1) {
diff --git a/tests/__snapshots__/help.spec.ts.snap b/tests/__snapshots__/help.spec.ts.snap
index be127db..47efcbe 100644
--- a/tests/__snapshots__/help.spec.ts.snap
+++ b/tests/__snapshots__/help.spec.ts.snap
@@ -23,6 +23,11 @@ EXAMPLES:
 Received value: 123"
 `;
 
+exports[`invalid usage missing required parameter 1`] = `
+"Error: Missing required parameter \\"value-a\\"
+"
+`;
+
 exports[`show help command help 1`] = `
 "my-cli test
 
@@ -175,6 +180,28 @@ exports[`show help parameters with no name 1`] = `
 "
 `;
 
+exports[`show help parameters with optional -- 1`] = `
+"my-cli
+
+USAGE:
+  my-cli [flags...] <arg-a> [arg-b] [--] [arg-c]
+
+FLAGS:
+  -h, --help        Show help                                                             
+"
+`;
+
+exports[`show help parameters with required -- 1`] = `
+"my-cli
+
+USAGE:
+  my-cli [flags...] <arg-a> [arg-b] -- <arg-c>
+
+FLAGS:
+  -h, --help        Show help                                                             
+"
+`;
+
 exports[`show help undefined flags 1`] = `
 "FLAGS:
   -h, --help        Show help                                                             
diff --git a/tests/arguments.spec.ts b/tests/arguments.spec.ts
index 3e98ef2..9d28db0 100644
--- a/tests/arguments.spec.ts
+++ b/tests/arguments.spec.ts
@@ -26,6 +26,42 @@ describe('error handling', () => {
 			}).toThrow('Invalid parameter: "[value.a]". Invalid character found "."');
 		});
 
+		test('duplicate parameters', () => {
+			expect(() => {
+				const parsed = cli(
+					{
+						parameters: ['[value-a]', '[value-a]', '[value-a]'],
+					},
+				);
+
+				expect<string[]>(parsed._).toStrictEqual([]);
+			}).toThrow('Invalid parameter: "value-a" is used more than once');
+		});
+
+		test('duplicate parameters across --', () => {
+			expect(() => {
+				const parsed = cli(
+					{
+						parameters: ['[value-a]', '--', '[value-a]'],
+					},
+				);
+
+				expect<string[]>(parsed._).toStrictEqual([]);
+			}).toThrow('Invalid parameter: "value-a" is used more than once');
+		});
+
+		test('multiple --', () => {
+			expect(() => {
+				const parsed = cli(
+					{
+						parameters: ['[value-a]', '--', '[value-b]', '--', '[value-c]'],
+					},
+				);
+
+				expect<string[]>(parsed._).toStrictEqual([]);
+			}).toThrow('Invalid parameter: "--". Must be wrapped in <> (required parameter) or [] (optional parameter)');
+		});
+
 		test('optional parameter before required parameter', () => {
 			expect(() => {
 				const parsed = cli(
@@ -105,6 +141,19 @@ describe('error handling', () => {
 			expect(mockConsoleError).toHaveBeenCalledWith('Error: Missing required parameter "value-a"\n');
 			expect(mockProcessExit).toHaveBeenCalledWith(1);
 		});
+
+		test('missing -- parameter', () => {
+			cli(
+				{
+					parameters: ['--', '<value-a>'],
+				},
+				undefined,
+				[],
+			);
+
+			expect(mockConsoleError).toHaveBeenCalledWith('Error: Missing required parameter "value-a"\n');
+			expect(mockProcessExit).toHaveBeenCalledWith(1);
+		});
 	});
 });
 
@@ -130,6 +179,46 @@ describe('parses arguments', () => {
 		expect(callback).toHaveBeenCalled();
 	});
 
+	test('simple parsing across --', () => {
+		const callback = jest.fn();
+		const parsed = cli(
+			{
+				parameters: ['<value-a>', '[value-b]', '[value c]', '--', '<value-d>', '[value-e]', '[value f]'],
+			},
+			(callbackParsed) => {
+				expect<string>(callbackParsed._.valueA).toBe('valueA');
+				expect<string | undefined>(callbackParsed._.valueB).toBe('valueB');
+				expect<string | undefined>(callbackParsed._.valueD).toBe('valueD');
+				callback();
+			},
+			['valueA', 'valueB', '--', 'valueD'],
+		);
+
+		expect<string>(parsed._.valueA).toBe('valueA');
+		expect<string | undefined>(parsed._.valueB).toBe('valueB');
+		expect<string | undefined>(parsed._.valueD).toBe('valueD');
+		expect(callback).toHaveBeenCalled();
+	});
+
+	test('simple parsing with empty --', () => {
+		const callback = jest.fn();
+		const parsed = cli(
+			{
+				parameters: ['<value-a>', '[value-b]', '[value c]', '--', '[value-d]'],
+			},
+			(callbackParsed) => {
+				expect<string>(callbackParsed._.valueA).toBe('valueA');
+				expect<string | undefined>(callbackParsed._.valueB).toBe('valueB');
+				callback();
+			},
+			['valueA', 'valueB'],
+		);
+
+		expect<string>(parsed._.valueA).toBe('valueA');
+		expect<string | undefined>(parsed._.valueB).toBe('valueB');
+		expect(callback).toHaveBeenCalled();
+	});
+
 	test('spread', () => {
 		const callback = jest.fn();
 		const parsed = cli(
@@ -147,6 +236,25 @@ describe('parses arguments', () => {
 		expect(callback).toHaveBeenCalled();
 	});
 
+	test('spread with --', () => {
+		const callback = jest.fn();
+		const parsed = cli(
+			{
+				parameters: ['<value-a...>', '--', '<value-b...>'],
+			},
+			(callbackParsed) => {
+				expect<string[]>(callbackParsed._.valueA).toStrictEqual(['valueA', 'valueB']);
+				expect<string[]>(callbackParsed._.valueB).toStrictEqual(['valueC', 'valueD']);
+				callback();
+			},
+			['valueA', 'valueB', '--', 'valueC', 'valueD'],
+		);
+
+		expect<string[]>(parsed._.valueA).toStrictEqual(['valueA', 'valueB']);
+		expect<string[]>(parsed._.valueB).toStrictEqual(['valueC', 'valueD']);
+		expect(callback).toHaveBeenCalled();
+	});
+
 	test('command', () => {
 		const callback = jest.fn();
 
diff --git a/tests/help.spec.ts b/tests/help.spec.ts
index dcee102..a17462a 100644
--- a/tests/help.spec.ts
+++ b/tests/help.spec.ts
@@ -3,6 +3,7 @@ import { cli, command } from '../src';
 
 let mockProcessExit: jest.SpyInstance;
 let mockConsoleLog: jest.SpyInstance;
+let mockConsoleError: jest.SpyInstance;
 
 beforeAll(() => {
 	process.stdout.columns = 90;
@@ -11,12 +12,14 @@ beforeAll(() => {
 
 beforeEach(() => {
 	mockConsoleLog = jest.spyOn(console, 'log').mockImplementation();
+	mockConsoleError = jest.spyOn(console, 'error').mockImplementation();
 
 	mockProcessExit.mockClear();
 });
 
 afterEach(() => {
 	mockConsoleLog.mockRestore();
+	mockConsoleError.mockRestore();
 });
 
 afterAll(() => {
@@ -98,6 +101,38 @@ describe('show help', () => {
 		expect(calls[0][0]).toMatchSnapshot();
 	});
 
+	test('parameters with optional --', () => {
+		cli(
+			{
+				name: 'my-cli',
+				parameters: ['<arg-a>', '[arg-b]', '--', '[arg-c]'],
+			},
+			undefined,
+			['--help'],
+		);
+
+		expect(mockProcessExit).toHaveBeenCalledWith(0);
+
+		const { calls } = mockConsoleLog.mock;
+		expect(calls[0][0]).toMatchSnapshot();
+	});
+
+	test('parameters with required --', () => {
+		cli(
+			{
+				name: 'my-cli',
+				parameters: ['<arg-a>', '[arg-b]', '--', '<arg-c>'],
+			},
+			undefined,
+			['--help'],
+		);
+
+		expect(mockProcessExit).toHaveBeenCalledWith(0);
+
+		const { calls } = mockConsoleLog.mock;
+		expect(calls[0][0]).toMatchSnapshot();
+	});
+
 	test('empty commands', () => {
 		cli(
 			{
@@ -397,6 +432,23 @@ describe('show help', () => {
 	});
 });
 
+describe('invalid usage', () => {
+	test('missing required parameter', () => {
+		cli(
+			{
+				name: 'my-cli',
+				parameters: ['<value-a>'],
+			},
+			undefined,
+			[],
+		);
+
+		expect(mockProcessExit).toHaveBeenCalledWith(1);
+		const { calls } = mockConsoleError.mock;
+		expect(calls[0][0]).toMatchSnapshot();
+	});
+});
+
 test('show version', () => {
 	cli(
 		{