From ffe5a1fe864e84cf3d9b7355bd354d76955280f0 Mon Sep 17 00:00:00 2001 From: Alexander Wang Date: Wed, 1 Nov 2023 12:05:45 -0700 Subject: [PATCH] compiler: fix vars crashing. Change scoping behavior --- ci/release/changelogs/next.md | 2 + d2compiler/compile_test.go | 24 ++ d2ir/compile.go | 8 + .../vars/errors/split-var-usage.exp.json | 280 ++++++++++++++++++ .../vars/errors/undeclared-var-usage.exp.json | 11 + 5 files changed, 325 insertions(+) create mode 100644 testdata/d2compiler/TestCompile2/vars/errors/split-var-usage.exp.json create mode 100644 testdata/d2compiler/TestCompile2/vars/errors/undeclared-var-usage.exp.json diff --git a/ci/release/changelogs/next.md b/ci/release/changelogs/next.md index 8234ad230..83af57a92 100644 --- a/ci/release/changelogs/next.md +++ b/ci/release/changelogs/next.md @@ -4,6 +4,7 @@ - Grid cells can now contain nested edges [#1629](https://github.com/terrastruct/d2/pull/1629) - Edges can now go across constant nears, sequence diagrams, and grids including nested ones. [#1631](https://github.com/terrastruct/d2/pull/1631) +- All vars defined in a scope are accessible everywhere in that scope, i.e., an object can use a var defined after itself. [#1695](https://github.com/terrastruct/d2/pull/1695) #### Bugfixes ⛑️ @@ -15,3 +16,4 @@ - Fixes elk growing shapes with width/height set [#1679](https://github.com/terrastruct/d2/pull/1679) - Adds a compiler error when accidentally using an arrowhead on a shape [#1686](https://github.com/terrastruct/d2/pull/1686) - Correctly reports errors from invalid values set by globs. [#1691](https://github.com/terrastruct/d2/pull/1691) +- Fixes panic when spread substitution referenced a nonexistant var. [#1695](https://github.com/terrastruct/d2/pull/1695) diff --git a/d2compiler/compile_test.go b/d2compiler/compile_test.go index 70d1ba9e1..3c4a83c6f 100644 --- a/d2compiler/compile_test.go +++ b/d2compiler/compile_test.go @@ -4160,6 +4160,30 @@ mybox: { `, "") }, }, + { + name: "undeclared-var-usage", + run: func(t *testing.T) { + assertCompile(t, ` +x: { ...${v} } +`, `d2/testdata/d2compiler/TestCompile2/vars/errors/undeclared-var-usage.d2:2:4: could not resolve variable "v"`) + }, + }, + { + name: "split-var-usage", + run: func(t *testing.T) { + assertCompile(t, ` +x1 + +vars: { + v: { + style.fill: green + } +} + +x1: { ...${v} } +`, ``) + }, + }, } for _, tc := range tca { diff --git a/d2ir/compile.go b/d2ir/compile.go index 9f91c35e4..24e2e3f60 100644 --- a/d2ir/compile.go +++ b/d2ir/compile.go @@ -125,6 +125,8 @@ func (c *compiler) compileSubstitutions(m *Map, varsStack []*Map) { if f.Name == "vars" && f.Map() != nil { varsStack = append([]*Map{f.Map()}, varsStack...) } + } + for _, f := range m.Fields { if f.Primary() != nil { c.resolveSubstitutions(varsStack, f) } @@ -459,6 +461,12 @@ func (c *compiler) compileMap(dst *Map, ast, scopeAST *d2ast.Map) { Value: []d2ast.InterpolationBox{{Substitution: n.Substitution}}, }, }, + References: []*FieldReference{{ + Context_: &RefContext{ + Scope: ast, + ScopeMap: dst, + }, + }}, } dst.Fields = append(dst.Fields, f) case n.Import != nil: diff --git a/testdata/d2compiler/TestCompile2/vars/errors/split-var-usage.exp.json b/testdata/d2compiler/TestCompile2/vars/errors/split-var-usage.exp.json new file mode 100644 index 000000000..aedfa7123 --- /dev/null +++ b/testdata/d2compiler/TestCompile2/vars/errors/split-var-usage.exp.json @@ -0,0 +1,280 @@ +{ + "graph": { + "name": "", + "isFolderOnly": false, + "ast": { + "range": "d2/testdata/d2compiler/TestCompile2/vars/errors/split-var-usage.d2,0:0:0-10:0:65", + "nodes": [ + { + "map_key": { + "range": "d2/testdata/d2compiler/TestCompile2/vars/errors/split-var-usage.d2,1:0:1-1:2:3", + "key": { + "range": "d2/testdata/d2compiler/TestCompile2/vars/errors/split-var-usage.d2,1:0:1-1:2:3", + "path": [ + { + "unquoted_string": { + "range": "d2/testdata/d2compiler/TestCompile2/vars/errors/split-var-usage.d2,1:0:1-1:2:3", + "value": [ + { + "string": "x1", + "raw_string": "x1" + } + ] + } + } + ] + }, + "primary": {}, + "value": {} + } + }, + { + "map_key": { + "range": "d2/testdata/d2compiler/TestCompile2/vars/errors/split-var-usage.d2,3:0:5-7:1:47", + "key": { + "range": "d2/testdata/d2compiler/TestCompile2/vars/errors/split-var-usage.d2,3:0:5-3:4:9", + "path": [ + { + "unquoted_string": { + "range": "d2/testdata/d2compiler/TestCompile2/vars/errors/split-var-usage.d2,3:0:5-3:4:9", + "value": [ + { + "string": "vars", + "raw_string": "vars" + } + ] + } + } + ] + }, + "primary": {}, + "value": { + "map": { + "range": "d2/testdata/d2compiler/TestCompile2/vars/errors/split-var-usage.d2,3:6:11-7:1:47", + "nodes": [ + { + "map_key": { + "range": "d2/testdata/d2compiler/TestCompile2/vars/errors/split-var-usage.d2,4:2:15-6:3:45", + "key": { + "range": "d2/testdata/d2compiler/TestCompile2/vars/errors/split-var-usage.d2,4:2:15-4:3:16", + "path": [ + { + "unquoted_string": { + "range": "d2/testdata/d2compiler/TestCompile2/vars/errors/split-var-usage.d2,4:2:15-4:3:16", + "value": [ + { + "string": "v", + "raw_string": "v" + } + ] + } + } + ] + }, + "primary": {}, + "value": { + "map": { + "range": "d2/testdata/d2compiler/TestCompile2/vars/errors/split-var-usage.d2,4:5:18-6:3:45", + "nodes": [ + { + "map_key": { + "range": "d2/testdata/d2compiler/TestCompile2/vars/errors/split-var-usage.d2,5:4:24-5:21:41", + "key": { + "range": "d2/testdata/d2compiler/TestCompile2/vars/errors/split-var-usage.d2,5:4:24-5:14:34", + "path": [ + { + "unquoted_string": { + "range": "d2/testdata/d2compiler/TestCompile2/vars/errors/split-var-usage.d2,5:4:24-5:9:29", + "value": [ + { + "string": "style", + "raw_string": "style" + } + ] + } + }, + { + "unquoted_string": { + "range": "d2/testdata/d2compiler/TestCompile2/vars/errors/split-var-usage.d2,5:10:30-5:14:34", + "value": [ + { + "string": "fill", + "raw_string": "fill" + } + ] + } + } + ] + }, + "primary": {}, + "value": { + "unquoted_string": { + "range": "d2/testdata/d2compiler/TestCompile2/vars/errors/split-var-usage.d2,5:16:36-5:21:41", + "value": [ + { + "string": "green", + "raw_string": "green" + } + ] + } + } + } + } + ] + } + } + } + } + ] + } + } + } + }, + { + "map_key": { + "range": "d2/testdata/d2compiler/TestCompile2/vars/errors/split-var-usage.d2,9:0:49-9:15:64", + "key": { + "range": "d2/testdata/d2compiler/TestCompile2/vars/errors/split-var-usage.d2,9:0:49-9:2:51", + "path": [ + { + "unquoted_string": { + "range": "d2/testdata/d2compiler/TestCompile2/vars/errors/split-var-usage.d2,9:0:49-9:2:51", + "value": [ + { + "string": "x1", + "raw_string": "x1" + } + ] + } + } + ] + }, + "primary": {}, + "value": { + "map": { + "range": "d2/testdata/d2compiler/TestCompile2/vars/errors/split-var-usage.d2,9:4:53-9:15:64", + "nodes": [ + { + "substitution": { + "range": "d2/testdata/d2compiler/TestCompile2/vars/errors/split-var-usage.d2,9:6:55-9:13:62", + "spread": true, + "path": [ + { + "unquoted_string": { + "range": "d2/testdata/d2compiler/TestCompile2/vars/errors/split-var-usage.d2,9:11:60-9:12:61", + "value": [ + { + "string": "v", + "raw_string": "v" + } + ] + } + } + ] + } + } + ] + } + } + } + } + ] + }, + "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": "x1", + "id_val": "x1", + "references": [ + { + "key": { + "range": "d2/testdata/d2compiler/TestCompile2/vars/errors/split-var-usage.d2,1:0:1-1:2:3", + "path": [ + { + "unquoted_string": { + "range": "d2/testdata/d2compiler/TestCompile2/vars/errors/split-var-usage.d2,1:0:1-1:2:3", + "value": [ + { + "string": "x1", + "raw_string": "x1" + } + ] + } + } + ] + }, + "key_path_index": 0, + "map_key_edge_index": -1 + }, + { + "key": { + "range": "d2/testdata/d2compiler/TestCompile2/vars/errors/split-var-usage.d2,9:0:49-9:2:51", + "path": [ + { + "unquoted_string": { + "range": "d2/testdata/d2compiler/TestCompile2/vars/errors/split-var-usage.d2,9:0:49-9:2:51", + "value": [ + { + "string": "x1", + "raw_string": "x1" + } + ] + } + } + ] + }, + "key_path_index": 0, + "map_key_edge_index": -1 + } + ], + "attributes": { + "label": { + "value": "x1" + }, + "labelDimensions": { + "width": 0, + "height": 0 + }, + "style": { + "fill": { + "value": "green" + } + }, + "near_key": null, + "shape": { + "value": "rectangle" + }, + "direction": { + "value": "" + }, + "constraint": null + }, + "zIndex": 0 + } + ] + }, + "err": null +} diff --git a/testdata/d2compiler/TestCompile2/vars/errors/undeclared-var-usage.exp.json b/testdata/d2compiler/TestCompile2/vars/errors/undeclared-var-usage.exp.json new file mode 100644 index 000000000..27a4b42c9 --- /dev/null +++ b/testdata/d2compiler/TestCompile2/vars/errors/undeclared-var-usage.exp.json @@ -0,0 +1,11 @@ +{ + "graph": null, + "err": { + "errs": [ + { + "range": "d2/testdata/d2compiler/TestCompile2/vars/errors/undeclared-var-usage.d2,1:3:4-1:14:15", + "errmsg": "d2/testdata/d2compiler/TestCompile2/vars/errors/undeclared-var-usage.d2:2:4: could not resolve variable \"v\"" + } + ] + } +}