style fixes

Author: Wojciech Jabłoński

uast/diff/apply.go

@@ -5,7 +5,7 @@ import (
 	"gopkg.in/bblfsh/sdk.v2/uast/nodes"
 )
 
-func Apply(root nodes.Node, changelist Changelist) nodes.Node {
+func (changelist Changelist) Apply(root nodes.Node) nodes.Node {
 	nodeDict := make(map[ID]nodes.Node)
 	nodes.WalkPreOrder(root, func(node nodes.Node) bool {
 		nodeDict[nodes.UniqueKey(node)] = node
@@ -15,11 +15,11 @@ func Apply(root nodes.Node, changelist Changelist) nodes.Node {
 	for _, change := range changelist {
 		switch ch := change.(type) {
 		case Create:
-			//create a node and add to the dictionary
+			// create a node and add to the dictionary
 			nodeDict[nodes.UniqueKey(ch.Node)] = ch.Node
 
 		case Attach:
-			//get src and chld from the dictionary, attach (modify src)
+			// get src and chld from the dictionary, attach (modify src)
 			parent, ok := nodeDict[ch.Parent]
 			if !ok {
 				panic("invalid attachment point")
@@ -44,7 +44,7 @@ func Apply(root nodes.Node, changelist Changelist) nodes.Node {
 			}
 
 		case Deatach:
-			//get the src from the dictionary, deatach (modify src)
+			// get the src from the dictionary, deatach (modify src)
 			parent := nodeDict[ch.Parent]
 
 			switch key := ch.Key.(type) {

uast/diff/changelist.go

@@ -18,7 +18,6 @@ type changeBase struct {
 func (changeBase) isChange()                {}
 func (ch changeBase) TransactionID() uint64 { return ch.txID }
 
-// TODO: proper ID of a node somehow
 type ID nodes.Comparable
 
 // key in a node, string for nodes.Object and int for nodes.Array
@@ -56,5 +55,6 @@ type Attach struct {
 type Deatach struct {
 	changeBase
 	Parent ID
-	Key    Key //or string, how to do alternative?
+	Key    Key 	// Currently deatach semantics are only defined for nodes.Object so the Key is
+				// practically always a string
 }

uast/diff/changelist_test.go

@@ -1,8 +1,9 @@
 package diff
 
 import (
-    "fmt"
+    "strings"
     "os"
+    "path/filepath"
     "io/ioutil"
     "testing"
     "gopkg.in/bblfsh/sdk.v2/uast/yaml"
@@ -22,23 +23,26 @@ func readUAST(t testing.TB, path string) nodes.Node {
 }
 
 func TestChangelist(t *testing.T) {
-    fd, err := os.Open(fmt.Sprintf("%v/config.txt", dataDir))
+    dir, err := os.Open(dataDir)
     require.NoError(t, err)
-    var n int
-    _, err = fmt.Fscanf(fd, "%d\n", &n)
+    defer dir.Close()
+    names, err := dir.Readdirnames(-1)
     require.NoError(t, err)
 
-    for i := 0; i < n; i++ {
-        name := fmt.Sprintf("%v/testcase_%v", dataDir, i)
-        t.Run(name, func(t *testing.T) {
-            src_name := fmt.Sprintf("%v_src.uast", name)
-            dst_name := fmt.Sprintf("%v_dst.uast", name)
-            src := readUAST(t, src_name)
-            dst := readUAST(t, dst_name)
-
-            changes := Changes(src, dst)
-            newsrc := Apply(src, changes)
-            require.True(t, nodes.Equal(newsrc, dst))
-        })
+    for _, fname := range names {
+        if strings.HasSuffix(fname, "_src.uast") {
+            name := fname[:len(fname)-len("_src.uast")]
+
+            t.Run(name, func(t *testing.T) {
+                src_name := filepath.Join(dataDir, name + "_src.uast")
+                dst_name := filepath.Join(dataDir, name + "_dst.uast")
+                src := readUAST(t, src_name)
+                dst := readUAST(t, dst_name)
+
+                changes := Changes(src, dst)
+                newsrc := changes.Apply(src)
+                require.True(t, nodes.Equal(newsrc, dst))
+            })
+        }
     }
 }

uast/diff/diff.go

@@ -25,7 +25,7 @@ type permuteDecision struct {
 	permutation []int
 }
 
-//min is a convinience method for choosing the cheapest decision
+// min is a convinience method for choosing the cheapest decision
 func min(self, candidate decisionType) decisionType {
 	if self.cost() > candidate.cost() {
 		return candidate
@@ -34,10 +34,10 @@ func min(self, candidate decisionType) decisionType {
 	}
 }
 
-//type for cache
+// type for cache
 type keyType struct{ k1, k2 ID }
 
-//cache for diff computation
+// cache for diff computation
 type cacheStorage struct {
 	decisions map[keyType]decisionType
 	counts     map[ID]int
@@ -104,16 +104,12 @@ func (ds *cacheStorage) decideAction(src, dst nodes.Node) decisionType {
 		// the code below iterates over each of the keys from src and dst exactly once, that's why
 		// keys isn't reset between iterations.
 		keys := make(map[string]bool)
-		iterate := func(keyset nodes.Object) {
-			for key := range keyset {
-				if in := keys[key]; !in {
-					keys[key] = true
-					cost += ds.decideAction(src[key], dst[key]).cost()
-				}
+		for _, key := range append(src.Keys(), dst.Keys()...) {
+			if in := keys[key]; !in {
+				keys[key] = true
+				cost += ds.decideAction(src[key], dst[key]).cost()
 			}
 		}
-		iterate(src)
-		iterate(dst)
 
 		if cost == 0 {
 			bestDecision = min(bestDecision, sameDecision{basicDecision{cost}})
@@ -133,9 +129,7 @@ func (ds *cacheStorage) decideAction(src, dst nodes.Node) decisionType {
 				bestDecision = min(bestDecision, sameDecision{basicDecision{cost}})
 				break
 			}
-		}
-
-		if len(src) != len(dst) {
+		} else {
 			cost = 2
 		}
 
@@ -223,28 +217,24 @@ func (ds *cacheStorage) generateDifference(src, dst nodes.Node, parentID ID, par
 	case matchDecision:
 		src, dst := src.(nodes.Object), dst.(nodes.Object)
 		keys := make(map[string]bool)
-		iterate := func(keyset nodes.Object) {
-			for key := range keyset {
-				if in := keys[key]; !in {
-					keys[key] = true
-					if _, ok := dst[key]; !ok {
-						ds.push(Deatach{Parent: nodes.UniqueKey(src), Key: String(key)})
-					} else  if _, ok := src[key]; !ok {
-						ds.createRec(dst[key])
-						ds.push(Attach{
-							Parent: nodes.UniqueKey(src),
-							Key: String(key),
-							Child: nodes.UniqueKey(dst[key]),
-						})
-					} else {
-						ds.generateDifference(
-							src[key], dst[key], nodes.UniqueKey(src), String(key))
-					}
+		for _, key := range append(src.Keys(), dst.Keys()...) {
+			if in := keys[key]; !in {
+				keys[key] = true
+				if _, ok := dst[key]; !ok {
+					ds.push(Deatach{Parent: nodes.UniqueKey(src), Key: String(key)})
+				} else  if _, ok := src[key]; !ok {
+					ds.createRec(dst[key])
+					ds.push(Attach{
+						Parent: nodes.UniqueKey(src),
+						Key: String(key),
+						Child: nodes.UniqueKey(dst[key]),
+					})
+				} else {
+					ds.generateDifference(
+						src[key], dst[key], nodes.UniqueKey(src), String(key))
 				}
 			}
 		}
-		iterate(src)
-		iterate(dst)
 
 	case permuteDecision:
 		src, dst := src.(nodes.Array), dst.(nodes.Array)