Improve Program Circular Include Message (#273)

Adding more info in the exception to make identifying the circular include more easily.

thrifty-schema/src/main/kotlin/com/microsoft/thrifty/schema/Loader.kt

@@ -164,9 +164,9 @@ class Loader {
         }
 
         // Link included programs together
-        val visited = HashSet<Program>(loadedPrograms.size)
+        val visited = HashMap<Program, Program?>(loadedPrograms.size)
         for (program in loadedPrograms.values) {
-            program.loadIncludedPrograms(this, visited)
+            program.loadIncludedPrograms(this, visited, null)
         }
     }
 

thrifty-schema/src/main/kotlin/com/microsoft/thrifty/schema/Program.kt

@@ -117,24 +117,37 @@ class Program internal constructor(element: ThriftFileElement) {
     /**
      * Loads this program's symbol table and list of included Programs.
      * @param loader
-     * @param visited
+     * @param visited A [MutableMap] used to track a parent [Program], if it was visited from one.
+     * @param parent The parent [Program] that is including this [Program],
+     * `null` if this [Program] is not being loaded from another [Program].
      */
-    internal fun loadIncludedPrograms(loader: Loader, visited: MutableSet<Program>) {
-        if (!visited.add(this)) {
+    internal fun loadIncludedPrograms(loader: Loader, visited: MutableMap<Program, Program?>, parent: Program?) {
+        if (visited.containsKey(this)) {
             if (includedPrograms == null) {
-                loader.errorReporter().error(location, "Circular include; file includes itself transitively")
+                val includeChain = StringBuilder(this.location.programName);
+                var current: Program? = parent
+                while (current != null) {
+                    includeChain.append(" -> ")
+                    includeChain.append(current.location.programName)
+                    if (current == this) {
+                        break
+                    }
+                    current = visited[current]
+                }
+                loader.errorReporter().error(location, "Circular include; file includes itself transitively $includeChain")
                 throw IllegalStateException("Circular include: " + location.path
-                        + " includes itself transitively")
+                        + " includes itself transitively " + includeChain)
             }
             return
         }
+        visited[this] = parent
 
         check(this.includedPrograms == null) { "Included programs already resolved" }
 
         val includes = mutableListOf<Program>()
         for (thriftImport in thriftIncludes) {
             val included = loader.resolveIncludedProgram(location, thriftImport)
-            included.loadIncludedPrograms(loader, visited)
+            included.loadIncludedPrograms(loader, visited, this)
             includes.add(included)
         }
 

thrifty-schema/src/test/kotlin/com/microsoft/thrifty/schema/LoaderTest.kt

@@ -268,19 +268,21 @@ class LoaderTest {
 
     @Test
     fun circularInclude() {
-        val f1 = tempDir.newFile()
-        val f2 = tempDir.newFile()
-        val f3 = tempDir.newFile()
+        val f1 = tempDir.newFile("A")
+        val f2 = tempDir.newFile("B")
+        val f3 = tempDir.newFile("C")
+        val f4 = tempDir.newFile("D")
 
         f1.writeText("include '${f2.name}'")
         f2.writeText("include '${f3.name}'")
-        f3.writeText("include '${f1.name}'")
+        f3.writeText("include '${f4.name}'")
+        f4.writeText("include '${f2.name}'")
 
         try {
-            load(f1, f2, f3)
+            load(f1, f2, f3, f4)
             fail("Circular includes should fail to load")
         } catch (e: LoadFailedException) {
-            assertHasError(e, "Circular include")
+            assertHasError(e, "Circular include; file includes itself transitively B -> D -> C -> B")
         }
 
     }