From 2b8ac831a3d2bc225c1a72955e390b5a9583cdaa Mon Sep 17 00:00:00 2001
From: David Symonds <dsymonds@gmail.com>
Date: Sat, 26 Mar 2016 15:20:39 +1100
Subject: [PATCH] Topologically sort ast.FileSet.Files so imported files come
 first.

This matches protoc behaviour.
---
 .gitignore       |  1 +
 Makefile         | 15 +++++++++++++++
 ast/ast.go       | 36 ++++++++++++++++++++++++++++++++++++
 main.go          |  3 +--
 parser/parser.go |  1 +
 testdata/run.sh  |  2 +-
 6 files changed, 55 insertions(+), 3 deletions(-)

diff --git a/.gitignore b/.gitignore
index 195d161..29a3ab0 100644
--- a/.gitignore
+++ b/.gitignore
@@ -1,5 +1,6 @@
 .*.swp
 .DS_Store
+_mini*
 _testmain.go
 gotoc
 testdata/*.actual
diff --git a/Makefile b/Makefile
index 6819497..76bcd91 100644
--- a/Makefile
+++ b/Makefile
@@ -21,3 +21,18 @@ baseline:
 	@protoc --descriptor_set_out=$(MINI_TMP) --include_imports testdata/mini.proto
 	@protoc --decode=google.protobuf.FileDescriptorSet -I $(PROTOBUF) $(PROTOBUF)/src/google/protobuf/descriptor.proto < $(MINI_TMP)
 	@rm $(MINI_TMP)
+
+MINI_FSET_GOTOC=_mini_fset_gotoc.txt
+MINI_FSET_PROTOC=_mini_fset_protoc.txt
+baselinediff:
+	go build
+	# First, gotoc
+	@./gotoc --descriptor_only testdata/mini.proto > $(MINI_FSET_GOTOC)
+	@sed -i '' 's/: </ {/g' $(MINI_FSET_GOTOC)
+	@sed -i '' 's/>$$/}/g' $(MINI_FSET_GOTOC)
+	# Next, protoc
+	@protoc --descriptor_set_out=$(MINI_TMP) --include_imports testdata/mini.proto
+	@protoc --decode=google.protobuf.FileDescriptorSet -I $(PROTOBUF) $(PROTOBUF)/src/google/protobuf/descriptor.proto < $(MINI_TMP) > $(MINI_FSET_PROTOC)
+	# Compare
+	diff -ud $(MINI_FSET_PROTOC) $(MINI_FSET_GOTOC)
+	@rm $(MINI_TMP) $(MINI_FSET_GOTOC) $(MINI_FSET_PROTOC)
diff --git a/ast/ast.go b/ast/ast.go
index 558ccb6..de887ea 100644
--- a/ast/ast.go
+++ b/ast/ast.go
@@ -17,9 +17,45 @@ type Node interface {
 
 // FileSet describes a set of proto files.
 type FileSet struct {
+	// Files is sorted in topological order, bottom up.
+	// That means that a file X will only import a file Y
+	// if Y occurs in this slice before X.
 	Files []*File
 }
 
+// Sort sorts fs.Files topologically.
+func (fs *FileSet) Sort() {
+	in := fs.Files                   // old version of fs.Files; shrinks each loop
+	out := make([]*File, 0, len(in)) // new version of fs.Files; grows each loop
+	done := make(map[string]bool)    // filenames that we've seen and that don't have un-done imports
+	for len(in) > 0 {
+		// Find a file that doesn't have an un-done import.
+		var next *File
+		for i, f := range in {
+			ok := true
+			for _, imp := range f.Imports {
+				if !done[imp] {
+					ok = false
+					break
+				}
+			}
+			if !ok {
+				continue
+			}
+			next = f
+			copy(in[i:], in[i+1:])
+			in = in[:len(in)-1]
+			break
+		}
+		if next == nil {
+			panic("import loop!") // shouldn't happen
+		}
+		out = append(out, next)
+		done[next.Name] = true
+	}
+	fs.Files = out
+}
+
 // File represents a single proto file.
 type File struct {
 	Name    string // filename
diff --git a/main.go b/main.go
index f78c5f6..85b4665 100644
--- a/main.go
+++ b/main.go
@@ -76,8 +76,7 @@ func main() {
 	// Prepare request.
 	cgRequest := &plugin.CodeGeneratorRequest{
 		FileToGenerate: flag.Args(),
-		// TODO: proto_file should be topologically sorted (bottom-up)
-		ProtoFile: fds.File,
+		ProtoFile:      fds.File,
 	}
 	buf, err := proto.Marshal(cgRequest)
 	if err != nil {
diff --git a/parser/parser.go b/parser/parser.go
index c1539bc..e9764ba 100644
--- a/parser/parser.go
+++ b/parser/parser.go
@@ -86,6 +86,7 @@ func ParseFiles(filenames []string, importPaths []string) (*ast.FileSet, error)
 	if err := resolveSymbols(fset); err != nil {
 		return nil, err
 	}
+	fset.Sort()
 	return fset, nil
 }
 
diff --git a/testdata/run.sh b/testdata/run.sh
index bdf897e..bbd071f 100755
--- a/testdata/run.sh
+++ b/testdata/run.sh
@@ -17,7 +17,7 @@ for ((i=1; $i <= $MAX; i=$((i+1)))); do
 
   $GOTOC --descriptor_only $i.proto > $i.actual
   $PROTOCMP $i.expected $i.actual || {
-    echo "==> FAILED" 1>&2
+    echo "==> FAILED expected vs. actual" 1>&2
     failures=$(($failures + 1))
   }