Merge pull request #1914 from alixander/variable-scope

d2ir: fix multiple substitutions in imports

ci/release/changelogs/next.md

@@ -6,3 +6,4 @@
 
 - Fix executable plugins that implement standalone router [#1910](https://github.com/terrastruct/d2/pull/1910)
 - Fix compiler error with multiple nested spread substitutions [#1913](https://github.com/terrastruct/d2/pull/1913)
+- Fix substitutions from imports into different scopes [#1914](https://github.com/terrastruct/d2/pull/1914)

d2compiler/compile_test.go

@@ -10,6 +10,7 @@ import (
 
 	"oss.terrastruct.com/util-go/assert"
 	"oss.terrastruct.com/util-go/diff"
+	"oss.terrastruct.com/util-go/mapfs"
 
 	"oss.terrastruct.com/d2/d2compiler"
 	"oss.terrastruct.com/d2/d2format"
@@ -23,6 +24,8 @@ func TestCompile(t *testing.T) {
 	testCases := []struct {
 		name string
 		text string
+		// For tests that use imports, define `index.d2` as text and other files here
+		files map[string]string
 
 		expErr     string
 		assertions func(t *testing.T, g *d2graph.Graph)
@@ -2868,15 +2871,60 @@ d2/testdata/d2compiler/TestCompile/no_arrowheads_in_shape.d2:2:3: "source-arrowh
 			expErr: `d2/testdata/d2compiler/TestCompile/fixed-pos-shape-hierarchy.d2:4:2: position keywords cannot be used with shape "hierarchy"
 d2/testdata/d2compiler/TestCompile/fixed-pos-shape-hierarchy.d2:5:2: position keywords cannot be used with shape "hierarchy"`,
 		},
+		{
+			name: "vars-in-imports",
+			text: `dev: {
+  vars: {
+    env: Dev
+  }
+  ...@template.d2
+}
+
+qa: {
+  vars: {
+    env: Qa
+  }
+  ...@template.d2
+}
+`,
+			files: map[string]string{
+				"template.d2": `env: {
+  label: ${env} Environment
+  vm: {
+    label: My Virtual machine!
+  }
+}`,
+			},
+			assertions: func(t *testing.T, g *d2graph.Graph) {
+				tassert.Equal(t, "dev.env", g.Objects[1].AbsID())
+				tassert.Equal(t, "Dev Environment", g.Objects[1].Label.Value)
+				tassert.Equal(t, "qa.env", g.Objects[2].AbsID())
+				tassert.Equal(t, "Qa Environment", g.Objects[2].Label.Value)
+			},
+		},
 	}
 
 	for _, tc := range testCases {
 		tc := tc
 		t.Run(tc.name, func(t *testing.T) {
 			t.Parallel()
-
+			opts := &d2compiler.CompileOptions{}
+			if tc.files != nil {
+				tc.files["index.d2"] = tc.text
+				renamed := make(map[string]string)
+				for file, content := range tc.files {
+					renamed[fmt.Sprintf("d2/testdata/d2compiler/TestCompile/%v", file)] = content
+				}
+				fs, err := mapfs.New(renamed)
+				assert.Success(t, err)
+				t.Cleanup(func() {
+					err = fs.Close()
+					assert.Success(t, err)
+				})
+				opts.FS = fs
+			}
 			d2Path := fmt.Sprintf("d2/testdata/d2compiler/%v.d2", t.Name())
-			g, _, err := d2compiler.Compile(d2Path, strings.NewReader(tc.text), nil)
+			g, _, err := d2compiler.Compile(d2Path, strings.NewReader(tc.text), opts)
 			if tc.expErr != "" {
 				if err == nil {
 					t.Fatalf("expected error with: %q", tc.expErr)

d2ir/compile.go

@@ -31,8 +31,7 @@ type compiler struct {
 	imports []string
 	// importStack is used to detect cyclic imports.
 	importStack []string
-	// importCache enables reuse of files imported multiple times.
-	importCache map[string]*Map
+	seenImports map[string]struct{}
 	utf16Pos    bool
 
 	// Stack of globs that must be recomputed at each new object in and below the current scope.
@@ -62,7 +61,7 @@ func Compile(ast *d2ast.Map, opts *CompileOptions) (*Map, []string, error) {
 		err: &d2parser.ParseError{},
 		fs:  opts.FS,
 
-		importCache: make(map[string]*Map),
+		seenImports: make(map[string]struct{}),
 		utf16Pos:    opts.UTF16Pos,
 	}
 	m := &Map{}

d2ir/import.go

@@ -82,16 +82,11 @@ func (c *compiler) __import(imp *d2ast.Import) (*Map, bool) {
 
 	// Only get immediate imports.
 	if len(c.importStack) == 2 {
-		if _, ok := c.importCache[impPath]; !ok {
+		if _, ok := c.seenImports[impPath]; !ok {
 			c.imports = append(c.imports, imp.PathWithPre())
 		}
 	}
 
-	ir, ok := c.importCache[impPath]
-	if ok {
-		return ir, true
-	}
-
 	var f fs.File
 	var err error
 	if c.fs == nil {
@@ -113,13 +108,13 @@ func (c *compiler) __import(imp *d2ast.Import) (*Map, bool) {
 		return nil, false
 	}
 
-	ir = &Map{}
+	ir := &Map{}
 	ir.initRoot()
 	ir.parent.(*Field).References[0].Context_.Scope = ast
 
 	c.compileMap(ir, ast, ast)
 
-	c.importCache[impPath] = ir
+	c.seenImports[impPath] = struct{}{}
 
 	return ir, true
 }