From 1a9d1ffefb9103b511f38031d7d89b250b23ba02 Mon Sep 17 00:00:00 2001 From: Alexander Wang Date: Tue, 20 Jun 2023 17:39:38 -0700 Subject: [PATCH] basic move --- d2oracle/edit.go | 76 ++- d2oracle/edit_test.go | 49 +- .../d2oracle/TestMove/layers-basic.exp.json | 450 ++++++++++++++++++ .../TestMove/scenarios-out-of-scope.exp.json | 4 + 4 files changed, 554 insertions(+), 25 deletions(-) create mode 100644 testdata/d2oracle/TestMove/layers-basic.exp.json create mode 100644 testdata/d2oracle/TestMove/scenarios-out-of-scope.exp.json diff --git a/d2oracle/edit.go b/d2oracle/edit.go index 99b8d99f5..5ca66601d 100644 --- a/d2oracle/edit.go +++ b/d2oracle/edit.go @@ -1086,7 +1086,8 @@ func renameConflictsToParent(g *d2graph.Graph, key *d2ast.KeyPath) (*d2graph.Gra } for _, k := range renameOrder { var err error - g, err = move(g, k, renames[k], false) + // TODO boardPath + g, err = move(g, nil, k, renames[k], false) if err != nil { return nil, err } @@ -1458,7 +1459,8 @@ func Rename(g *d2graph.Graph, key, newName string) (_ *d2graph.Graph, newKey str mk.Key.Path[len(mk.Key.Path)-1] = d2ast.MakeValueBox(d2ast.RawString(newName, true)).StringBox() } - g, err = move(g, key, d2format.Format(mk), false) + // TODO + g, err = move(g, nil, key, d2format.Format(mk), false) return g, newName, err } @@ -1472,16 +1474,30 @@ func trimReservedSuffix(path []*d2ast.StringBox) []*d2ast.StringBox { } // Does not handle edge keys, on account of edge keys can only be reserved, e.g. (a->b).style.color: red -func Move(g *d2graph.Graph, key, newKey string, includeDescendants bool) (_ *d2graph.Graph, err error) { +func Move(g *d2graph.Graph, boardPath []string, key, newKey string, includeDescendants bool) (_ *d2graph.Graph, err error) { defer xdefer.Errorf(&err, "failed to move: %#v to %#v", key, newKey) - return move(g, key, newKey, includeDescendants) + return move(g, boardPath, key, newKey, includeDescendants) } -func move(g *d2graph.Graph, key, newKey string, includeDescendants bool) (*d2graph.Graph, error) { +func move(g *d2graph.Graph, boardPath []string, key, newKey string, includeDescendants bool) (*d2graph.Graph, error) { if key == newKey { return g, nil } - newKey, _, err := generateUniqueKey(g, newKey, nil, nil) + + boardG := g + baseAST := g.AST + + if len(boardPath) > 0 { + // When compiling a nested board, we can read from boardG but only write to baseBoardG + boardG = GetBoardGraph(g, boardPath) + if boardG == nil { + return nil, fmt.Errorf("board %v not found", boardPath) + } + // TODO beter name + baseAST = boardG.BaseAST + } + + newKey, _, err := generateUniqueKey(boardG, newKey, nil, nil) if err != nil { return nil, err } @@ -1497,6 +1513,7 @@ func move(g *d2graph.Graph, key, newKey string, includeDescendants bool) (*d2gra } edgeTrimCommon(mk) edgeTrimCommon(mk2) + if len(mk.Edges) > 0 && mk.EdgeKey == nil { if d2format.Format(mk.Key) != d2format.Format(mk2.Key) { // TODO just prevent moving edges at all @@ -1527,7 +1544,7 @@ func move(g *d2graph.Graph, key, newKey string, includeDescendants bool) (*d2gra return recompile(g.AST) } - prevG, _ := recompile(g.AST) + prevG, _ := recompile(boardG.AST) ak := d2graph.Key(mk.Key) ak2 := d2graph.Key(mk2.Key) @@ -1535,20 +1552,27 @@ func move(g *d2graph.Graph, key, newKey string, includeDescendants bool) (*d2gra isCrossScope := strings.Join(ak[:len(ak)-1], ".") != strings.Join(ak2[:len(ak2)-1], ".") if isCrossScope && !includeDescendants { - g, err = renameConflictsToParent(g, mk.Key) + boardG, err = renameConflictsToParent(boardG, mk.Key) if err != nil { return nil, err } } - obj, ok := g.Root.HasChild(ak) + obj, ok := boardG.Root.HasChild(ak) if !ok { return nil, fmt.Errorf("key referenced by from does not exist") } - toParent := g.Root + if len(boardPath) > 0 { + writeableRefs := getWriteableRefs(obj, baseAST) + if len(writeableRefs) != len(obj.References) { + return nil, OutsideScopeError{} + } + } + + toParent := boardG.Root if isCrossScope && len(ak2) > 1 { - toParent, ok = g.Root.HasChild(ak2[:len(ak2)-1]) + toParent, ok = boardG.Root.HasChild(ak2[:len(ak2)-1]) if !ok { return nil, fmt.Errorf("key referenced by to parent does not exist") } @@ -1618,7 +1642,7 @@ func move(g *d2graph.Graph, key, newKey string, includeDescendants bool) (*d2gra // 2. Ensure parent node Key has a map to accept moved node. // This map will be what MOVE will append the new key to - toScope := g.AST + toScope := boardG.AST if isCrossScope && len(ak2) > 1 && needsLandingMap { mostNestedParentRefs := getMostNestedRefs(toParent) mapExists := false @@ -1710,7 +1734,7 @@ func move(g *d2graph.Graph, key, newKey string, includeDescendants bool) (*d2gra } absKey.Path = append(absKey.Path, ref.Key.Path...) if !includeDescendants { - hoistRefChildren(g, absKey, ref) + hoistRefChildren(boardG, absKey, ref) } deleteFromMap(ref.Scope, ref.MapKey) detachedMK := &d2ast.Key{Primary: ref.MapKey.Primary, Key: cloneKey(ref.MapKey.Key)} @@ -1752,7 +1776,7 @@ func move(g *d2graph.Graph, key, newKey string, includeDescendants bool) (*d2gra return nil, err } - newPath, err := pathFromScopeKey(g, &d2ast.Key{Key: d2ast.MakeKeyPath(append(resolvedParent.AbsIDArray(), resolvedScopeKey...))}, ak2) + newPath, err := pathFromScopeKey(boardG, &d2ast.Key{Key: d2ast.MakeKeyPath(append(resolvedParent.AbsIDArray(), resolvedScopeKey...))}, ak2) if err != nil { return nil, err } @@ -1766,7 +1790,7 @@ func move(g *d2graph.Graph, key, newKey string, includeDescendants bool) (*d2gra return nil, err } - newPath, err := pathFromScopeKey(g, &d2ast.Key{Key: d2ast.MakeKeyPath(append(resolvedParent.AbsIDArray(), resolvedScopeKey...))}, ak2) + newPath, err := pathFromScopeKey(boardG, &d2ast.Key{Key: d2ast.MakeKeyPath(append(resolvedParent.AbsIDArray(), resolvedScopeKey...))}, ak2) if err != nil { return nil, err } @@ -1778,7 +1802,7 @@ func move(g *d2graph.Graph, key, newKey string, includeDescendants bool) (*d2gra return nil, err } - newPath, err := pathFromScopeKey(g, &d2ast.Key{Key: d2ast.MakeKeyPath(append(resolvedParent.AbsIDArray(), resolvedScopeKey...))}, ak2) + newPath, err := pathFromScopeKey(boardG, &d2ast.Key{Key: d2ast.MakeKeyPath(append(resolvedParent.AbsIDArray(), resolvedScopeKey...))}, ak2) if err != nil { return nil, err } @@ -1876,11 +1900,11 @@ func move(g *d2graph.Graph, key, newKey string, includeDescendants bool) (*d2gra detachedMK := &d2ast.Key{ Key: cloneKey(ref.Key), } - oldPath, err := pathFromScopeObj(g, mk, ref.ScopeObj) + oldPath, err := pathFromScopeObj(boardG, mk, ref.ScopeObj) if err != nil { return nil, err } - newPath, err := pathFromScopeObj(g, mk2, ref.ScopeObj) + newPath, err := pathFromScopeObj(boardG, mk2, ref.ScopeObj) if err != nil { return nil, err } @@ -1911,14 +1935,14 @@ func move(g *d2graph.Graph, key, newKey string, includeDescendants bool) (*d2gra // When moving a node connected to an edge, we have to ensure parents continue to exist // e.g. the `c` out of `a.b.c -> ...` // `a.b` needs to exist - newPath, err := pathFromScopeObj(g, mk2, ref.ScopeObj) + newPath, err := pathFromScopeObj(boardG, mk2, ref.ScopeObj) if err != nil { return nil, err } if len(go2.Filter(ref.Key.Path, func(x *d2ast.StringBox) bool { return x.Unbox().ScalarString() != "_" })) > 1 { detachedK := cloneKey(ref.Key) detachedK.Path = detachedK.Path[:len(detachedK.Path)-1] - ensureNode(g, refEdges, ref.ScopeObj, ref.Scope, ref.MapKey, detachedK, false) + ensureNode(boardG, refEdges, ref.ScopeObj, ref.Scope, ref.MapKey, detachedK, false) } if includeDescendants { @@ -1929,11 +1953,19 @@ func move(g *d2graph.Graph, key, newKey string, includeDescendants bool) (*d2gra } } - if err := updateNear(prevG, g, &key, &newKey, includeDescendants); err != nil { + if err := updateNear(prevG, boardG, &key, &newKey, includeDescendants); err != nil { return nil, err } - return recompile(g.AST) + if len(boardPath) > 0 { + replaced := ReplaceBoardNode(g.AST, baseAST, boardPath) + if !replaced { + return nil, fmt.Errorf("board %v AST not found", boardPath) + } + return recompile(g.AST) + } + + return recompile(boardG.AST) } // filterReserved takes a Value and splits it into 2 diff --git a/d2oracle/edit_test.go b/d2oracle/edit_test.go index 68c061e50..2128480db 100644 --- a/d2oracle/edit_test.go +++ b/d2oracle/edit_test.go @@ -2651,8 +2651,9 @@ func TestMove(t *testing.T) { t.Parallel() testCases := []struct { - skip bool - name string + skip bool + name string + boardPath []string text string key string @@ -4833,6 +4834,48 @@ a } `, }, + { + name: "layers-basic", + + text: `a +layers: { + x: { + b + c + } +} +`, + key: `c`, + newKey: `b.c`, + boardPath: []string{"root", "layers", "x"}, + + exp: `a +layers: { + x: { + b: { + c + } + } +} +`, + }, + { + name: "scenarios-out-of-scope", + + text: `a +scenarios: { + x: { + b + c + } +} +`, + key: `a`, + newKey: `b.a`, + boardPath: []string{"root", "scenarios", "x"}, + + expErr: `failed to move: "a" to "b.a": operation would modify AST outside of given scope`, + }, } for _, tc := range testCases { @@ -4848,7 +4891,7 @@ a testFunc: func(g *d2graph.Graph) (*d2graph.Graph, error) { objectsBefore := len(g.Objects) var err error - g, err = d2oracle.Move(g, tc.key, tc.newKey, tc.includeDescendants) + g, err = d2oracle.Move(g, tc.boardPath, tc.key, tc.newKey, tc.includeDescendants) if err == nil { objectsAfter := len(g.Objects) if objectsBefore != objectsAfter { diff --git a/testdata/d2oracle/TestMove/layers-basic.exp.json b/testdata/d2oracle/TestMove/layers-basic.exp.json new file mode 100644 index 000000000..dc8b931af --- /dev/null +++ b/testdata/d2oracle/TestMove/layers-basic.exp.json @@ -0,0 +1,450 @@ +{ + "graph": { + "name": "", + "isFolderOnly": false, + "ast": { + "range": "d2/testdata/d2oracle/TestMove/layers-basic.d2,0:0:0-8:0:48", + "nodes": [ + { + "map_key": { + "range": "d2/testdata/d2oracle/TestMove/layers-basic.d2,0:0:0-0:1:1", + "key": { + "range": "d2/testdata/d2oracle/TestMove/layers-basic.d2,0:0:0-0:1:1", + "path": [ + { + "unquoted_string": { + "range": "d2/testdata/d2oracle/TestMove/layers-basic.d2,0:0:0-0:1:1", + "value": [ + { + "string": "a", + "raw_string": "a" + } + ] + } + } + ] + }, + "primary": {}, + "value": {} + } + }, + { + "map_key": { + "range": "d2/testdata/d2oracle/TestMove/layers-basic.d2,1:0:2-7:1:47", + "key": { + "range": "d2/testdata/d2oracle/TestMove/layers-basic.d2,1:0:2-1:6:8", + "path": [ + { + "unquoted_string": { + "range": "d2/testdata/d2oracle/TestMove/layers-basic.d2,1:0:2-1:6:8", + "value": [ + { + "string": "layers", + "raw_string": "layers" + } + ] + } + } + ] + }, + "primary": {}, + "value": { + "map": { + "range": "d2/testdata/d2oracle/TestMove/layers-basic.d2,1:8:10-7:1:47", + "nodes": [ + { + "map_key": { + "range": "d2/testdata/d2oracle/TestMove/layers-basic.d2,2:2:14-6:3:45", + "key": { + "range": "d2/testdata/d2oracle/TestMove/layers-basic.d2,2:2:14-2:3:15", + "path": [ + { + "unquoted_string": { + "range": "d2/testdata/d2oracle/TestMove/layers-basic.d2,2:2:14-2:3:15", + "value": [ + { + "string": "x", + "raw_string": "x" + } + ] + } + } + ] + }, + "primary": {}, + "value": { + "map": { + "range": "d2/testdata/d2oracle/TestMove/layers-basic.d2,2:5:17-6:3:45", + "nodes": [ + { + "map_key": { + "range": "d2/testdata/d2oracle/TestMove/layers-basic.d2,3:4:23-5:5:41", + "key": { + "range": "d2/testdata/d2oracle/TestMove/layers-basic.d2,3:4:23-3:5:24", + "path": [ + { + "unquoted_string": { + "range": "d2/testdata/d2oracle/TestMove/layers-basic.d2,3:4:23-3:5:24", + "value": [ + { + "string": "b", + "raw_string": "b" + } + ] + } + } + ] + }, + "primary": {}, + "value": { + "map": { + "range": "d2/testdata/d2oracle/TestMove/layers-basic.d2,3:7:26-5:5:41", + "nodes": [ + { + "map_key": { + "range": "d2/testdata/d2oracle/TestMove/layers-basic.d2,4:6:34-4:7:35", + "key": { + "range": "d2/testdata/d2oracle/TestMove/layers-basic.d2,4:6:34-4:7:35", + "path": [ + { + "unquoted_string": { + "range": "d2/testdata/d2oracle/TestMove/layers-basic.d2,4:6:34-4:7:35", + "value": [ + { + "string": "c", + "raw_string": "c" + } + ] + } + } + ] + }, + "primary": {}, + "value": {} + } + } + ] + } + } + } + } + ] + } + } + } + } + ] + } + } + } + } + ] + }, + "root": { + "id": "", + "id_val": "", + "attributes": { + "label": { + "value": "" + }, + "labelDimensions": { + "width": 0, + "height": 0 + }, + "style": {}, + "near_key": null, + "shape": { + "value": "" + }, + "direction": { + "value": "" + }, + "constraint": null + }, + "zIndex": 0 + }, + "edges": null, + "objects": [ + { + "id": "a", + "id_val": "a", + "references": [ + { + "key": { + "range": "d2/testdata/d2oracle/TestMove/layers-basic.d2,0:0:0-0:1:1", + "path": [ + { + "unquoted_string": { + "range": "d2/testdata/d2oracle/TestMove/layers-basic.d2,0:0:0-0:1:1", + "value": [ + { + "string": "a", + "raw_string": "a" + } + ] + } + } + ] + }, + "key_path_index": 0, + "map_key_edge_index": -1 + } + ], + "attributes": { + "label": { + "value": "a" + }, + "labelDimensions": { + "width": 0, + "height": 0 + }, + "style": {}, + "near_key": null, + "shape": { + "value": "rectangle" + }, + "direction": { + "value": "" + }, + "constraint": null + }, + "zIndex": 0 + } + ], + "layers": [ + { + "name": "x", + "isFolderOnly": false, + "ast": { + "range": ",1:0:0-2:0:0", + "nodes": [ + { + "map_key": { + "range": ",0:0:0-0:0:0", + "key": { + "range": ",0:0:0-0:0:0", + "path": [ + { + "unquoted_string": { + "range": ",0:0:0-0:0:0", + "value": [ + { + "string": "b" + } + ] + } + } + ] + }, + "primary": {}, + "value": { + "map": { + "range": ",1:0:0-2:0:0", + "nodes": [ + { + "map_key": { + "range": ",0:0:0-0:0:0", + "key": { + "range": ",0:0:0-0:0:0", + "path": [ + { + "unquoted_string": { + "range": ",0:0:0-0:0:0", + "value": [ + { + "string": "c" + } + ] + } + } + ] + }, + "primary": {}, + "value": {} + } + } + ] + } + } + } + } + ] + }, + "baseAST": { + "range": "d2/testdata/d2oracle/TestMove/layers-basic.d2,2:5:17-6:3:45", + "nodes": [ + { + "map_key": { + "range": "d2/testdata/d2oracle/TestMove/layers-basic.d2,3:4:23-5:5:41", + "key": { + "range": "d2/testdata/d2oracle/TestMove/layers-basic.d2,3:4:23-3:5:24", + "path": [ + { + "unquoted_string": { + "range": "d2/testdata/d2oracle/TestMove/layers-basic.d2,3:4:23-3:5:24", + "value": [ + { + "string": "b", + "raw_string": "b" + } + ] + } + } + ] + }, + "primary": {}, + "value": { + "map": { + "range": "d2/testdata/d2oracle/TestMove/layers-basic.d2,3:7:26-5:5:41", + "nodes": [ + { + "map_key": { + "range": "d2/testdata/d2oracle/TestMove/layers-basic.d2,4:6:34-4:7:35", + "key": { + "range": "d2/testdata/d2oracle/TestMove/layers-basic.d2,4:6:34-4:7:35", + "path": [ + { + "unquoted_string": { + "range": "d2/testdata/d2oracle/TestMove/layers-basic.d2,4:6:34-4:7:35", + "value": [ + { + "string": "c", + "raw_string": "c" + } + ] + } + } + ] + }, + "primary": {}, + "value": {} + } + } + ] + } + } + } + } + ] + }, + "root": { + "id": "", + "id_val": "", + "attributes": { + "label": { + "value": "" + }, + "labelDimensions": { + "width": 0, + "height": 0 + }, + "style": {}, + "near_key": null, + "shape": { + "value": "" + }, + "direction": { + "value": "" + }, + "constraint": null + }, + "zIndex": 0 + }, + "edges": null, + "objects": [ + { + "id": "b", + "id_val": "b", + "references": [ + { + "key": { + "range": "d2/testdata/d2oracle/TestMove/layers-basic.d2,3:4:23-3:5:24", + "path": [ + { + "unquoted_string": { + "range": "d2/testdata/d2oracle/TestMove/layers-basic.d2,3:4:23-3:5:24", + "value": [ + { + "string": "b", + "raw_string": "b" + } + ] + } + } + ] + }, + "key_path_index": 0, + "map_key_edge_index": -1 + } + ], + "attributes": { + "label": { + "value": "b" + }, + "labelDimensions": { + "width": 0, + "height": 0 + }, + "style": {}, + "near_key": null, + "shape": { + "value": "rectangle" + }, + "direction": { + "value": "" + }, + "constraint": null + }, + "zIndex": 0 + }, + { + "id": "c", + "id_val": "c", + "references": [ + { + "key": { + "range": "d2/testdata/d2oracle/TestMove/layers-basic.d2,4:6:34-4:7:35", + "path": [ + { + "unquoted_string": { + "range": "d2/testdata/d2oracle/TestMove/layers-basic.d2,4:6:34-4:7:35", + "value": [ + { + "string": "c", + "raw_string": "c" + } + ] + } + } + ] + }, + "key_path_index": 0, + "map_key_edge_index": -1 + } + ], + "attributes": { + "label": { + "value": "c" + }, + "labelDimensions": { + "width": 0, + "height": 0 + }, + "style": {}, + "near_key": null, + "shape": { + "value": "rectangle" + }, + "direction": { + "value": "" + }, + "constraint": null + }, + "zIndex": 0 + } + ] + } + ] + }, + "err": "" +} diff --git a/testdata/d2oracle/TestMove/scenarios-out-of-scope.exp.json b/testdata/d2oracle/TestMove/scenarios-out-of-scope.exp.json new file mode 100644 index 000000000..8b6a3a125 --- /dev/null +++ b/testdata/d2oracle/TestMove/scenarios-out-of-scope.exp.json @@ -0,0 +1,4 @@ +{ + "graph": null, + "err": "failed to move: \"a\" to \"b.a\": operation would modify AST outside of given scope" +}