From 3ab9c4b182a3772dcb37d7534c671335b249b7a2 Mon Sep 17 00:00:00 2001 From: Alexander Wang Date: Thu, 22 Dec 2022 21:20:18 -0800 Subject: [PATCH 1/3] test --- d2compiler/compile_test.go | 20 + .../TestCompile/sql-regression.exp.json | 456 ++++++++++++++++++ 2 files changed, 476 insertions(+) create mode 100644 testdata/d2compiler/TestCompile/sql-regression.exp.json diff --git a/d2compiler/compile_test.go b/d2compiler/compile_test.go index 5923293fc..74a91f393 100644 --- a/d2compiler/compile_test.go +++ b/d2compiler/compile_test.go @@ -1650,6 +1650,26 @@ choo: { } }, }, + { + name: "sql-regression", + + text: `a: { + style: { + fill: lemonchiffon + } + + legend: { + shape: sql_table + "***": not in terraform yet + } + + thing +} +`, + assertions: func(t *testing.T, g *d2graph.Graph) { + tassert.Equal(t, 3, len(g.Objects)) + }, + }, } for _, tc := range testCases { diff --git a/testdata/d2compiler/TestCompile/sql-regression.exp.json b/testdata/d2compiler/TestCompile/sql-regression.exp.json new file mode 100644 index 000000000..35e28df48 --- /dev/null +++ b/testdata/d2compiler/TestCompile/sql-regression.exp.json @@ -0,0 +1,456 @@ +{ + "graph": { + "ast": { + "range": "d2/testdata/d2compiler/TestCompile/sql-regression.d2,0:0:0-12:0:124", + "nodes": [ + { + "map_key": { + "range": "d2/testdata/d2compiler/TestCompile/sql-regression.d2,0:0:0-11:1:123", + "key": { + "range": "d2/testdata/d2compiler/TestCompile/sql-regression.d2,0:0:0-0:1:1", + "path": [ + { + "unquoted_string": { + "range": "d2/testdata/d2compiler/TestCompile/sql-regression.d2,0:0:0-0:1:1", + "value": [ + { + "string": "a", + "raw_string": "a" + } + ] + } + } + ] + }, + "primary": {}, + "value": { + "map": { + "range": "d2/testdata/d2compiler/TestCompile/sql-regression.d2,0:3:3-11:0:122", + "nodes": [ + { + "map_key": { + "range": "d2/testdata/d2compiler/TestCompile/sql-regression.d2,1:2:7-3:3:42", + "key": { + "range": "d2/testdata/d2compiler/TestCompile/sql-regression.d2,1:2:7-1:7:12", + "path": [ + { + "unquoted_string": { + "range": "d2/testdata/d2compiler/TestCompile/sql-regression.d2,1:2:7-1:7:12", + "value": [ + { + "string": "style", + "raw_string": "style" + } + ] + } + } + ] + }, + "primary": {}, + "value": { + "map": { + "range": "d2/testdata/d2compiler/TestCompile/sql-regression.d2,1:9:14-3:2:41", + "nodes": [ + { + "map_key": { + "range": "d2/testdata/d2compiler/TestCompile/sql-regression.d2,2:4:20-2:22:38", + "key": { + "range": "d2/testdata/d2compiler/TestCompile/sql-regression.d2,2:4:20-2:8:24", + "path": [ + { + "unquoted_string": { + "range": "d2/testdata/d2compiler/TestCompile/sql-regression.d2,2:4:20-2:8:24", + "value": [ + { + "string": "fill", + "raw_string": "fill" + } + ] + } + } + ] + }, + "primary": {}, + "value": { + "unquoted_string": { + "range": "d2/testdata/d2compiler/TestCompile/sql-regression.d2,2:10:26-2:22:38", + "value": [ + { + "string": "lemonchiffon", + "raw_string": "lemonchiffon" + } + ] + } + } + } + } + ] + } + } + } + }, + { + "map_key": { + "range": "d2/testdata/d2compiler/TestCompile/sql-regression.d2,5:2:46-8:3:112", + "key": { + "range": "d2/testdata/d2compiler/TestCompile/sql-regression.d2,5:2:46-5:8:52", + "path": [ + { + "unquoted_string": { + "range": "d2/testdata/d2compiler/TestCompile/sql-regression.d2,5:2:46-5:8:52", + "value": [ + { + "string": "legend", + "raw_string": "legend" + } + ] + } + } + ] + }, + "primary": {}, + "value": { + "map": { + "range": "d2/testdata/d2compiler/TestCompile/sql-regression.d2,5:10:54-8:2:111", + "nodes": [ + { + "map_key": { + "range": "d2/testdata/d2compiler/TestCompile/sql-regression.d2,6:4:60-6:20:76", + "key": { + "range": "d2/testdata/d2compiler/TestCompile/sql-regression.d2,6:4:60-6:9:65", + "path": [ + { + "unquoted_string": { + "range": "d2/testdata/d2compiler/TestCompile/sql-regression.d2,6:4:60-6:9:65", + "value": [ + { + "string": "shape", + "raw_string": "shape" + } + ] + } + } + ] + }, + "primary": {}, + "value": { + "unquoted_string": { + "range": "d2/testdata/d2compiler/TestCompile/sql-regression.d2,6:11:67-6:20:76", + "value": [ + { + "string": "sql_table", + "raw_string": "sql_table" + } + ] + } + } + } + }, + { + "map_key": { + "range": "d2/testdata/d2compiler/TestCompile/sql-regression.d2,7:4:81-7:31:108", + "key": { + "range": "d2/testdata/d2compiler/TestCompile/sql-regression.d2,7:4:81-7:9:86", + "path": [ + { + "double_quoted_string": { + "range": "d2/testdata/d2compiler/TestCompile/sql-regression.d2,7:4:81-7:9:86", + "value": [ + { + "string": "***", + "raw_string": "***" + } + ] + } + } + ] + }, + "primary": {}, + "value": { + "unquoted_string": { + "range": "d2/testdata/d2compiler/TestCompile/sql-regression.d2,7:11:88-7:31:108", + "value": [ + { + "string": "not in terraform yet", + "raw_string": "not in terraform yet" + } + ] + } + } + } + } + ] + } + } + } + }, + { + "map_key": { + "range": "d2/testdata/d2compiler/TestCompile/sql-regression.d2,10:2:116-10:7:121", + "key": { + "range": "d2/testdata/d2compiler/TestCompile/sql-regression.d2,10:2:116-10:7:121", + "path": [ + { + "unquoted_string": { + "range": "d2/testdata/d2compiler/TestCompile/sql-regression.d2,10:2:116-10:7:121", + "value": [ + { + "string": "thing", + "raw_string": "thing" + } + ] + } + } + ] + }, + "primary": {}, + "value": {} + } + } + ] + } + } + } + } + ] + }, + "root": { + "id": "", + "id_val": "", + "label_dimensions": { + "width": 0, + "height": 0 + }, + "attributes": { + "label": { + "value": "" + }, + "style": {}, + "near_key": null, + "shape": { + "value": "" + }, + "direction": { + "value": "" + } + }, + "zIndex": 0 + }, + "edges": [], + "objects": [ + { + "id": "a", + "id_val": "a", + "label_dimensions": { + "width": 0, + "height": 0 + }, + "references": [ + { + "key": { + "range": "d2/testdata/d2compiler/TestCompile/sql-regression.d2,0:0:0-0:1:1", + "path": [ + { + "unquoted_string": { + "range": "d2/testdata/d2compiler/TestCompile/sql-regression.d2,0:0:0-0:1:1", + "value": [ + { + "string": "a", + "raw_string": "a" + } + ] + } + } + ] + }, + "key_path_index": 0, + "map_key_edge_index": 0 + } + ], + "attributes": { + "label": { + "value": "a" + }, + "style": { + "fill": { + "value": "lemonchiffon" + } + }, + "near_key": null, + "shape": { + "value": "" + }, + "direction": { + "value": "" + } + }, + "zIndex": 0 + }, + { + "id": "legend", + "id_val": "legend", + "label_dimensions": { + "width": 0, + "height": 0 + }, + "references": [ + { + "key": { + "range": "d2/testdata/d2compiler/TestCompile/sql-regression.d2,5:2:46-5:8:52", + "path": [ + { + "unquoted_string": { + "range": "d2/testdata/d2compiler/TestCompile/sql-regression.d2,5:2:46-5:8:52", + "value": [ + { + "string": "legend", + "raw_string": "legend" + } + ] + } + } + ] + }, + "key_path_index": 0, + "map_key_edge_index": 0 + } + ], + "sql_table": { + "columns": [ + { + "name": { + "label": "***", + "fontSize": 0, + "fontFamily": "", + "language": "", + "color": "", + "italic": false, + "bold": false, + "underline": false, + "labelWidth": 0, + "labelHeight": 0 + }, + "type": { + "label": "not in terraform yet", + "fontSize": 0, + "fontFamily": "", + "language": "", + "color": "", + "italic": false, + "bold": false, + "underline": false, + "labelWidth": 0, + "labelHeight": 0 + }, + "constraint": "", + "reference": "" + } + ] + }, + "attributes": { + "label": { + "value": "legend" + }, + "style": {}, + "near_key": null, + "shape": { + "value": "sql_table" + }, + "direction": { + "value": "" + } + }, + "zIndex": 0 + }, + { + "id": "\"***\"", + "id_val": "***", + "label_dimensions": { + "width": 0, + "height": 0 + }, + "references": [ + { + "key": { + "range": "d2/testdata/d2compiler/TestCompile/sql-regression.d2,7:4:81-7:9:86", + "path": [ + { + "double_quoted_string": { + "range": "d2/testdata/d2compiler/TestCompile/sql-regression.d2,7:4:81-7:9:86", + "value": [ + { + "string": "***", + "raw_string": "***" + } + ] + } + } + ] + }, + "key_path_index": 0, + "map_key_edge_index": 0 + } + ], + "attributes": { + "label": { + "value": "not in terraform yet" + }, + "style": {}, + "near_key": null, + "shape": { + "value": "" + }, + "direction": { + "value": "" + } + }, + "zIndex": 0 + }, + { + "id": "thing", + "id_val": "thing", + "label_dimensions": { + "width": 0, + "height": 0 + }, + "references": [ + { + "key": { + "range": "d2/testdata/d2compiler/TestCompile/sql-regression.d2,10:2:116-10:7:121", + "path": [ + { + "unquoted_string": { + "range": "d2/testdata/d2compiler/TestCompile/sql-regression.d2,10:2:116-10:7:121", + "value": [ + { + "string": "thing", + "raw_string": "thing" + } + ] + } + } + ] + }, + "key_path_index": 0, + "map_key_edge_index": 0 + } + ], + "attributes": { + "label": { + "value": "thing" + }, + "style": {}, + "near_key": null, + "shape": { + "value": "" + }, + "direction": { + "value": "" + } + }, + "zIndex": 0 + } + ] + }, + "err": null +} From 48973ab330052e22fce095274322f8fe77123087 Mon Sep 17 00:00:00 2001 From: Alexander Wang Date: Thu, 22 Dec 2022 21:23:17 -0800 Subject: [PATCH 2/3] test --- d2compiler/compile_test.go | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/d2compiler/compile_test.go b/d2compiler/compile_test.go index 74a91f393..8ad7785f4 100644 --- a/d2compiler/compile_test.go +++ b/d2compiler/compile_test.go @@ -1657,13 +1657,11 @@ choo: { style: { fill: lemonchiffon } - - legend: { + e: { shape: sql_table - "***": not in terraform yet + b: c } - - thing + d } `, assertions: func(t *testing.T, g *d2graph.Graph) { From 3c176ebfbc3f3840f16ad23063907f9a6ec82316 Mon Sep 17 00:00:00 2001 From: Alexander Wang Date: Thu, 22 Dec 2022 23:41:15 -0800 Subject: [PATCH 3/3] fix style compiling issue --- ci/release/changelogs/next.md | 1 + d2compiler/compile.go | 40 +++--- d2compiler/compile_test.go | 4 +- .../TestCompile/sql-regression.exp.json | 136 ++++++------------ 4 files changed, 60 insertions(+), 121 deletions(-) diff --git a/ci/release/changelogs/next.md b/ci/release/changelogs/next.md index af4479824..765c924de 100644 --- a/ci/release/changelogs/next.md +++ b/ci/release/changelogs/next.md @@ -13,3 +13,4 @@ - Fixed an issue with elk layouts accounting for edge labels as if they were placed on the side of the edge. [#483](https://github.com/terrastruct/d2/pull/483) - Fixed an issue where dagre layouts may not have enough spacing for all edge labels. [#484](https://github.com/terrastruct/d2/pull/484) - Fixed connections being clipped if they were at the very top or left edges of the diagram. [#493](https://github.com/terrastruct/d2/pull/493) +- Fixed edge case where style being defined in same scope as sql_table caused compiler to skip compiling sql_table. [#506](https://github.com/terrastruct/d2/issues/506) diff --git a/d2compiler/compile.go b/d2compiler/compile.go index 901f6bcee..819dae9e3 100644 --- a/d2compiler/compile.go +++ b/d2compiler/compile.go @@ -583,16 +583,25 @@ func (c *compiler) compileShapes(obj *d2graph.Object) { c.compileShapes(obj) } - for _, obj := range obj.ChildrenArray { - switch obj.Attributes.Shape.Value { + for i := 0; i < len(obj.ChildrenArray); i++ { + ch := obj.ChildrenArray[i] + switch ch.Attributes.Shape.Value { case d2target.ShapeClass, d2target.ShapeSQLTable: - flattenContainer(obj.Graph, obj) + flattenContainer(obj.Graph, ch) } - if obj.IDVal == "style" { - obj.Parent.Attributes.Style = obj.Attributes.Style + if ch.IDVal == "style" { + obj.Attributes.Style = ch.Attributes.Style if obj.Graph != nil { - flattenContainer(obj.Graph, obj) - removeObject(obj.Graph, obj) + flattenContainer(obj.Graph, ch) + for i := 0; i < len(obj.Graph.Objects); i++ { + if obj.Graph.Objects[i] == ch { + obj.Graph.Objects = append(obj.Graph.Objects[:i], obj.Graph.Objects[i+1:]...) + break + } + } + delete(obj.Children, ch.ID) + obj.ChildrenArray = append(obj.ChildrenArray[:i], obj.ChildrenArray[i+1:]...) + i-- } } } @@ -706,23 +715,6 @@ func (c *compiler) compileSQLTable(obj *d2graph.Object) { } } -// TODO too similar to flattenContainer, should reconcile in a refactor -func removeObject(g *d2graph.Graph, obj *d2graph.Object) { - for i := 0; i < len(obj.Graph.Objects); i++ { - if obj.Graph.Objects[i] == obj { - obj.Graph.Objects = append(obj.Graph.Objects[:i], obj.Graph.Objects[i+1:]...) - break - } - } - delete(obj.Parent.Children, obj.ID) - for i, child := range obj.Parent.ChildrenArray { - if obj == child { - obj.Parent.ChildrenArray = append(obj.Parent.ChildrenArray[:i], obj.Parent.ChildrenArray[i+1:]...) - break - } - } -} - func flattenContainer(g *d2graph.Graph, obj *d2graph.Object) { absID := obj.AbsID() diff --git a/d2compiler/compile_test.go b/d2compiler/compile_test.go index 8ad7785f4..4009aa68b 100644 --- a/d2compiler/compile_test.go +++ b/d2compiler/compile_test.go @@ -1657,9 +1657,9 @@ choo: { style: { fill: lemonchiffon } - e: { + b: { shape: sql_table - b: c + c } d } diff --git a/testdata/d2compiler/TestCompile/sql-regression.exp.json b/testdata/d2compiler/TestCompile/sql-regression.exp.json index 35e28df48..1341021c7 100644 --- a/testdata/d2compiler/TestCompile/sql-regression.exp.json +++ b/testdata/d2compiler/TestCompile/sql-regression.exp.json @@ -1,11 +1,11 @@ { "graph": { "ast": { - "range": "d2/testdata/d2compiler/TestCompile/sql-regression.d2,0:0:0-12:0:124", + "range": "d2/testdata/d2compiler/TestCompile/sql-regression.d2,0:0:0-10:0:87", "nodes": [ { "map_key": { - "range": "d2/testdata/d2compiler/TestCompile/sql-regression.d2,0:0:0-11:1:123", + "range": "d2/testdata/d2compiler/TestCompile/sql-regression.d2,0:0:0-9:1:86", "key": { "range": "d2/testdata/d2compiler/TestCompile/sql-regression.d2,0:0:0-0:1:1", "path": [ @@ -25,7 +25,7 @@ "primary": {}, "value": { "map": { - "range": "d2/testdata/d2compiler/TestCompile/sql-regression.d2,0:3:3-11:0:122", + "range": "d2/testdata/d2compiler/TestCompile/sql-regression.d2,0:3:3-9:0:85", "nodes": [ { "map_key": { @@ -91,17 +91,17 @@ }, { "map_key": { - "range": "d2/testdata/d2compiler/TestCompile/sql-regression.d2,5:2:46-8:3:112", + "range": "d2/testdata/d2compiler/TestCompile/sql-regression.d2,4:2:45-7:3:80", "key": { - "range": "d2/testdata/d2compiler/TestCompile/sql-regression.d2,5:2:46-5:8:52", + "range": "d2/testdata/d2compiler/TestCompile/sql-regression.d2,4:2:45-4:3:46", "path": [ { "unquoted_string": { - "range": "d2/testdata/d2compiler/TestCompile/sql-regression.d2,5:2:46-5:8:52", + "range": "d2/testdata/d2compiler/TestCompile/sql-regression.d2,4:2:45-4:3:46", "value": [ { - "string": "legend", - "raw_string": "legend" + "string": "b", + "raw_string": "b" } ] } @@ -111,17 +111,17 @@ "primary": {}, "value": { "map": { - "range": "d2/testdata/d2compiler/TestCompile/sql-regression.d2,5:10:54-8:2:111", + "range": "d2/testdata/d2compiler/TestCompile/sql-regression.d2,4:5:48-7:2:79", "nodes": [ { "map_key": { - "range": "d2/testdata/d2compiler/TestCompile/sql-regression.d2,6:4:60-6:20:76", + "range": "d2/testdata/d2compiler/TestCompile/sql-regression.d2,5:4:54-5:20:70", "key": { - "range": "d2/testdata/d2compiler/TestCompile/sql-regression.d2,6:4:60-6:9:65", + "range": "d2/testdata/d2compiler/TestCompile/sql-regression.d2,5:4:54-5:9:59", "path": [ { "unquoted_string": { - "range": "d2/testdata/d2compiler/TestCompile/sql-regression.d2,6:4:60-6:9:65", + "range": "d2/testdata/d2compiler/TestCompile/sql-regression.d2,5:4:54-5:9:59", "value": [ { "string": "shape", @@ -135,7 +135,7 @@ "primary": {}, "value": { "unquoted_string": { - "range": "d2/testdata/d2compiler/TestCompile/sql-regression.d2,6:11:67-6:20:76", + "range": "d2/testdata/d2compiler/TestCompile/sql-regression.d2,5:11:61-5:20:70", "value": [ { "string": "sql_table", @@ -148,17 +148,17 @@ }, { "map_key": { - "range": "d2/testdata/d2compiler/TestCompile/sql-regression.d2,7:4:81-7:31:108", + "range": "d2/testdata/d2compiler/TestCompile/sql-regression.d2,6:4:75-6:5:76", "key": { - "range": "d2/testdata/d2compiler/TestCompile/sql-regression.d2,7:4:81-7:9:86", + "range": "d2/testdata/d2compiler/TestCompile/sql-regression.d2,6:4:75-6:5:76", "path": [ { - "double_quoted_string": { - "range": "d2/testdata/d2compiler/TestCompile/sql-regression.d2,7:4:81-7:9:86", + "unquoted_string": { + "range": "d2/testdata/d2compiler/TestCompile/sql-regression.d2,6:4:75-6:5:76", "value": [ { - "string": "***", - "raw_string": "***" + "string": "c", + "raw_string": "c" } ] } @@ -166,17 +166,7 @@ ] }, "primary": {}, - "value": { - "unquoted_string": { - "range": "d2/testdata/d2compiler/TestCompile/sql-regression.d2,7:11:88-7:31:108", - "value": [ - { - "string": "not in terraform yet", - "raw_string": "not in terraform yet" - } - ] - } - } + "value": {} } } ] @@ -186,17 +176,17 @@ }, { "map_key": { - "range": "d2/testdata/d2compiler/TestCompile/sql-regression.d2,10:2:116-10:7:121", + "range": "d2/testdata/d2compiler/TestCompile/sql-regression.d2,8:2:83-8:3:84", "key": { - "range": "d2/testdata/d2compiler/TestCompile/sql-regression.d2,10:2:116-10:7:121", + "range": "d2/testdata/d2compiler/TestCompile/sql-regression.d2,8:2:83-8:3:84", "path": [ { "unquoted_string": { - "range": "d2/testdata/d2compiler/TestCompile/sql-regression.d2,10:2:116-10:7:121", + "range": "d2/testdata/d2compiler/TestCompile/sql-regression.d2,8:2:83-8:3:84", "value": [ { - "string": "thing", - "raw_string": "thing" + "string": "d", + "raw_string": "d" } ] } @@ -287,8 +277,8 @@ "zIndex": 0 }, { - "id": "legend", - "id_val": "legend", + "id": "b", + "id_val": "b", "label_dimensions": { "width": 0, "height": 0 @@ -296,15 +286,15 @@ "references": [ { "key": { - "range": "d2/testdata/d2compiler/TestCompile/sql-regression.d2,5:2:46-5:8:52", + "range": "d2/testdata/d2compiler/TestCompile/sql-regression.d2,4:2:45-4:3:46", "path": [ { "unquoted_string": { - "range": "d2/testdata/d2compiler/TestCompile/sql-regression.d2,5:2:46-5:8:52", + "range": "d2/testdata/d2compiler/TestCompile/sql-regression.d2,4:2:45-4:3:46", "value": [ { - "string": "legend", - "raw_string": "legend" + "string": "b", + "raw_string": "b" } ] } @@ -319,7 +309,7 @@ "columns": [ { "name": { - "label": "***", + "label": "c", "fontSize": 0, "fontFamily": "", "language": "", @@ -331,7 +321,7 @@ "labelHeight": 0 }, "type": { - "label": "not in terraform yet", + "label": "", "fontSize": 0, "fontFamily": "", "language": "", @@ -349,7 +339,7 @@ }, "attributes": { "label": { - "value": "legend" + "value": "b" }, "style": {}, "near_key": null, @@ -363,8 +353,8 @@ "zIndex": 0 }, { - "id": "\"***\"", - "id_val": "***", + "id": "d", + "id_val": "d", "label_dimensions": { "width": 0, "height": 0 @@ -372,59 +362,15 @@ "references": [ { "key": { - "range": "d2/testdata/d2compiler/TestCompile/sql-regression.d2,7:4:81-7:9:86", - "path": [ - { - "double_quoted_string": { - "range": "d2/testdata/d2compiler/TestCompile/sql-regression.d2,7:4:81-7:9:86", - "value": [ - { - "string": "***", - "raw_string": "***" - } - ] - } - } - ] - }, - "key_path_index": 0, - "map_key_edge_index": 0 - } - ], - "attributes": { - "label": { - "value": "not in terraform yet" - }, - "style": {}, - "near_key": null, - "shape": { - "value": "" - }, - "direction": { - "value": "" - } - }, - "zIndex": 0 - }, - { - "id": "thing", - "id_val": "thing", - "label_dimensions": { - "width": 0, - "height": 0 - }, - "references": [ - { - "key": { - "range": "d2/testdata/d2compiler/TestCompile/sql-regression.d2,10:2:116-10:7:121", + "range": "d2/testdata/d2compiler/TestCompile/sql-regression.d2,8:2:83-8:3:84", "path": [ { "unquoted_string": { - "range": "d2/testdata/d2compiler/TestCompile/sql-regression.d2,10:2:116-10:7:121", + "range": "d2/testdata/d2compiler/TestCompile/sql-regression.d2,8:2:83-8:3:84", "value": [ { - "string": "thing", - "raw_string": "thing" + "string": "d", + "raw_string": "d" } ] } @@ -437,7 +383,7 @@ ], "attributes": { "label": { - "value": "thing" + "value": "d" }, "style": {}, "near_key": null,