From 4dbae3c9257d309329249df0546b08edfd971899 Mon Sep 17 00:00:00 2001 From: Alexander Wang Date: Thu, 16 Feb 2023 17:44:54 -0800 Subject: [PATCH 1/3] prevent sql table and class from having nested children --- d2compiler/compile.go | 11 + d2compiler/compile_test.go | 30 ++ .../TestCompile/edge_in_column.exp.json | 386 +----------------- .../TestCompile/no-nested-columns-2.exp.json | 12 + .../no-nested-columns-class.exp.json | 12 + .../no-nested-columns-sql-2.exp.json | 12 + .../no-nested-columns-sql.exp.json | 12 + .../TestCompile/no-nested-columns.exp.json | 12 + 8 files changed, 110 insertions(+), 377 deletions(-) create mode 100644 testdata/d2compiler/TestCompile/no-nested-columns-2.exp.json create mode 100644 testdata/d2compiler/TestCompile/no-nested-columns-class.exp.json create mode 100644 testdata/d2compiler/TestCompile/no-nested-columns-sql-2.exp.json create mode 100644 testdata/d2compiler/TestCompile/no-nested-columns-sql.exp.json create mode 100644 testdata/d2compiler/TestCompile/no-nested-columns.exp.json diff --git a/d2compiler/compile.go b/d2compiler/compile.go index 8442e54178..3bc0f26d1d 100644 --- a/d2compiler/compile.go +++ b/d2compiler/compile.go @@ -160,6 +160,17 @@ func (c *compiler) compileField(obj *d2graph.Object, f *d2ir.Field) { return } + if obj.Parent != nil { + if obj.Parent.Attributes.Shape.Value == d2target.ShapeSQLTable { + c.errorf(f.LastRef().AST(), "sql_table columns cannot have children") + return + } + if obj.Parent.Attributes.Shape.Value == d2target.ShapeClass { + c.errorf(f.LastRef().AST(), "class fields cannot have children") + return + } + } + obj = obj.EnsureChild(d2graphIDA([]string{f.Name})) if f.Primary() != nil { c.compileLabel(obj.Attributes, f) diff --git a/d2compiler/compile_test.go b/d2compiler/compile_test.go index 1ed5b18cad..fa238a2c5a 100644 --- a/d2compiler/compile_test.go +++ b/d2compiler/compile_test.go @@ -1483,6 +1483,36 @@ d2/testdata/d2compiler/TestCompile/errors/reserved_icon_style.d2:2:9: near key " shape: sql_table x: {p -> q} }`, + expErr: `d2/testdata/d2compiler/TestCompile/edge_in_column.d2:3:7: sql_table columns cannot have children +d2/testdata/d2compiler/TestCompile/edge_in_column.d2:3:12: sql_table columns cannot have children`, + }, + { + name: "no-nested-columns-sql", + + text: `x: { + shape: sql_table + a -- b.b +}`, + expErr: `d2/testdata/d2compiler/TestCompile/no-nested-columns-sql.d2:3:10: sql_table columns cannot have children`, + }, + { + name: "no-nested-columns-sql-2", + + text: `x: { + shape: sql_table + a +} +x.a.b`, + expErr: `d2/testdata/d2compiler/TestCompile/no-nested-columns-sql-2.d2:5:5: sql_table columns cannot have children`, + }, + { + name: "no-nested-columns-class", + + text: `x: { + shape: class + a.a +}`, + expErr: `d2/testdata/d2compiler/TestCompile/no-nested-columns-class.d2:3:5: class fields cannot have children`, }, { name: "edge_to_style", diff --git a/testdata/d2compiler/TestCompile/edge_in_column.exp.json b/testdata/d2compiler/TestCompile/edge_in_column.exp.json index 7783ac781f..4a390573c1 100644 --- a/testdata/d2compiler/TestCompile/edge_in_column.exp.json +++ b/testdata/d2compiler/TestCompile/edge_in_column.exp.json @@ -1,384 +1,16 @@ { - "graph": { - "name": "", - "ast": { - "range": "d2/testdata/d2compiler/TestCompile/edge_in_column.d2,0:0:0-3:1:39", - "nodes": [ - { - "map_key": { - "range": "d2/testdata/d2compiler/TestCompile/edge_in_column.d2,0:0:0-3:1:39", - "key": { - "range": "d2/testdata/d2compiler/TestCompile/edge_in_column.d2,0:0:0-0:1:1", - "path": [ - { - "unquoted_string": { - "range": "d2/testdata/d2compiler/TestCompile/edge_in_column.d2,0:0:0-0:1:1", - "value": [ - { - "string": "x", - "raw_string": "x" - } - ] - } - } - ] - }, - "primary": {}, - "value": { - "map": { - "range": "d2/testdata/d2compiler/TestCompile/edge_in_column.d2,0:3:3-3:0:38", - "nodes": [ - { - "map_key": { - "range": "d2/testdata/d2compiler/TestCompile/edge_in_column.d2,1:2:7-1:18:23", - "key": { - "range": "d2/testdata/d2compiler/TestCompile/edge_in_column.d2,1:2:7-1:7:12", - "path": [ - { - "unquoted_string": { - "range": "d2/testdata/d2compiler/TestCompile/edge_in_column.d2,1:2:7-1:7:12", - "value": [ - { - "string": "shape", - "raw_string": "shape" - } - ] - } - } - ] - }, - "primary": {}, - "value": { - "unquoted_string": { - "range": "d2/testdata/d2compiler/TestCompile/edge_in_column.d2,1:9:14-1:18:23", - "value": [ - { - "string": "sql_table", - "raw_string": "sql_table" - } - ] - } - } - } - }, - { - "map_key": { - "range": "d2/testdata/d2compiler/TestCompile/edge_in_column.d2,2:2:26-2:13:37", - "key": { - "range": "d2/testdata/d2compiler/TestCompile/edge_in_column.d2,2:2:26-2:3:27", - "path": [ - { - "unquoted_string": { - "range": "d2/testdata/d2compiler/TestCompile/edge_in_column.d2,2:2:26-2:3:27", - "value": [ - { - "string": "x", - "raw_string": "x" - } - ] - } - } - ] - }, - "primary": {}, - "value": { - "map": { - "range": "d2/testdata/d2compiler/TestCompile/edge_in_column.d2,2:5:29-2:12:36", - "nodes": [ - { - "map_key": { - "range": "d2/testdata/d2compiler/TestCompile/edge_in_column.d2,2:6:30-2:12:36", - "edges": [ - { - "range": "d2/testdata/d2compiler/TestCompile/edge_in_column.d2,2:6:30-2:12:36", - "src": { - "range": "d2/testdata/d2compiler/TestCompile/edge_in_column.d2,2:6:30-2:8:32", - "path": [ - { - "unquoted_string": { - "range": "d2/testdata/d2compiler/TestCompile/edge_in_column.d2,2:6:30-2:7:31", - "value": [ - { - "string": "p", - "raw_string": "p" - } - ] - } - } - ] - }, - "src_arrow": "", - "dst": { - "range": "d2/testdata/d2compiler/TestCompile/edge_in_column.d2,2:10:34-2:12:36", - "path": [ - { - "unquoted_string": { - "range": "d2/testdata/d2compiler/TestCompile/edge_in_column.d2,2:11:35-2:12:36", - "value": [ - { - "string": "q", - "raw_string": "q" - } - ] - } - } - ] - }, - "dst_arrow": ">" - } - ], - "primary": {}, - "value": {} - } - } - ] - } - } - } - } - ] - } - } - } - } - ] - }, - "root": { - "id": "", - "id_val": "", - "label_dimensions": { - "width": 0, - "height": 0 - }, - "attributes": { - "label": { - "value": "" - }, - "style": {}, - "near_key": null, - "shape": { - "value": "" - }, - "direction": { - "value": "" - }, - "constraint": { - "value": "" - } - }, - "zIndex": 0 - }, - "edges": [ - { - "index": 0, - "minWidth": 0, - "minHeight": 0, - "label_dimensions": { - "width": 0, - "height": 0 - }, - "isCurve": false, - "src_arrow": false, - "dst_arrow": true, - "references": [ - { - "map_key_edge_index": 0 - } - ], - "attributes": { - "label": { - "value": "" - }, - "style": {}, - "near_key": null, - "shape": { - "value": "" - }, - "direction": { - "value": "" - }, - "constraint": { - "value": "" - } - }, - "zIndex": 0 - } - ], - "objects": [ - { - "id": "x", - "id_val": "x", - "label_dimensions": { - "width": 0, - "height": 0 - }, - "references": [ - { - "key": { - "range": "d2/testdata/d2compiler/TestCompile/edge_in_column.d2,0:0:0-0:1:1", - "path": [ - { - "unquoted_string": { - "range": "d2/testdata/d2compiler/TestCompile/edge_in_column.d2,0:0:0-0:1:1", - "value": [ - { - "string": "x", - "raw_string": "x" - } - ] - } - } - ] - }, - "key_path_index": 0, - "map_key_edge_index": -1 - } - ], - "sql_table": { - "columns": [ - { - "name": { - "label": "x", - "fontSize": 0, - "fontFamily": "", - "language": "", - "color": "", - "italic": false, - "bold": false, - "underline": false, - "labelWidth": 0, - "labelHeight": 0 - }, - "type": { - "label": "", - "fontSize": 0, - "fontFamily": "", - "language": "", - "color": "", - "italic": false, - "bold": false, - "underline": false, - "labelWidth": 0, - "labelHeight": 0 - }, - "constraint": "", - "reference": "" - } - ] - }, - "attributes": { - "label": { - "value": "x" - }, - "style": {}, - "near_key": null, - "shape": { - "value": "sql_table" - }, - "direction": { - "value": "" - }, - "constraint": { - "value": "" - } - }, - "zIndex": 0 - }, + "graph": null, + "err": { + "ioerr": null, + "errs": [ { - "id": "p", - "id_val": "p", - "label_dimensions": { - "width": 0, - "height": 0 - }, - "references": [ - { - "key": { - "range": "d2/testdata/d2compiler/TestCompile/edge_in_column.d2,2:6:30-2:8:32", - "path": [ - { - "unquoted_string": { - "range": "d2/testdata/d2compiler/TestCompile/edge_in_column.d2,2:6:30-2:7:31", - "value": [ - { - "string": "p", - "raw_string": "p" - } - ] - } - } - ] - }, - "key_path_index": 0, - "map_key_edge_index": 0 - } - ], - "attributes": { - "label": { - "value": "p" - }, - "style": {}, - "near_key": null, - "shape": { - "value": "rectangle" - }, - "direction": { - "value": "" - }, - "constraint": { - "value": "" - } - }, - "zIndex": 0 + "range": "d2/testdata/d2compiler/TestCompile/edge_in_column.d2,2:6:30-2:7:31", + "errmsg": "d2/testdata/d2compiler/TestCompile/edge_in_column.d2:3:7: sql_table columns cannot have children" }, { - "id": "q", - "id_val": "q", - "label_dimensions": { - "width": 0, - "height": 0 - }, - "references": [ - { - "key": { - "range": "d2/testdata/d2compiler/TestCompile/edge_in_column.d2,2:10:34-2:12:36", - "path": [ - { - "unquoted_string": { - "range": "d2/testdata/d2compiler/TestCompile/edge_in_column.d2,2:11:35-2:12:36", - "value": [ - { - "string": "q", - "raw_string": "q" - } - ] - } - } - ] - }, - "key_path_index": 0, - "map_key_edge_index": 0 - } - ], - "attributes": { - "label": { - "value": "q" - }, - "style": {}, - "near_key": null, - "shape": { - "value": "rectangle" - }, - "direction": { - "value": "" - }, - "constraint": { - "value": "" - } - }, - "zIndex": 0 + "range": "d2/testdata/d2compiler/TestCompile/edge_in_column.d2,2:11:35-2:12:36", + "errmsg": "d2/testdata/d2compiler/TestCompile/edge_in_column.d2:3:12: sql_table columns cannot have children" } ] - }, - "err": null + } } diff --git a/testdata/d2compiler/TestCompile/no-nested-columns-2.exp.json b/testdata/d2compiler/TestCompile/no-nested-columns-2.exp.json new file mode 100644 index 0000000000..c7a0d92baa --- /dev/null +++ b/testdata/d2compiler/TestCompile/no-nested-columns-2.exp.json @@ -0,0 +1,12 @@ +{ + "graph": null, + "err": { + "ioerr": null, + "errs": [ + { + "range": "d2/testdata/d2compiler/TestCompile/no-nested-columns-2.d2,4:4:34-4:5:35", + "errmsg": "d2/testdata/d2compiler/TestCompile/no-nested-columns-2.d2:5:5: sql_table columns cannot have children" + } + ] + } +} diff --git a/testdata/d2compiler/TestCompile/no-nested-columns-class.exp.json b/testdata/d2compiler/TestCompile/no-nested-columns-class.exp.json new file mode 100644 index 0000000000..cc9546885a --- /dev/null +++ b/testdata/d2compiler/TestCompile/no-nested-columns-class.exp.json @@ -0,0 +1,12 @@ +{ + "graph": null, + "err": { + "ioerr": null, + "errs": [ + { + "range": "d2/testdata/d2compiler/TestCompile/no-nested-columns-class.d2,2:4:24-2:5:25", + "errmsg": "d2/testdata/d2compiler/TestCompile/no-nested-columns-class.d2:3:5: class fields cannot have children" + } + ] + } +} diff --git a/testdata/d2compiler/TestCompile/no-nested-columns-sql-2.exp.json b/testdata/d2compiler/TestCompile/no-nested-columns-sql-2.exp.json new file mode 100644 index 0000000000..15bef32df2 --- /dev/null +++ b/testdata/d2compiler/TestCompile/no-nested-columns-sql-2.exp.json @@ -0,0 +1,12 @@ +{ + "graph": null, + "err": { + "ioerr": null, + "errs": [ + { + "range": "d2/testdata/d2compiler/TestCompile/no-nested-columns-sql-2.d2,4:4:34-4:5:35", + "errmsg": "d2/testdata/d2compiler/TestCompile/no-nested-columns-sql-2.d2:5:5: sql_table columns cannot have children" + } + ] + } +} diff --git a/testdata/d2compiler/TestCompile/no-nested-columns-sql.exp.json b/testdata/d2compiler/TestCompile/no-nested-columns-sql.exp.json new file mode 100644 index 0000000000..782ccce94f --- /dev/null +++ b/testdata/d2compiler/TestCompile/no-nested-columns-sql.exp.json @@ -0,0 +1,12 @@ +{ + "graph": null, + "err": { + "ioerr": null, + "errs": [ + { + "range": "d2/testdata/d2compiler/TestCompile/no-nested-columns-sql.d2,2:9:33-2:10:34", + "errmsg": "d2/testdata/d2compiler/TestCompile/no-nested-columns-sql.d2:3:10: sql_table columns cannot have children" + } + ] + } +} diff --git a/testdata/d2compiler/TestCompile/no-nested-columns.exp.json b/testdata/d2compiler/TestCompile/no-nested-columns.exp.json new file mode 100644 index 0000000000..622bdebd0f --- /dev/null +++ b/testdata/d2compiler/TestCompile/no-nested-columns.exp.json @@ -0,0 +1,12 @@ +{ + "graph": null, + "err": { + "ioerr": null, + "errs": [ + { + "range": "d2/testdata/d2compiler/TestCompile/no-nested-columns.d2,2:9:33-2:10:34", + "errmsg": "d2/testdata/d2compiler/TestCompile/no-nested-columns.d2:3:10: sql_table columns cannot have children" + } + ] + } +} From 4c6353c412f1af6114dcef620b3adddfc41e6587 Mon Sep 17 00:00:00 2001 From: Alexander Wang Date: Thu, 16 Feb 2023 17:45:16 -0800 Subject: [PATCH 2/3] tests --- .../TestCompile/no-nested-columns-2.exp.json | 12 ------------ .../TestCompile/no-nested-columns.exp.json | 12 ------------ 2 files changed, 24 deletions(-) delete mode 100644 testdata/d2compiler/TestCompile/no-nested-columns-2.exp.json delete mode 100644 testdata/d2compiler/TestCompile/no-nested-columns.exp.json diff --git a/testdata/d2compiler/TestCompile/no-nested-columns-2.exp.json b/testdata/d2compiler/TestCompile/no-nested-columns-2.exp.json deleted file mode 100644 index c7a0d92baa..0000000000 --- a/testdata/d2compiler/TestCompile/no-nested-columns-2.exp.json +++ /dev/null @@ -1,12 +0,0 @@ -{ - "graph": null, - "err": { - "ioerr": null, - "errs": [ - { - "range": "d2/testdata/d2compiler/TestCompile/no-nested-columns-2.d2,4:4:34-4:5:35", - "errmsg": "d2/testdata/d2compiler/TestCompile/no-nested-columns-2.d2:5:5: sql_table columns cannot have children" - } - ] - } -} diff --git a/testdata/d2compiler/TestCompile/no-nested-columns.exp.json b/testdata/d2compiler/TestCompile/no-nested-columns.exp.json deleted file mode 100644 index 622bdebd0f..0000000000 --- a/testdata/d2compiler/TestCompile/no-nested-columns.exp.json +++ /dev/null @@ -1,12 +0,0 @@ -{ - "graph": null, - "err": { - "ioerr": null, - "errs": [ - { - "range": "d2/testdata/d2compiler/TestCompile/no-nested-columns.d2,2:9:33-2:10:34", - "errmsg": "d2/testdata/d2compiler/TestCompile/no-nested-columns.d2:3:10: sql_table columns cannot have children" - } - ] - } -} From 91ee64f3350e5c358a9f8a01074e8077b4012664 Mon Sep 17 00:00:00 2001 From: Alexander Wang Date: Thu, 16 Feb 2023 17:46:31 -0800 Subject: [PATCH 3/3] changelog --- ci/release/changelogs/next.md | 1 + 1 file changed, 1 insertion(+) diff --git a/ci/release/changelogs/next.md b/ci/release/changelogs/next.md index 5aee5ea42c..59bb6569de 100644 --- a/ci/release/changelogs/next.md +++ b/ci/release/changelogs/next.md @@ -12,3 +12,4 @@ - Fixes edge case where layouts with dagre show a connection from the bottom side of shapes being slightly disconnected from the shape. [#820](https://github.com/terrastruct/d2/pull/820) - Fixes rare compiler bug when using underscores in edges to create objects across containers. [#824](https://github.com/terrastruct/d2/pull/824) - Fixes rare possibility of rendered connections being hidden or cut off. [#828](https://github.com/terrastruct/d2/pull/828) +- Creating nested children within `sql_table` and `class` shapes are now prevented (caused confusion when accidentally done). [#834](https://github.com/terrastruct/d2/pull/834)