diff: refactor

Signed-off-by: Denys Smirnov <[email protected]>

Author: Denys Smirnov

uast/diff/apply.go

@@ -1,21 +1,22 @@
 package diff
 
 import (
+	"errors"
 	"fmt"
 
 	"gopkg.in/bblfsh/sdk.v2/uast/nodes"
 )
 
-// Apply is a method that takes a tree (nodes.Node) and applies the current changelist to that
-// tree.
-func (changelist Changelist) Apply(root nodes.Node) nodes.Node {
+// Apply is a method that takes a tree (nodes.Node) and applies the current changelist to
+// that tree.
+func (cl Changelist) Apply(root nodes.Node) (nodes.Node, error) {
 	nodeDict := make(map[ID]nodes.Node)
 	nodes.WalkPreOrder(root, func(node nodes.Node) bool {
 		nodeDict[nodes.UniqueKey(node)] = node
 		return true
 	})
 
-	for _, change := range changelist {
+	for _, change := range cl {
 		switch ch := change.(type) {
 		case Create:
 			// create a node and add to the dictionary
@@ -25,47 +26,45 @@ func (changelist Changelist) Apply(root nodes.Node) nodes.Node {
 			// get src and chld from the dictionary, attach (modify src)
 			parent, ok := nodeDict[ch.Parent]
 			if !ok {
-				panic("invalid attachment point")
+				return nil, errors.New("diff: invalid attachment point")
 			}
 			child, ok := nodeDict[ch.Child]
 			if !ok {
 				child, ok = ch.Child.(nodes.Value)
 				if !ok {
-					panic(fmt.Errorf("unknown type of a child: %v (type %T)", ch.Child, ch.Child))
+					return nil, fmt.Errorf("diff: unknown type of a child: %T", ch.Child)
 				}
 			}
 
 			switch key := ch.Key.(type) {
-
 			case String:
 				parent := parent.(nodes.Object)
 				parent[string(key)] = child
-
 			case Int:
 				parent := parent.(nodes.Array)
 				parent[int(key)] = child
+			default:
+				return nil, fmt.Errorf("diff: unknown type of a key: %T", ch.Key)
 			}
-
-		case Deatach:
+		case Detach:
 			// get the src from the dictionary, deatach (modify src)
 			parent := nodeDict[ch.Parent]
 
 			switch key := ch.Key.(type) {
-
 			case String:
 				parent := parent.(nodes.Object)
 				delete(parent, string(key))
-
 			case Int:
-				panic(fmt.Errorf("cannot deatach from an Array"))
+				return nil, errors.New("diff: cannot detach from an Array")
+			default:
+				return nil, fmt.Errorf("diff: unknown type of a key: %T", ch.Key)
 			}
 
 		case Delete:
-			panic(fmt.Errorf("delete is not supported in a Changelist"))
-
+			return nil, errors.New("diff: delete is not supported in a Changelist")
 		default:
-			panic(fmt.Sprintf("unknown change %v of type %T", change, change))
+			return nil, fmt.Errorf("diff: unknown change of type %T", change)
 		}
 	}
-	return root
+	return root, nil
 }

uast/diff/changelist.go

@@ -11,16 +11,10 @@ type Changelist []Change
 // Change is a single operation performed
 type Change interface {
 	isChange()
-	TransactionID() uint64
+	// GroupID is the same for multiple changes that are a part of one high-level operation.
+	GroupID() uint64
 }
 
-type changeBase struct {
-	txID uint64
-}
-
-func (changeBase) isChange()                {}
-func (ch changeBase) TransactionID() uint64 { return ch.txID }
-
 // ID is a type representing node unique ID that can be compared in O(1)
 type ID nodes.Comparable
 
@@ -40,28 +34,55 @@ func (String) isKey() {}
 
 // Create a node. Each array and object is created separately.
 type Create struct {
-	changeBase
+	group uint64
+
+	// Node to create.
 	Node nodes.Node
 }
 
+func (Create) isChange() {}
+
+func (ch Create) GroupID() uint64 { return ch.group }
+
 // Delete a node by ID
 type Delete struct {
-	changeBase
-	NodeID ID
+	group uint64
+
+	// Node to delete.
+	Node ID
 }
 
+func (Delete) isChange() {}
+
+func (ch Delete) GroupID() uint64 { return ch.group }
+
 // Attach a node as a child of another node with a given key
 type Attach struct {
-	changeBase
+	group uint64
+
+	// Parent node ID to attach the key to.
 	Parent ID
-	Key    Key
-	Child  ID
+	// Key to attach.
+	Key Key
+	// Child to attach on a given key.
+	Child ID
 }
 
-// Deatach a child from a node
-type Deatach struct {
-	changeBase
+func (Attach) isChange() {}
+
+func (ch Attach) GroupID() uint64 { return ch.group }
+
+// Detach a child from a node
+type Detach struct {
+	group uint64
+
+	// Parent node ID to detach the key from.
 	Parent ID
-	Key    Key // Currently deatach semantics are only defined for nodes.Object so the Key is
-	// practically always a string
+	// Key to detach. Currently detach semantics are only defined for nodes.Object so the
+	// Key is practically always a String.
+	Key Key
 }
+
+func (Detach) isChange() {}
+
+func (ch Detach) GroupID() uint64 { return ch.group }

uast/diff/changelist_test.go

@@ -40,7 +40,8 @@ func TestChangelist(t *testing.T) {
 				dst := readUAST(t, dstName)
 
 				changes := Changes(src, dst)
-				newsrc := changes.Apply(src)
+				newsrc, err := changes.Apply(src)
+				require.NoError(t, err)
 				require.True(t, nodes.Equal(newsrc, dst))
 			})
 		}