fix: incorrect globals conflict handling (#745)

globals/eval_report.go

@@ -44,8 +44,10 @@ type (
 // NewEvalReport creates a new globals evaluation report.
 func NewEvalReport() EvalReport {
 	return EvalReport{
-		Globals: eval.NewObject(project.NewPath("/")),
-		Errors:  make(map[GlobalPathKey]EvalError),
+		Globals: eval.NewObject(eval.Info{
+			DefinedAt: project.NewPath("/"),
+		}),
+		Errors: make(map[GlobalPathKey]EvalError),
 	}
 }
 

globals/globals.go

@@ -16,6 +16,7 @@ package globals
 
 import (
 	"sort"
+	"strings"
 
 	hhcl "github.com/hashicorp/hcl/v2"
 	"github.com/hashicorp/hcl/v2/hclsyntax"
@@ -68,6 +69,10 @@ func (a GlobalPathKey) rootname() string {
 	return a.path[0]
 }
 
+func (a GlobalPathKey) name() string {
+	return strings.Join(a.path[:a.numPaths], ".")
+}
+
 // Load loads all the globals from the cfgdir.
 func Load(tree *config.Root, cfgdir project.Path, ctx *eval.Context) EvalReport {
 	logger := log.With().
@@ -343,11 +348,11 @@ func (le loadedExprs) eval(ctx *eval.Context) EvalReport {
 				oldValue, hasOldValue := globals.GetKeyPath(accessor.Path())
 				if hasOldValue &&
 					accessor.isattr &&
-					oldValue.ConfigOrigin().String() == sortedGlobals.origin.String() {
+					oldValue.Info().DefinedAt.Dir().String() == expr.Origin.Dir().String() {
 					pendingExprsErrs[accessor].Append(
 						errors.E(hcl.ErrTerramateSchema, expr.Range(),
 							"global.%s attribute redefined: previously defined at %s",
-							accessor.rootname(), oldValue.ConfigOrigin().String()))
+							accessor.name(), oldValue.Info().DefinedAt.String()))
 
 					continue
 				}
@@ -379,7 +384,14 @@ func (le loadedExprs) eval(ctx *eval.Context) EvalReport {
 					// expression when extending an existing object.
 					logger.Trace().Msg("setting global")
 
-					err = globals.SetAt(accessor.Path(), eval.NewValue(val, sortedGlobals.origin))
+					err = globals.SetAt(
+						accessor.Path(),
+						eval.NewValue(val,
+							eval.Info{
+								DefinedAt: expr.Origin,
+								Dir:       sortedGlobals.origin,
+							},
+						))
 					if err != nil {
 						pendingExprsErrs[accessor].Append(errors.E(err, "setting global"))
 						continue

hcl/eval/object.go

@@ -51,38 +51,47 @@ type (
 	//       }
 	//   }
 	Object struct {
-		configdir project.Path
+		origin Info
 		// Keys is a map of key names to values.
 		Keys map[string]Value
 	}
 
 	// Value is an evaluated value.
 	Value interface {
-		// ConfigOrigin is the origin of the configuration value.
-		// It's not the file where it's defined but the directory that instantiates
-		// it. As the value could be imported, in this case the ConfigOrigin is
-		// the importing directory.
-		ConfigOrigin() project.Path
+		// Info provides extra information for the value.
+		Info() Info
 
 		// IsObject tells if the value is an object.
 		IsObject() bool
 	}
 
 	// CtyValue is a wrapper for a raw cty value.
 	CtyValue struct {
-		configdir project.Path
+		origin Info
 		cty.Value
 	}
 
+	// Info provides extra information for the configuration value.
+	Info struct {
+		// Dir defines the directory from there the value is being instantiated,
+		// which means it's the scope directory (not the file where it's defined).
+		// For values that comes from imports, the Dir will be the directory
+		// which imports the value.
+		Dir project.Path
+
+		// DefinedAt provides the source file where the value is defined.
+		DefinedAt project.Path
+	}
+
 	// ObjectPath represents a path inside the object.
 	ObjectPath []string
 )
 
 // NewObject creates a new object for configdir directory.
-func NewObject(configdir project.Path) *Object {
+func NewObject(origin Info) *Object {
 	return &Object{
-		configdir: configdir,
-		Keys:      make(map[string]Value),
+		origin: origin,
+		Keys:   make(map[string]Value),
 	}
 }
 
@@ -110,9 +119,8 @@ func (obj *Object) GetKeyPath(path ObjectPath) (Value, bool) {
 	return v.(*Object).GetKeyPath(next)
 }
 
-// ConfigOrigin is the configuration directory which the root of the object comes
-// from.
-func (obj *Object) ConfigOrigin() project.Path { return obj.configdir }
+// Info provides extra information for the object value.
+func (obj *Object) Info() Info { return obj.origin }
 
 // IsObject returns true for [Object] values.
 func (obj *Object) IsObject() bool { return true }
@@ -126,7 +134,7 @@ func (obj *Object) SetFrom(values map[string]Value) *Object {
 }
 
 // SetFromCtyValues sets the object from the values map.
-func (obj *Object) SetFromCtyValues(values map[string]cty.Value, origin project.Path) *Object {
+func (obj *Object) SetFromCtyValues(values map[string]cty.Value, origin Info) *Object {
 	for k, v := range values {
 		if v.Type().IsObjectType() {
 			subtree := NewObject(origin)
@@ -145,7 +153,7 @@ func (obj *Object) SetAt(path ObjectPath, value Value) error {
 		key := path[0]
 		subobj, ok := obj.Keys[key]
 		if !ok {
-			subobj = NewObject(value.ConfigOrigin())
+			subobj = NewObject(value.Info())
 			obj.Set(key, subobj)
 		}
 		if !subobj.IsObject() {
@@ -205,7 +213,7 @@ func (obj *Object) SetAt(path ObjectPath, value Value) error {
 		// where's the definition of each value and overwrite the values if they
 		// come from a child scope or ignore new values that comes from the parent
 		// but conflicts with definitions in the child.
-		if !strings.HasPrefix(value.ConfigOrigin().String(), old.ConfigOrigin().String()) {
+		if !strings.HasPrefix(value.Info().Dir.String(), old.Info().Dir.String()) {
 			// old is from a child scope, we must recursively merge the objects
 			// or ignore the value as it was overwriten by the child scope.
 			if old.IsObject() && value.IsObject() {
@@ -271,17 +279,17 @@ func (obj *Object) String() string {
 // NewCtyValue creates a new cty.Value wrapper.
 // Note: The cty.Value val is marked with the origin path and must be unmarked
 // before use with any hashicorp API otherwise it panics.
-func NewCtyValue(val cty.Value, origin project.Path) CtyValue {
+func NewCtyValue(val cty.Value, origin Info) CtyValue {
 	val = val.Mark(origin)
 	return CtyValue{
-		configdir: origin,
-		Value:     val,
+		origin: origin,
+		Value:  val,
 	}
 }
 
 // NewValue returns a new object Value from a cty.Value.
 // Note: this is not a wrapper as it returns an [Object] if val is a cty.Object.
-func NewValue(val cty.Value, origin project.Path) Value {
+func NewValue(val cty.Value, origin Info) Value {
 	if val.Type().IsObjectType() {
 		obj := NewObject(origin)
 		obj.SetFromCtyValues(val.AsValueMap(), origin)
@@ -290,8 +298,8 @@ func NewValue(val cty.Value, origin project.Path) Value {
 	return NewCtyValue(val, origin)
 }
 
-// ConfigOrigin is the configuration directory which defines the value.
-func (v CtyValue) ConfigOrigin() project.Path { return v.configdir }
+// Info provides extra information for the value.
+func (v CtyValue) Info() Info { return v.origin }
 
 // IsObject returns false for CtyValue values.
 func (v CtyValue) IsObject() bool { return false }

hcl/eval/object_test.go

@@ -28,8 +28,12 @@ import (
 
 type strValue string
 
-func (s strValue) IsObject() bool             { return false }
-func (s strValue) ConfigOrigin() project.Path { return project.NewPath("/") }
+func (s strValue) IsObject() bool { return false }
+func (s strValue) Info() eval.Info {
+	return eval.Info{
+		Dir: project.NewPath("/"),
+	}
+}
 
 func TestCtyObjectSetAt(t *testing.T) {
 	type testcase struct {
@@ -41,8 +45,13 @@ func TestCtyObjectSetAt(t *testing.T) {
 		wantErr error
 	}
 
+	defaultOrigin := eval.Info{
+		DefinedAt: "/file.tm",
+		Dir:       "/",
+	}
+
 	newobj := func(sets ...map[string]eval.Value) *eval.Object {
-		obj := eval.NewObject(project.NewPath("/"))
+		obj := eval.NewObject(defaultOrigin)
 		for _, set := range sets {
 			obj.SetFrom(set)
 		}

stack/globals_test.go

@@ -2318,6 +2318,48 @@ func TestLoadGlobals(t *testing.T) {
 				),
 			},
 		},
+		{
+			name: "regression test for a bug which incorrectly returned ErrRedefined errors",
+			layout: []string{
+				"s:stack",
+				"d:modules",
+			},
+			configs: []hclconfig{
+				{
+					path:     "/modules",
+					filename: "imported.tm",
+					add: Doc(
+						Globals(
+							Labels("hello"),
+							Expr("world", `{
+								a = 1
+							}`),
+						),
+					),
+				},
+				{
+					path: "/",
+					add: Doc(
+						Import(
+							Str("source", `/modules/imported.tm`),
+						),
+						Globals(
+							Labels("hello", "world"),
+							Number("a", 2),
+						),
+					),
+				},
+			},
+			want: map[string]*hclwrite.Block{
+				"/stack": Globals(
+					EvalExpr(t, "hello", `{
+							world = {
+								a = 2
+							}
+						}`),
+				),
+			},
+		},
 	}
 
 	for _, tcase := range tcases {