Make Error Messages Clearer

src/main/java/org/quiltmc/loader/api/plugin/ModLocation.java

@@ -17,6 +17,7 @@
 package org.quiltmc.loader.api.plugin;
 
 import org.jetbrains.annotations.ApiStatus;
+import org.jetbrains.annotations.Nullable;
 import org.quiltmc.loader.api.plugin.solver.ModLoadOption;
 import org.quiltmc.loader.impl.util.QuiltLoaderInternal;
 import org.quiltmc.loader.impl.util.QuiltLoaderInternalType;
@@ -29,8 +30,16 @@ public interface ModLocation {
 	/** @return True if the mod is directly on the classpath, otherwise false. */
 	boolean onClasspath();
 
+	/**
+	 * @return the containing mod option for this location. If the method returns {@code null}, the mod is not contained within
+	 * 		   another mod
+	 */
+	@Nullable ModLoadOption containingOption();
+
 	/** @return True if the mod was directly scanned as a file or folder, rather than being scanned as a sub-mod.
 	 *         This also returns true if {@link #onClasspath()} returns true.
 	 *         Influences {@link ModLoadOption#isMandatory()} in the built-in plugins. */
-	boolean isDirect();
+	default boolean isDirect() {
+		return this.containingOption() == null;
+	}
 }

src/main/java/org/quiltmc/loader/api/plugin/QuiltPluginContext.java

@@ -20,6 +20,7 @@
 import java.util.concurrent.Callable;
 
 import org.jetbrains.annotations.ApiStatus;
+import org.jetbrains.annotations.Nullable;
 import org.quiltmc.loader.api.gui.QuiltDisplayedError;
 import org.quiltmc.loader.api.gui.QuiltLoaderText;
 import org.quiltmc.loader.api.gui.QuiltTreeNode;
@@ -29,6 +30,7 @@
 import org.quiltmc.loader.api.plugin.solver.Rule;
 import org.quiltmc.loader.api.plugin.solver.RuleContext;
 import org.quiltmc.loader.api.plugin.solver.TentativeLoadOption;
+import org.quiltmc.loader.impl.plugin.quilt.QuiltModOption;
 import org.quiltmc.loader.impl.util.QuiltLoaderInternal;
 import org.quiltmc.loader.impl.util.QuiltLoaderInternalType;
 
@@ -75,9 +77,20 @@ public interface QuiltPluginContext {
 	 * or vice versa. Or any mod type of which a loader plugin can load).
 	 * 
 	 * @param guiNode The GUI node to display the loaded mod details under
-	 * @param direct True if the file is directly loaded rather than being included in another mod (see {@link ModLocation#isDirect()}) */
+	 * @param direct True if the file is directly loaded rather than being included in another mod (see {@link ModLocation#isDirect()})
+	 * @deprecated Please use {@link #addFileToScan(Path, QuiltTreeNode, ModLoadOption)}
+	 * 			   as specifying the containing option improves error messages */
+	@Deprecated
 	void addFileToScan(Path file, QuiltTreeNode guiNode, boolean direct);
 
+	/** Adds an additional file to scan for mods, which will go through the same steps as files found in mod folders.
+	 * (This is more flexible than loading files manually, since it allows fabric mods to be jar-in-jar'd in quilt mods,
+	 * or vice versa. Or any mod type of which a loader plugin can load).
+	 *
+	 * @param guiNode The GUI node to display the loaded mod details under
+	 * @param containing The {@link ModLoadOption} that contains the file to scan, null if it is not contained. */
+	void addFileToScan(Path file, QuiltTreeNode guiNode, @Nullable ModLoadOption containing);
+
 	/** Adds an additional folder to scan for mods, which will be treated in the same way as the regular mods folder.
 	 *
 	 * @return true if the given folder is a new folder, or false if it has already been added and scanned before. */

src/main/java/org/quiltmc/loader/api/plugin/solver/ModLoadOption.java

@@ -138,6 +138,8 @@ public void populateModsTabInfo(PluginGuiTreeNode guiNode) {
 
 	public abstract ModContainerExt convertToMod(Path transformedResourceRoot);
 
+	public abstract @Nullable ModLoadOption getContainingMod();
+
 	@Override
 	public String toString() {
 		return shortString();

src/main/java/org/quiltmc/loader/impl/discovery/ArgumentModCandidateFinder.java

@@ -123,7 +123,7 @@ private static void addMod(QuiltPluginContext ctx, String pathStr, String origin
 				error.appendReportText(" (Inside the file " + source + ")");
 			}
 		} else {
-			ctx.addFileToScan(path, argModsNode.addChild(QuiltLoaderText.of(pathStr)), true);
+			ctx.addFileToScan(path, argModsNode.addChild(QuiltLoaderText.of(pathStr)).getNew(), null);
 		}
 	}
 }

src/main/java/org/quiltmc/loader/impl/plugin/BasePluginContext.java

@@ -72,13 +72,19 @@ public String toString() {
 	@Deprecated
 	public void addFileToScan(Path file, PluginGuiTreeNode guiNode, boolean direct) {
 		// TODO: Log / store / do something to store the plugin
-		manager.scanModFile(file, new ModLocationImpl(false, direct), (QuiltStatusNode) guiNode);
+		manager.scanModFile(file, new ModLocationImpl(false, direct, null), (QuiltStatusNode) guiNode);
 	}
 
 	@Override
 	public void addFileToScan(Path file, QuiltTreeNode guiNode, boolean direct) {
 		// TODO: Log / store / do something to store the plugin
-		manager.scanModFile(file, new ModLocationImpl(false, direct), (QuiltStatusNode) guiNode);
+		manager.scanModFile(file, new ModLocationImpl(false, direct, null), (QuiltStatusNode) guiNode);
+	}
+
+	@Override
+	public void addFileToScan(Path file, QuiltTreeNode guiNode, ModLoadOption containingOption) {
+		// TODO: Log / store / do something to store the plugin
+		manager.scanModFile(file, new ModLocationImpl(false, containingOption == null, containingOption), (QuiltStatusNode) guiNode);
 	}
 
 	@Override

src/main/java/org/quiltmc/loader/impl/plugin/ModLocationImpl.java

@@ -16,18 +16,23 @@
 
 package org.quiltmc.loader.impl.plugin;
 
+import org.jetbrains.annotations.Nullable;
 import org.quiltmc.loader.api.plugin.ModLocation;
+import org.quiltmc.loader.api.plugin.solver.ModLoadOption;
 import org.quiltmc.loader.impl.util.QuiltLoaderInternal;
 import org.quiltmc.loader.impl.util.QuiltLoaderInternalType;
 
 @QuiltLoaderInternal(QuiltLoaderInternalType.NEW_INTERNAL)
 public final class ModLocationImpl implements ModLocation {
 
 	private final boolean classpath, direct;
+	@Nullable
+	private final ModLoadOption containingOption;
 
-	ModLocationImpl(boolean classpath, boolean direct) {
+	ModLocationImpl(boolean classpath, boolean direct, @Nullable ModLoadOption containingOption) {
 		this.classpath = classpath;
 		this.direct = direct;
+		this.containingOption = containingOption;
 	}
 
 	@Override
@@ -39,4 +44,9 @@ public boolean onClasspath() {
 	public boolean isDirect() {
 		return direct;
 	}
+
+	@Override
+	public @Nullable ModLoadOption containingOption() {
+		return containingOption;
+	}
 }

src/main/java/org/quiltmc/loader/impl/plugin/QuiltPluginManagerImpl.java

@@ -44,6 +44,7 @@
 import java.util.Queue;
 import java.util.Set;
 import java.util.SortedMap;
+import java.util.Stack;
 import java.util.TreeMap;
 import java.util.concurrent.Callable;
 import java.util.concurrent.ConcurrentLinkedQueue;
@@ -959,8 +960,8 @@ private void appendModTable(Consumer<String> to) {
 			// - loader plugin
 			// - source path(s)
 			row.put(modColumn, mod.metadata().name());
-			row.put(id, mod.metadata().id());
-			row.put(version, mod.metadata().version());
+			row.put(id, mod.id());
+			row.put(version, mod.version());
 			row.put(plugin, mod.loader().pluginId());
 			StringBuilder flagStr = new StringBuilder();
 			flagStr.append(theQuiltPlugin.hasDepsChanged(mod) ? QuiltLoaderImpl.FLAG_DEPS_CHANGED : '.');
@@ -1288,7 +1289,7 @@ private void scanClasspath() {
 				if (FasterFiles.isDirectory(path)) {
 					clNode.icon(QuiltLoaderGui.iconFolder());
 				}
-				scanModFile(path, new ModLocationImpl(true, true), clNode);
+				scanModFile(path, new ModLocationImpl(true, true, null), clNode);
 			}
 		});
 	}
@@ -1380,8 +1381,16 @@ private ModSolveResultImpl runSingleCycle() throws ModResolutionException, Timeo
 	private void handleSolverFailure() throws TimeoutException, ModSolvingError {
 
 		SolverErrorHelper helper = new SolverErrorHelper(this);
-		boolean failed = false;
 
+		// Storage for a complete traversal of the rule graph
+		// In order to make sure we detect all errors, we need to test removing each rule individually
+		// This gives an O(n!) time complexity for the do while, but this code only runs once and the set of breaking rules
+		// isn't that large.
+		Stack<Rule> removedRules = new Stack<>();
+		Map<Integer, Set<Rule>> seen = new HashMap<>();
+		seen.put(0, new HashSet<>());
+
+		boolean failed = false;
 		solver_error_iteration: do {
 			Collection<Rule> rules = solver.getError();
 
@@ -1433,28 +1442,43 @@ private void handleSolverFailure() throws TimeoutException, ModSolvingError {
 			}
 
 			// No plugin blamed any rules
-			// So we'll just pick one of them randomly and remove it.
-
 			failed = true;
-			helper.reportSolverError(rules);
-
-			Rule pickedRule = rules.stream().filter(r -> r instanceof QuiltRuleBreak).findAny().orElse(null);
 
-			if (pickedRule == null) {
-				pickedRule = rules.stream().filter(r -> r instanceof QuiltRuleDep).findAny().orElse(null);
-			}
+			// Report the current error
+			helper.reportSolverError(rules);
 
-			if (pickedRule == null) {
-				pickedRule = rules.stream().filter(r -> !(r instanceof ModIdDefinition)).findAny().orElse(null);
+			// Pick a rule that we haven't removed yet and remove it.
+			Rule pickedRule = null;
+			for (Rule next : rules) {
+				if (seen.get(removedRules.size()).add(next)) {
+					pickedRule = next;
+					break;
+				}
 			}
 
 			if (pickedRule == null) {
-				pickedRule = rules.iterator().next();
+				if (removedRules.isEmpty()) { // We have checked every rule from the initial error
+					break;
+				} else { // There are no more rules to remove on this branch
+					Rule reAdd = removedRules.pop();
+					solver.redefine(reAdd);
+					solver.hasSolution(); // We know this is false, don't care about the result
+					continue;
+				}
 			}
 
+			// Remove the rule from the solver and go down to the next level.
 			solver.removeRule(pickedRule);
+			removedRules.push(pickedRule);
+			seen.put(removedRules.size(), new HashSet<>());
 
-		} while (!solver.hasSolution());
+			// Removing this rule fixes everything, so we can skip the branch, but there might be more errors
+			if (solver.hasSolution()) {
+				Rule reAdd = removedRules.pop();
+				solver.redefine(reAdd);
+				solver.hasSolution(); // We know this is false, don't care about the result
+			}
+		} while (true);
 
 		helper.reportErrors();
 
@@ -1828,7 +1852,7 @@ public FileVisitResult preVisitDirectory(Path dir, BasicFileAttributes attrs) th
 					Path relative = path.relativize(dir);
 					int count = relative.getNameCount();
 					if (isTest() && dir.getFileName().toString().endsWith(".jar")) {
-						ModLocationImpl location = new ModLocationImpl(false, true);
+						ModLocationImpl location = new ModLocationImpl(false, true, null);
 						// Tests use mods-as-folders to allow them to be entered into the git history properly
 						scanModFile(dir, location, guiNode.addChild(QuiltLoaderText.of(dir.getFileName().toString())));
 						return FileVisitResult.SKIP_SUBTREE;
@@ -1943,7 +1967,7 @@ public FileVisitResult visitFile(Path file, BasicFileAttributes attrs) throws IO
 						node = guiNode.addChild(QuiltLoaderText.translate("gui.prefix.no_parent_file"), SortOrder.ALPHABETICAL_ORDER);
 					}
 
-					scanModFile(file, new ModLocationImpl(false, true), node);
+					scanModFile(file, new ModLocationImpl(false, true, null), node);
 					return FileVisitResult.CONTINUE;
 				}
 			});