fix: improve robustness of add-on args parsing (#681)

Co-authored-by: AdrianGonz97 <31664583+AdrianGonz97@users.noreply.github.com>

Author: jycouet

.changeset/odd-words-hunt.md

@@ -0,0 +1,5 @@
+---
+'sv': patch
+---
+
+fix(add): improve robustness of add-on args parsing

packages/cli/commands/add/index.ts

@@ -59,25 +59,16 @@ export const add = new Command('add')
 			process.exit(1);
 		}
 
-		// occurs when an `=` isn't present (e.g. `sv add foo`)
-		if (optionFlags === undefined) {
-			prev.push({ id: addonId, options: undefined });
-			return prev;
-		}
-
-		// validates that the options are relatively well-formed.
-		// occurs when no <name> or <value> is specified (e.g. `sv add foo=demo`).
-		if (optionFlags.length > 0 && !/.+:.*/.test(optionFlags)) {
-			console.error(
-				`Malformed arguments: An add-on's option in '${value}' is missing it's option name or value (e.g. 'addon=option:value').`
-			);
+		try {
+			const options = common.parseAddonOptions(optionFlags);
+			prev.push({ id: addonId, options });
+		} catch (error) {
+			if (error instanceof Error) {
+				console.error(error.message);
+			}
 			process.exit(1);
 		}
 
-		// parses the option flags into a array of `<name>:<value>` strings
-		const options: string[] = optionFlags.match(/[^+]*:[^:]*(?=\+|$)/g) ?? [];
-
-		prev.push({ id: addonId, options });
 		return prev;
 	})
 	.option('-C, --cwd <path>', 'path to working directory', defaultCwd)

packages/cli/tests/common.ts

@@ -0,0 +1,42 @@
+import { describe, expect, it } from 'vitest';
+import { parseAddonOptions } from '../utils/common.ts';
+
+describe('parseAddonOptions', () => {
+	it('returns undefined on undefined', () => {
+		expect(parseAddonOptions(undefined)).toEqual(undefined);
+	});
+	it('returns undefined on empty string', () => {
+		expect(parseAddonOptions('')).toEqual(undefined);
+	});
+	it('parses options and values', () => {
+		expect(parseAddonOptions('option:value')).toEqual(['option:value']);
+	});
+	it('parses values with empty strings', () => {
+		expect(parseAddonOptions('foo:')).toEqual(['foo:']);
+		expect(parseAddonOptions('foo:+bar:+baz:')).toEqual(['foo:', 'bar:', 'baz:']);
+	});
+	it('parses sentences', () => {
+		expect(parseAddonOptions('foo:the quick brown fox')).toEqual(['foo:the quick brown fox']);
+	});
+	it('parses lists', () => {
+		expect(parseAddonOptions('foo:en,es,de')).toEqual(['foo:en,es,de']);
+	});
+	it('parses values with colons', () => {
+		expect(parseAddonOptions('option:foo:bar:baz')).toEqual(['option:foo:bar:baz']);
+	});
+	it('errors on missing value', () => {
+		expect(() => parseAddonOptions('foo')).toThrowError(
+			"Malformed arguments: The following add-on options: 'foo' are missing their option name or value (e.g. 'addon=option1:value1+option2:value2')."
+		);
+	});
+	it('errors when one of two options is missing a value', () => {
+		expect(() => parseAddonOptions('foo:value1+bar')).toThrowError(
+			"Malformed arguments: The following add-on options: 'bar' are missing their option name or value (e.g. 'addon=option1:value1+option2:value2')."
+		);
+	});
+	it('errors on two missing values', () => {
+		expect(() => parseAddonOptions('foo+bar')).toThrowError(
+			"Malformed arguments: The following add-on options: 'foo', 'bar' are missing their option name or value (e.g. 'addon=option1:value1+option2:value2')."
+		);
+	});
+});

packages/cli/utils/common.ts

@@ -115,3 +115,23 @@ export function forwardExitCode(error: unknown) {
 		process.exit(1);
 	}
 }
+
+export function parseAddonOptions(optionFlags: string | undefined): string[] | undefined {
+	// occurs when an `=` isn't present (e.g. `sv add foo`)
+	if (optionFlags === undefined || optionFlags === '') {
+		return undefined;
+	}
+
+	// Split on + and validate each option individually
+	const options = optionFlags.split('+');
+
+	// Validate that each individual option follows the name:value pattern
+	const malformed = options.filter((option) => !/.+:.*/.test(option));
+
+	if (malformed.length > 0) {
+		const message = `Malformed arguments: The following add-on options: ${malformed.map((o) => `'${o}'`).join(', ')} are missing their option name or value (e.g. 'addon=option1:value1+option2:value2').`;
+		throw new Error(message);
+	}
+
+	return options;
+}

packages/cli/vitest.config.ts

@@ -0,0 +1,11 @@
+import { defineProject } from 'vitest/config';
+
+export default defineProject({
+	test: {
+		name: 'cli',
+		include: ['./tests/**/index.ts', './tests/*.ts'],
+		expect: {
+			requireAssertions: true
+		}
+	}
+});