Add better parchment version validation

.gitignore

@@ -3,7 +3,6 @@
 /**/.project
 /**/.settings/
 /**/bin/
-/**/test/
 /**/build/
 /repo/
 /cache/

build.gradle

@@ -34,6 +34,11 @@ dependencies {
     implementation libs.commonsio
 
     implementation libs.bundles.utils
+
+    // This is explicitly NOT a JPMS project, so we can use a test souceset and Eclipse is happy
+    // If we ever add JPMS, this must be split into a sub-project.
+    testImplementation testLibs.junit.api
+    testRuntimeOnly testLibs.bundles.junit.runtime
 }
 
 license {

settings.gradle

@@ -45,5 +45,17 @@ dependencyResolutionManagement.versionCatalogs.register('libs') {
     library 'utils-logging',  'net.minecraftforge', 'log-utils'       version '0.5.0'
     library 'utils-os',       'net.minecraftforge', 'os-utils'        version '0.1.0'
     bundle 'utils', ['utils-download', 'utils-files', 'utils-hash', 'utils-data', 'utils-logging', 'utils-os']
+
+
+}
+
+dependencyResolutionManagement.versionCatalogs.register('testLibs') {
+    description = 'Dependencies used exclusively for testing or test projects.'
+
+    version 'junit', '5.10.2'
+    library 'junit-api',               'org.junit.jupiter',  'junit-jupiter-api'       versionRef 'junit'
+    library 'junit-engine',            'org.junit.jupiter',  'junit-jupiter-engine'    versionRef 'junit'
+    library 'junit-platform-launcher', 'org.junit.platform', 'junit-platform-launcher' version    '1.10.2'
+    bundle 'junit-runtime', ['junit-engine', 'junit-platform-launcher']
 }
 //formatter:on

src/main/java/net/minecraftforge/mcmaven/impl/mappings/Mappings.java

@@ -127,8 +127,8 @@ public Artifact getArtifact(MCPSide side) {
         var mcpVersion = side.getMCP().getName().getVersion();
         var mcVersion = side.getMCP().getConfig().version;
         var artifactVersion = mcpVersion;
-        if (this.version != null && !mcVersion.equals(this.version))
-            artifactVersion = mcpVersion + '-' + this.version;
+        if (this.version() != null && !mcVersion.equals(this.version()))
+            artifactVersion += '-' + this.version();
 
         return Artifact.from(Constants.MC_GROUP, "mappings_" + this.channel, artifactVersion)
             .withExtension("zip");

src/main/java/net/minecraftforge/mcmaven/impl/mappings/ParchmentMappings.java

@@ -27,20 +27,22 @@
 import net.minecraftforge.mcmaven.impl.repo.mcpconfig.MCPSide;
 import net.minecraftforge.mcmaven.impl.repo.mcpconfig.MinecraftTasks;
 import net.minecraftforge.mcmaven.impl.util.Artifact;
-import net.minecraftforge.mcmaven.impl.util.Constants;
 import net.minecraftforge.mcmaven.impl.util.Task;
 import net.minecraftforge.mcmaven.impl.util.Util;
 import net.minecraftforge.srgutils.IMappingFile;
 import net.minecraftforge.util.file.FileUtils;
 import net.minecraftforge.util.hash.HashStore;
 
 public class ParchmentMappings extends Mappings {
+	private final ParchmentVersion parsedVersion;
     private Task downloadTask;
 
     public ParchmentMappings(String version) {
-        super("parchment", Objects.requireNonNull(version, "Parchment mappings version must be present"));
-        if (version.contains("-SNAPSHOT"))
-            throw new IllegalArgumentException("Parchment snapshots are not supported: " + version);
+    	this(ParchmentVersion.parse(version));
+    }
+    private ParchmentMappings(ParchmentVersion version) {
+        super("parchment", version.toFriendly());
+        this.parsedVersion = version;
     }
 
     @Override
@@ -51,9 +53,18 @@ public boolean isPrimary() {
     // Maybe download the maven-metadata.xml for the MC version and pick the latest one?
     @Override
     public Mappings withMCVersion(String mcVer) {
-        if (this.version().indexOf('-') != -1) // assume our version specifies the MC version
-            return this;
-        return new ParchmentMappings(mcVer + '-' + this.version());
+    	if (mcVer == null)
+    		throw new IllegalArgumentException("Minecraft Version can not be null");
+
+    	if (mcVer.equals(this.parsedVersion.mcVersion()))
+    		return this;
+
+    	return new ParchmentMappings(this.parsedVersion.withMinecraft(mcVer));
+    }
+
+    @Override
+    public Artifact getArtifact(MCPSide side) {
+    	return this.parsedVersion.getMappingArtifact(side.getMCP().getName().getVersion());
     }
 
     @Override
@@ -88,13 +99,8 @@ private Task downloadTask(MCP mcp) {
     }
 
     private File download(Cache cache) {
-        var maven = new MavenCache("parchment", Constants.PARCHMENT_MAVEN, cache.root());
-        var idx = version().indexOf('-');
-        if (idx == -1)
-            throw new IllegalStateException("Unknown Parchment version: " + version());
-        var mcversion = version().substring(0, idx);
-        var ver = version().substring(idx + 1);
-        var artifact = Artifact.from(Constants.PARCHMENT_GROUP, "parchment-" + mcversion, ver, "checked").withExtension("zip");
+        var maven = new MavenCache("parchment", ParchmentVersion.PARCHMENT_MAVEN, cache.root());
+        var artifact = this.parsedVersion.getArtifact();
         return maven.download(artifact);
     }
 
@@ -105,7 +111,7 @@ private File getMappings(MCP mcp, Task srgTask, Task clientTask, Task serverTask
         var data = dataTask.execute();
 
         var root = getFolder(new File(mcp.getBuildFolder(), "data/mapings"));
-        var output = new File(root, "parchment" + version() + ".zip");
+        var output = new File(root, "parchment-" + version() + ".zip");
         var cache = HashStore.fromFile(output)
             .add("srg", srg)
             .add("client", client)

src/main/java/net/minecraftforge/mcmaven/impl/mappings/ParchmentVersion.java

@@ -0,0 +1,169 @@
+/*
+ * Copyright (c) Forge Development LLC and contributors
+ * SPDX-License-Identifier: LGPL-2.1-only
+ */
+package net.minecraftforge.mcmaven.impl.mappings;
+
+import java.util.regex.Pattern;
+
+import net.minecraftforge.mcmaven.impl.util.Artifact;
+import net.minecraftforge.mcmaven.impl.util.Constants;
+
+public record ParchmentVersion(
+    String timestamp,
+    /* @Nullable */ String mcVersion,
+    /* @Nullable */ String mapMcVersion
+) {
+    private static final Pattern TIMESTAMP = Pattern.compile("\\d{4}.\\d{2}.\\d{2}");
+    private static final Pattern MCP_TIMESTAMP = Pattern.compile("\\d{8}.\\d{6}");
+
+    // Parchment related things
+    public static final String PARCHMENT_MAVEN = "https://maven.parchmentmc.org/";
+    private static final String PARCHMENT_GROUP = "org.parchmentmc.data"; // Name is "parchment-{mcversion}'
+
+    /**
+     * <pre>
+     * Parchment names can be specified in many variants, including some 'shorthand's
+     * Basing this implementation on https://parchmentmc.org/docs/getting-started
+     *
+     * Namely, (in case they change the site)
+     *
+     * For using Parchment on the same version of Minecraft:
+     *    YYYY.MM.DD-&lt;Environment MC version>
+     *    Examples:
+     *      2021.12.12-1.17.1 for Minecraft 1.17.1
+     *      2022.08.07-1.18.2 for Minecraft 1.18.2
+     *      2022.08.14-1.19.2 for Minecraft 1.19.2
+     *
+     * For using Parchment for an older version on a newer MC version
+     * 	  &lt;Mapping's MC version>-YYYY.MM.DD-&lt;Environment MC version>
+     *	  Examples:
+     *      1.17.1-2021.12.12-1.18
+     *          Minecraft 1.17.1 mappings (2021.12.12-1.17.1) in an MC 1.18 environment
+     *      1.18.2-2022.08.07-1.19.1
+     *          Minecraft 1.18.2 mappings (2021.08.07-1.18.2) in an MC 1.19.1 environment
+     *      1.19.2-2022.08.14-1.20
+     *          Minecraft 1.19.2 mappings (2021.08.14-1.19.2) in an MC 1.20 environment
+     *
+     * Parchment also publishes 'snapshots', These are not supported by MCMaven, and as such will
+     * throw an exception during parsing.
+     *     Example:
+     *       parchment-1.21.9/2025.10.05-nightly-SNAPSHOT
+     *       parchment-1.16.5/BLEEDING-SNAPSHOT
+     * They have also published two known versions that don't follow the standard format. And I don't feel like supporting it.
+     *     parchment-1.16.5/20210607-SNAPSHOT
+     *     parchment-1.16.5/20210608-SNAPSHOT
+     *
+     * Tho undocumented, the implementation of Parchment's FG6 plugin supported specifying the MCP timestamp as part of the Environment MC version
+     * https://github.com/ParchmentMC/Librarian/blob/c7f9878feb76d210aa65569fe38cad5297f439c5/src/main/java/org/parchmentmc/librarian/forgegradle/ParchmentMappingVersion.java#L52
+     *
+     * In addition to the above formats I DO support not specifying a Minecraft version. It will need to be filled in later
+     * to find the correct download, but typically we can pull it from MCPConfig/other context.
+     *
+     * Note: This does not do any case sanitization
+     *
+     * @throws IllegalArgumentExcetion if the version fails to parse
+     */
+    public static ParchmentVersion parse(String version) {
+        if (version == null)
+            throw new IllegalArgumentException("Parchment mappings version must be present");
+
+    	if (version.contains("-SNAPSHOT"))
+            throw new IllegalArgumentException("Parchment snapshots are not supported: " + version);
+
+    	var matcher = TIMESTAMP.matcher(version);
+    	if (!matcher.find())
+            throw new IllegalArgumentException("Parchment version does not contain a timestamp: " + version);
+
+    	int start = matcher.start();
+    	int end = matcher.end();
+
+    	// Simple case, we just specify the timestamp
+    	if (start == 0 && end == version.length())
+    		return new ParchmentVersion(version, null, null);
+
+		var timestamp = matcher.group();
+    	String mcVersion = null;
+
+    	// Minecraft Version is specified
+    	if (end < version.length()) {
+    		if (version.charAt(end) != '-' || end == version.length() - 1)
+    			throw new IllegalArgumentException("Parchment version does not specify Minecraft version: " + version);
+    		mcVersion = version.substring(end + 1);
+    	}
+
+    	// Default mapping version is the same as mc version
+    	var mapMcVersion = stripMcp(mcVersion);
+
+    	// Mappings MC version is specified
+    	if (start > 0) {
+    		if (version.charAt(start - 1) != '-' || start == 1)
+    			throw new IllegalArgumentException("Parchment version does not specify Mapping Minecraft version: " + version);
+    		mapMcVersion = version.substring(0, start - 1);
+    	}
+
+    	return new ParchmentVersion(timestamp, mcVersion, mapMcVersion);
+    }
+
+    /**
+     * Variant of {@link #parse(String)} that returns null instead of throwing IllegalArgumentException
+     */
+    public static ParchmentVersion tryParse(String version) {
+    	try {
+    		return parse(version);
+    	} catch (IllegalArgumentException e) {
+    		return null;
+    	}
+    }
+
+	// Remove MCP timestamp if its on there.
+    private static String stripMcp(String version) {
+    	if (version == null)
+    		return null;
+    	if (version.length() <= 17 || version.charAt(version.length() - 16) != '-')
+    		return version;
+
+    	var matcher = MCP_TIMESTAMP.matcher(version);
+    	if (matcher.find() && matcher.start() == version.length() - 15)
+    		return version.substring(0, version.length() - 16);
+
+    	return version;
+    }
+
+    public ParchmentVersion withMinecraft(String mcVersion) {
+    	return new ParchmentVersion(timestamp, mcVersion, mapMcVersion == null ? mcVersion : mapMcVersion);
+    }
+
+    public String toFriendly() {
+    	if (mapMcVersion == null) {
+    		if (mcVersion == null)
+    			return timestamp;
+    		return timestamp + '-' + mcVersion;
+    	}
+
+    	if (mcVersion == null)
+    		return mapMcVersion + '-' + timestamp;
+
+    	if (mapMcVersion.equals(stripMcp(mcVersion)))
+    		return timestamp + '-' + mcVersion;
+
+    	return mapMcVersion + '-' + timestamp + '-' + mcVersion;
+    }
+
+    public Artifact getArtifact() {
+        if (mapMcVersion == null)
+            throw new IllegalStateException("Unknown Parchment version: " + timestamp);
+        return Artifact.from(PARCHMENT_GROUP, "parchment-" + mapMcVersion, timestamp, "checked").withExtension("zip");
+    }
+
+    public Artifact getMappingArtifact(String mcpVersion) {
+        //net.minecraft:mappings_parchment:{MCP_VERSION}-TIMESTAMP[-{MAPPING_MC_VERSION}]@zip
+        var mcVersion = stripMcp(mcpVersion);
+        var artifactVersion = mcpVersion + '-' + timestamp;
+        if (mapMcVersion != null && !mcVersion.equals(mapMcVersion))
+        	artifactVersion += '-' + mapMcVersion;
+
+        return Artifact.from(Constants.MC_GROUP, "mappings_parchment", artifactVersion)
+            .withExtension("zip");
+    }
+}

src/main/java/net/minecraftforge/mcmaven/impl/util/Constants.java

@@ -15,10 +15,6 @@ public final class Constants {
     public static final String FORGE_ARTIFACT = FORGE_GROUP + ':' + FORGE_NAME;
     public static final String FORGE_MAVEN = "https://maven.minecraftforge.net/";
 
-    // Parchment related things
-    public static final String PARCHMENT_MAVEN = "https://maven.parchmentmc.org/";
-    public static final String PARCHMENT_GROUP = "org.parchmentmc.data"; // Name is "parchment-{mcversion}'
-
     // TODO Other toolchains such as FMLOnly (not required, but would be useful so we have the framework to use other toolchains)
     public static final String FMLONLY_NAME = "fmlonly";
     public static final String FMLONLY_ARTIFACT = FORGE_GROUP + ':' + FMLONLY_NAME;

src/test/java/net/minecraftforge/mcmaven/test/ParchmentVersionTests.java

@@ -0,0 +1,73 @@
+/*
+ * Copyright (c) Forge Development LLC and contributors
+ * SPDX-License-Identifier: LGPL-2.1-only
+ */
+package net.minecraftforge.mcmaven.test;
+
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.Test;
+
+import net.minecraftforge.mcmaven.impl.mappings.ParchmentVersion;
+
+public class ParchmentVersionTests {
+	@Test
+	public void parsing() {
+		parses("2026.01.01", null, "2026.01.01", null);
+		parses("2026.01.01-1.12-pre1", "1.12-pre1", "2026.01.01", "1.12-pre1");
+		parses("1.12.2-2026.01.01-1.13", "1.12.2", "2026.01.01", "1.13");
+		fails("2026.01.01-nightly-SNAPSHOT");
+		fails("BLEEDING-SNAPSHOT-1.12");
+		// Propagates MC version, but not MCP Version
+		parses("2026.01.01-1.12-20200101.000000", "1.12", "2026.01.01", "1.12-20200101.000000");
+	}
+
+	private void parses(String version, String mapMcVersion, String timestamp, String mcVersion) {
+		var parsed = ParchmentVersion.parse(version);
+		if (mapMcVersion == null)
+			Assertions.assertNull(parsed.mapMcVersion(), "Map MC Version was not null for " + version);
+		else
+			Assertions.assertEquals(mapMcVersion, parsed.mapMcVersion(), "Map MC Version did not match for " + version);
+
+		Assertions.assertEquals(timestamp, parsed.timestamp(), "Timestamp did not parse correctly: " + version);
+
+		if (mcVersion == null)
+			Assertions.assertNull(parsed.mcVersion(), "MC Version was not null for " + version);
+		else
+			Assertions.assertEquals(mcVersion, parsed.mcVersion(), "MC Version did not match for " + version);
+	}
+
+	private void fails(String version) {
+		Assertions.assertThrows(IllegalArgumentException.class, () -> ParchmentVersion.parse(version));
+	}
+
+	@Test
+	public void withMinecraft() {
+		// Should propagate to mapping version
+		var version = ParchmentVersion.parse("2026.01.01").withMinecraft("1.12");
+		Assertions.assertEquals(version.mapMcVersion(), "1.12");
+		Assertions.assertEquals(version.mcVersion(), "1.12");
+
+		// Should NOT propagate to mapping version
+		version = ParchmentVersion.parse("2026.01.01-1.6").withMinecraft("1.12");
+		Assertions.assertEquals(version.mapMcVersion(), "1.6");
+		Assertions.assertEquals(version.mcVersion(), "1.12");
+	}
+
+	@Test
+	public void friendly() {
+		friendly("2026.01.01");
+		friendly("1.12-2026.01.01");
+		friendly("2026.01.01-1.12");
+		friendly("1.12-2026.01.01-1.12", "2026.01.01-1.12");
+		friendly("1.12-2026.01.01-1.12-20200101.000000", "2026.01.01-1.12-20200101.000000");
+	}
+
+	private void friendly(String version) {
+		friendly(version, version);
+	}
+
+	private void friendly(String version, String friendly) {
+		var parsed = ParchmentVersion.parse(version);
+		Assertions.assertEquals(friendly, parsed.toFriendly());
+	}
+}