Throw CycleException when cycles are detected

This changes the DependencyGraph's topological sort to throw a
dedicated exception class so that it can be better handled.

Author: JordonPhillips

smithy-build/src/main/java/software/amazon/smithy/build/SmithyBuildImpl.java

@@ -33,6 +33,7 @@
 import software.amazon.smithy.model.node.ObjectNode;
 import software.amazon.smithy.model.transform.ModelTransformer;
 import software.amazon.smithy.model.validation.ValidatedResult;
+import software.amazon.smithy.utils.CycleException;
 import software.amazon.smithy.utils.DependencyGraph;
 import software.amazon.smithy.utils.Pair;
 import software.amazon.smithy.utils.SmithyBuilder;
@@ -242,7 +243,7 @@ private List<ResolvedPlugin> resolvePlugins(String projectionName, ProjectionCon
         List<PluginId> sorted;
         try {
             sorted = dependencyGraph.toSortedList();
-        } catch (IllegalStateException e) {
+        } catch (CycleException e) {
             throw new SmithyBuildException(e.getMessage(), e);
         }
 

smithy-build/src/test/java/software/amazon/smithy/build/SmithyBuildTest.java

@@ -624,7 +624,7 @@ public void detectsPluginCycles() throws Exception {
                 .build());
 
         SmithyBuildException e = assertThrows(SmithyBuildException.class, builder::build);
-        assertThat(e.getMessage(), containsString("Cycle(s) detected in dependency graph"));
+        assertThat(e.getMessage(), containsString("Cycle(s) detected"));
     }
 
     @Test

smithy-build/src/test/java/software/amazon/smithy/build/TimestampPlugin1.java

@@ -20,6 +20,9 @@ public void execute(PluginContext context) {
         } catch (InterruptedException ignored) {}
     }
 
+    // This is made serial to protect against test failures if we decide later to
+    // have plugins run in parallel. These plugins MUST run serially with respect
+    // to each other to function.
     @Override
     public boolean isSerial() {
         return true;

smithy-build/src/test/java/software/amazon/smithy/build/TimestampPlugin2.java

@@ -27,6 +27,9 @@ public List<String> runBefore() {
         return ListUtils.of("timestampPlugin1");
     }
 
+    // This is made serial to protect against test failures if we decide later to
+    // have plugins run in parallel. These plugins MUST run serially with respect
+    // to each other to function.
     @Override
     public boolean isSerial() {
         return true;

smithy-build/src/test/java/software/amazon/smithy/build/TimestampPlugin3.java

@@ -27,6 +27,9 @@ public List<String> runAfter() {
         return ListUtils.of("timestampPlugin1");
     }
 
+    // This is made serial to protect against test failures if we decide later to
+    // have plugins run in parallel. These plugins MUST run serially with respect
+    // to each other to function.
     @Override
     public boolean isSerial() {
         return true;

smithy-codegen-core/src/main/java/software/amazon/smithy/codegen/core/IntegrationTopologicalSort.java

@@ -10,6 +10,7 @@
 import java.util.List;
 import java.util.Map;
 import java.util.logging.Logger;
+import software.amazon.smithy.utils.CycleException;
 import software.amazon.smithy.utils.DependencyGraph;
 
 final class IntegrationTopologicalSort<I extends SmithyIntegration<?, ?, ?>> {
@@ -46,7 +47,7 @@ List<I> sort() {
         List<String> sorted;
         try {
             sorted = dependencyGraph.toSortedList(this::compareIntegrations);
-        } catch (IllegalStateException e) {
+        } catch (CycleException e) {
             throw new IllegalArgumentException(e);
         }
         for (String name : sorted) {

smithy-utils/src/main/java/software/amazon/smithy/utils/CycleException.java

@@ -0,0 +1,80 @@
+/*
+ * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
+ * SPDX-License-Identifier: Apache-2.0
+ */
+package software.amazon.smithy.utils;
+
+import java.util.List;
+import java.util.Set;
+import java.util.stream.Collectors;
+
+/**
+ * Signals that one or more cycles have been detected when attempting to topologically
+ * sort shapes in a {@link DependencyGraph}.
+ */
+public class CycleException extends RuntimeException {
+    private final List<?> sortedNodes;
+    private final Set<?> cyclicNodes;
+    private final Class<?> nodeType;
+
+    /**
+     * Constructs a CycleException.
+     *
+     * @param sortedNodes A list of the nodes that were sorted successfully.
+     * @param cyclicNodes A set of nodes that could not be sorted due to being part of a cycle.
+     * @param <T> The type of the node.
+     */
+    public <T> CycleException(List<T> sortedNodes, Set<T> cyclicNodes) {
+        super(String.format("Cycle(s) detected amongst: [%s]",
+                cyclicNodes.stream().map(Object::toString).collect(Collectors.joining(", "))));
+        this.sortedNodes = ListUtils.copyOf(sortedNodes);
+        this.cyclicNodes = SetUtils.orderedCopyOf(cyclicNodes);
+        if (this.cyclicNodes.isEmpty()) {
+            throw new IllegalArgumentException("Cyclic nodes cannot be empty");
+        }
+        this.nodeType = this.cyclicNodes.iterator().next().getClass();
+    }
+
+    /**
+     * Gets the set of nodes that are part of a cycle.
+     *
+     * <p>This contains all nodes that are a part of any cycles. To see a list of
+     * individual cycles, use {@link DependencyGraph#findCycles()}.
+     *
+     * @param expectedNodeType The expected type of the node, which will be checked to
+     *        be compatible with the actual type. This is necessary because
+     *        exceptions can't be generic.
+     * @return Returns a set of cyclic nodes.
+     * @param <T> The type of the graph's nodes.
+     */
+    @SuppressWarnings("unchecked")
+    public <T> Set<T> getCyclicNodes(Class<T> expectedNodeType) {
+        if (expectedNodeType.isAssignableFrom(this.nodeType)) {
+            return (Set<T>) cyclicNodes;
+        }
+        throw new IllegalArgumentException(String.format(
+                "Expected node type %s is not assignable from actual node type %s",
+                expectedNodeType.getName(),
+                this.nodeType.getName()));
+    }
+
+    /**
+     * Gets the list of nodes that could be sorted.
+     *
+     * @param expectedNodeType The expected type of the node, which will be checked to
+     *        be compatible with the actual type. This is necessary because
+     *        exceptions can't be generic.
+     * @return Returns the sorted list of non-cyclic nodes.
+     * @param <T> The type of the graph's nodes.
+     */
+    @SuppressWarnings("unchecked")
+    public <T> List<T> getSortedNodes(Class<T> expectedNodeType) {
+        if (expectedNodeType.isAssignableFrom(this.nodeType)) {
+            return (List<T>) sortedNodes;
+        }
+        throw new IllegalArgumentException(String.format(
+                "Expected node type %s is not assignable from actual node type %s",
+                expectedNodeType.getName(),
+                this.nodeType.getName()));
+    }
+}

smithy-utils/src/main/java/software/amazon/smithy/utils/DependencyGraph.java

@@ -349,15 +349,13 @@ private List<T> toSortedList(Queue<T> satisfied) {
 
         // Check for cycles.
         if (result.size() != reverseDependencies.size()) {
-            List<String> remaining = new ArrayList<>(reverseDependencies.size() - result.size());
+            Set<T> remaining = new LinkedHashSet<>(reverseDependencies.size() - result.size());
             for (T node : reverseDependencies.keySet()) {
                 if (!result.contains(node)) {
-                    remaining.add(node.toString());
+                    remaining.add(node);
                 }
             }
-            throw new IllegalStateException(
-                    String.format("Cycle(s) detected in dependency graph while attempting to sort among [%s]",
-                            String.join(", ", remaining)));
+            throw new CycleException(result, remaining);
         }
 
         return result;
@@ -402,4 +400,5 @@ public void clear() {
         forwardDependencies.clear();
         reverseDependencies.clear();
     }
+
 }

smithy-utils/src/test/java/software/amazon/smithy/utils/CycleExceptionTest.java

@@ -0,0 +1,33 @@
+/*
+ * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
+ * SPDX-License-Identifier: Apache-2.0
+ */
+package software.amazon.smithy.utils;
+
+import static org.junit.jupiter.api.Assertions.assertThrows;
+
+import java.util.Collections;
+import java.util.List;
+import java.util.Set;
+import org.junit.jupiter.api.Test;
+
+public class CycleExceptionTest {
+    @Test
+    public void requiresCycles() {
+        assertThrows(IllegalArgumentException.class,
+                () -> new CycleException(Collections.emptyList(), Collections.emptySet()));
+    }
+
+    @Test
+    public void checksNodeTypes() {
+        CycleException cycleException = new CycleException(ListUtils.of("foo"), SetUtils.of("bar"));
+        List<String> sortedStrings = cycleException.getSortedNodes(String.class);
+        Set<String> cyclicStrings = cycleException.getCyclicNodes(String.class);
+
+        List<Object> sortedObjects = cycleException.getSortedNodes(Object.class);
+        Set<Object> cyclicObjects = cycleException.getCyclicNodes(Object.class);
+
+        assertThrows(IllegalArgumentException.class, () -> cycleException.getSortedNodes(Integer.class));
+        assertThrows(IllegalArgumentException.class, () -> cycleException.getCyclicNodes(Integer.class));
+    }
+}

smithy-utils/src/test/java/software/amazon/smithy/utils/DependencyGraphTest.java

@@ -230,10 +230,14 @@ public void topologicallySortsWithCustomComparator() {
     @Test()
     public void sortedListThrowsErrorOnCycle() {
         DependencyGraph<String> graph = new DependencyGraph<>();
+        graph.add("foo");
+        graph.addDependency("bar", "foo");
         graph.addDependency("spam", "eggs");
         graph.addDependency("eggs", "spam");
 
-        assertThrows(IllegalStateException.class, graph::toSortedList);
+        CycleException exception = assertThrows(CycleException.class, graph::toSortedList);
+        assertThat(exception.getSortedNodes(String.class), contains("foo", "bar"));
+        assertThat(exception.getCyclicNodes(String.class), containsInAnyOrder("spam", "eggs"));
     }
 
     @Test