Fix mod dep parser breaking with spaces

packages/controller/src/ControlConnection.ts

@@ -708,14 +708,21 @@ export default class ControlConnection extends BaseConnection {
 
 					// We only select the mod portal release if it is more recent than the local candidate
 					if (release && (!candidate || lib.integerFullVersion(release.version) > candidate.integerVersion)) {
-						candidate = lib.ModInfo.fromJSON({
-							...release.info_json, // Also includes dependences
-							sha1: release.sha1,
-							version: release.version,
-							name: modReleases.name,
-							author: modReleases.owner,
-							title: modReleases.title,
-						});
+						// eslint-disable-next-line max-depth
+						try {
+							candidate = lib.ModInfo.fromJSON({
+								...release.info_json, // Also includes dependencies
+								sha1: release.sha1,
+								version: release.version,
+								name: modReleases.name,
+								author: modReleases.owner,
+								title: modReleases.title,
+							});
+						} catch {
+							// Can error with invalid dependencies, in which case we highlight it as invalid
+							// We modify the name to expose the error to the user.
+							invalid.add(`${modReleases.name} (Has invalid dependency)`);
+						}
 					}
 				}
 			}

packages/lib/src/data/ModInfo.ts

@@ -22,11 +22,12 @@ const UnsatisfiedSeverity: Record<ModDependencyUnsatisfiedReason, number> = {
 	"incompatible": 1, "missing_dependency": 2, "wrong_version": 3,
 };
 
-const lettersRegex = /^[a-zA-Z]+$/;
+const depSpecRegex = /^(?:(\?|\(\?\)|!|~|\+) *)?(.+?)(?: *([<>]=?|=) *([0-9.]+))?$/;
 
 export class ModDependency {
-	public type: ModDependencyType;
+	public spec: string;
 	public name: string;
+	public type: ModDependencyType;
 	public version: ModVersionEquality | undefined;
 
 	static getTypeFromPrefix(prefix: string): ModDependencyType {
@@ -42,50 +43,27 @@ export class ModDependency {
 			case "":
 				return "required";
 			default:
-				throw new Error(`Invalid dependency prefix: ${prefix}`);
+				throw new Error(`Invalid dependency prefix "${prefix}"`);
 		}
 	}
 
 	constructor (
-		public specification: string,
+		specification: string,
 	) {
-		const parts = specification.split(" ");
-
-		// Fix mods which include spaces in their name, joins consecutive words together
-		let index = 0;
-		while (index < parts.length - 1) {
-			const current = parts[index];
-			const next = parts[index + 1];
-			if (lettersRegex.test(current) && lettersRegex.test(next)) {
-				parts[index] = `${current} ${next}`;
-				parts.splice(index + 1, 1);
-			} else {
-				index += 1;
-			}
+		const match = depSpecRegex.exec(specification);
+		if (match === null) {
+			throw new Error(`Invalid dependency specification "${specification}"`);
 		}
 
-		// Parse the dependency specification
-		switch (parts.length) {
-			case 1:
-				this.type = "required";
-				this.name = parts[0];
-				break;
-			case 2:
-				this.type = ModDependency.getTypeFromPrefix(parts[0]);
-				this.name = parts[1];
-				break;
-			case 3:
-				this.type = "required";
-				this.name = parts[0];
-				this.version = ModVersionEquality.fromParts(parts[1], parts[2]);
-				break;
-			case 4:
-				this.type = ModDependency.getTypeFromPrefix(parts[0]);
-				this.name = parts[1];
-				this.version = ModVersionEquality.fromParts(parts[2], parts[3]);
-				break;
-			default:
-				throw new Error(`Invalid dependency specification: ${specification}`);
+		// Expand the match to the different parts
+		const [spec, type, name, equality, version] = match;
+		this.type = ModDependency.getTypeFromPrefix(type ?? "");
+		this.spec = spec;
+		this.name = name;
+
+		// Parse the equality and version
+		if ((equality || version) && !this.incompatible) {
+			this.version = ModVersionEquality.fromParts(equality, version);
 		}
 	}
 
@@ -124,7 +102,7 @@ export class ModDependency {
 	}
 
 	toJSON() {
-		return this.specification;
+		return this.spec;
 	}
 }
 
@@ -218,7 +196,7 @@ export default class ModInfo {
 	 * As they would be represented in info.json.
 	 */
 	get dependencySpecifications() {
-		return this.dependencies.map(d => d.specification);
+		return this.dependencies.map(d => d.spec);
 	}
 
 	checkDependencySatisfaction(mods: (ModInfo | ModRecord)[]) {
@@ -308,7 +286,13 @@ export default class ModInfo {
 		if (json.homepage) { modInfo.homepage = json.homepage; }
 		if (json.description) { modInfo.description = json.description; }
 		if (json.factorio_version) { modInfo.factorioVersion = json.factorio_version; }
-		if (json.dependencies) { modInfo.dependencies = json.dependencies.map(d => new ModDependency(d)); }
+
+		// Parse the dependencies
+		try {
+			if (json.dependencies) { modInfo.dependencies = json.dependencies.map(d => new ModDependency(d)); }
+		} catch (err: any) {
+			throw new Error(`Failed for parse dependencies for ${json.name}: ${err}`);
+		}
 
 		// Additional data
 		if (json.size) { modInfo.size = json.size; }

packages/lib/src/data/version.ts

@@ -151,10 +151,10 @@ export class ModVersionEquality {
 
 	static fromParts(equality: string, version: string) {
 		if (!isVersionEquality(equality)) {
-			throw new Error(`Unknown version equality: ${equality}`);
+			throw new Error(`Invalid version equality "${equality}"`);
 		}
 		if (!isPartialVersion(version)) {
-			throw new Error(`Malformed version string: ${version}`);
+			throw new Error(`Invalid version string "${version}"`);
 		}
 		return new this(equality, version);
 	}

packages/web_ui/src/components/ModDetails.tsx

@@ -98,7 +98,7 @@ export default function ModDetails<T extends lib.ModInfo | lib.ModRecord>(props:
 							{depWarnings.get(e.name) === "wrong_version" && <Tooltip title="Wrong version added.">
 								<FileSyncOutlined style={{ color: "#dd5e14" }} />{" "}
 							</Tooltip>}
-							{e.specification}
+							{e.spec}
 						</Space>
 						<br/>
 					</Fragment>)

test/lib/data/ModInfo.js

@@ -52,20 +52,46 @@ describe("lib/data/ModInfo", function() {
 				assert.equal(dependency.version.equality, ">=");
 				assert.equal(dependency.version.integerVersion, lib.integerFullVersion("1.2.3"));
 			});
+			it("should ignore the version for incompatible dependencies", function() {
+				const dependency = new ModDependency("! my-mod >= 1.2.3");
+				assert.equal(dependency.name, "my-mod");
+				assert.equal(dependency.type, "incompatible");
+				assert.equal(dependency.version, undefined);
+			});
+			it("should throw on an empty string", function() {
+				assert.throws(() => new ModDependency(""));
+			});
 			it("should throw if no version equality is given", function() {
+				this.skip(); // For alignment with factorio, this should be parsed as the mod name
 				assert.throws(() => new ModDependency("my-mod 1.2.3"));
 				assert.throws(() => new ModDependency("? my-mod 1.2.3"));
 			});
+			it("should parse no version equality as the mod name", function() {
+				const dependencyReq = new ModDependency("my-mod 1.2.3");
+				assert.equal(dependencyReq.name, "my-mod 1.2.3");
+				assert.equal(dependencyReq.type, "required");
+				assert.equal(dependencyReq.version, undefined);
+
+				const dependencyOpt = new ModDependency("? my-mod 1.2.3");
+				assert.equal(dependencyOpt.name, "my-mod 1.2.3");
+				assert.equal(dependencyOpt.type, "optional");
+				assert.equal(dependencyOpt.version, undefined);
+			});
 			it("should throw if invalid specification given", function() {
+				this.skip(); // For alignment with factorio, this should be parsed as the mod name
+				assert.throws(() => new ModDependency("& my-mod = 1.2.3"));
 				assert.throws(() => new ModDependency("? ? my-mod = 1.2.3"));
 			});
-			it("should throw if name contains spaces", function() {
-				// Apparently there are some legacy mods which include spaces
-				// Such as https://mods.factorio.com/mod/Explosive%20Excavation
-				this.skip();
-				assert.throws(() => new ModDependency("my mod"));
-				assert.throws(() => new ModDependency("? my mod"));
-				assert.throws(() => new ModDependency("? my mod >= 1.2.3"));
+			it("should parse invalid prefix as the mod name", function() {
+				const dependencyInvalid = new ModDependency("& my-mod = 1.2.3");
+				assert.equal(dependencyInvalid.name, "& my-mod");
+				assert.equal(dependencyInvalid.type, "required");
+				assert.equal(dependencyInvalid.version.version, "1.2.3");
+
+				const dependencyDupe = new ModDependency("? ? my-mod = 1.2.3");
+				assert.equal(dependencyDupe.name, "? my-mod");
+				assert.equal(dependencyDupe.type, "optional");
+				assert.equal(dependencyDupe.version.version, "1.2.3");
 			});
 			it("should not error when the name contains spaces", function() {
 				const dependencyReq = new ModDependency("my mod");
@@ -86,6 +112,44 @@ describe("lib/data/ModInfo", function() {
 				assert.equal(dependencyOptVer.type, "optional");
 				assert.equal(dependencyOptVer.version.version, "1.2.3");
 			});
+			it("should not error when the extra spaces are present", function() {
+				const dependencyReq = new ModDependency("  my mod  ");
+				assert.equal(dependencyReq.name, "  my mod  ");
+				assert.equal(dependencyReq.type, "required");
+
+				const dependencyOpt = new ModDependency("?   my mod  ");
+				assert.equal(dependencyOpt.name, "my mod  ");
+				assert.equal(dependencyOpt.type, "optional");
+
+				const dependencyVer = new ModDependency("my mod   >=   1.2.3");
+				assert.equal(dependencyVer.name, "my mod");
+				assert.equal(dependencyVer.type, "required");
+				assert.equal(dependencyVer.version.version, "1.2.3");
+
+				const dependencyOptVer = new ModDependency("?   my mod   >=   1.2.3");
+				assert.equal(dependencyOptVer.name, "my mod");
+				assert.equal(dependencyOptVer.type, "optional");
+				assert.equal(dependencyOptVer.version.version, "1.2.3");
+			});
+			it("should not error when there are no spaces present", function() {
+				const dependencyReq = new ModDependency("my-mod");
+				assert.equal(dependencyReq.name, "my-mod");
+				assert.equal(dependencyReq.type, "required");
+
+				const dependencyOpt = new ModDependency("?my-mod");
+				assert.equal(dependencyOpt.name, "my-mod");
+				assert.equal(dependencyOpt.type, "optional");
+
+				const dependencyVer = new ModDependency("my-mod>=1.2.3");
+				assert.equal(dependencyVer.name, "my-mod");
+				assert.equal(dependencyVer.type, "required");
+				assert.equal(dependencyVer.version.version, "1.2.3");
+
+				const dependencyOptVer = new ModDependency("?my-mod>=1.2.3");
+				assert.equal(dependencyOptVer.name, "my-mod");
+				assert.equal(dependencyOptVer.type, "optional");
+				assert.equal(dependencyOptVer.version.version, "1.2.3");
+			});
 			it("should be round trip json serialisable", function() {
 				testRoundTripJsonSerialisable(ModDependency, testMatrix(
 					["my-mod", "? my-mod", "! my-mod", "my-mod > 1.0.0", "~ my-mod <= 2.0.0"], // Specification
@@ -271,7 +335,12 @@ describe("lib/data/ModInfo", function() {
 				is_deleted: true,
 			}));
 		});
-
+		it("should throw an error if given invalid dependencies", function() {
+			assert.throws(() => ModInfo.fromJSON({
+				name: "my-mod",
+				dependencies: ["base", ""],
+			}), /my-mod/); // check the mod name is included in the error message
+		});
 		it("should sort integer mod versions lexicographically", function() {
 			let unsortedVersions = ["1.0.0", "1.1.0", "0.1.0", "3.0.0", "1.2.0", "0.3.1", "0.3.3", "2.1.1", "0.0.1"];
 			let sortedVersions = ["0.0.1", "0.1.0", "0.3.1", "0.3.3", "1.0.0", "1.1.0", "1.2.0", "2.1.1", "3.0.0"];