diff --git a/internal/addrs/move_endpoint.go b/internal/addrs/move_endpoint.go index 70172a080..b7c529dd5 100644 --- a/internal/addrs/move_endpoint.go +++ b/internal/addrs/move_endpoint.go @@ -47,6 +47,36 @@ func (e *MoveEndpoint) String() string { return e.relSubject.String() } +func (e *MoveEndpoint) Equal(other *MoveEndpoint) bool { + switch { + case (e == nil) != (other == nil): + return false + case e == nil: + return true + default: + // Since we only use ModuleInstance and AbsResourceInstance in our + // string representation, we have no ambiguity between address types + // and can safely just compare the string representations to + // compare the relSubject values. + return e.String() == other.String() && e.SourceRange == other.SourceRange + } +} + +// MightUnifyWith returns true if it is possible that a later call to +// UnifyMoveEndpoints might succeed if given the reciever and the other +// given endpoint. +// +// This is intended for early static validation of obviously-wrong situations, +// although there are still various semantic errors that this cannot catch. +func (e *MoveEndpoint) MightUnifyWith(other *MoveEndpoint) bool { + // For our purposes here we'll just do a unify without a base module + // address, because the rules for whether unify can succeed depend + // only on the relative part of the addresses, not on which module + // they were declared in. + from, to := UnifyMoveEndpoints(RootModuleInstance, e, other) + return from != nil && to != nil +} + // ConfigMovable transforms the reciever into a ConfigMovable by resolving it // relative to the given base module, which should be the module where // the MoveEndpoint expression was found. diff --git a/internal/configs/moved.go b/internal/configs/moved.go index 891636a73..5cfbd5dfb 100644 --- a/internal/configs/moved.go +++ b/internal/configs/moved.go @@ -6,8 +6,8 @@ import ( ) type Moved struct { - From *addrs.Target - To *addrs.Target + From *addrs.MoveEndpoint + To *addrs.MoveEndpoint DeclRange hcl.Range } @@ -25,7 +25,7 @@ func decodeMovedBlock(block *hcl.Block) (*Moved, hcl.Diagnostics) { from, traversalDiags := hcl.AbsTraversalForExpr(attr.Expr) diags = append(diags, traversalDiags...) if !traversalDiags.HasErrors() { - from, fromDiags := addrs.ParseTarget(from) + from, fromDiags := addrs.ParseMoveEndpoint(from) diags = append(diags, fromDiags.ToHCL()...) moved.From = from } @@ -35,7 +35,7 @@ func decodeMovedBlock(block *hcl.Block) (*Moved, hcl.Diagnostics) { to, traversalDiags := hcl.AbsTraversalForExpr(attr.Expr) diags = append(diags, traversalDiags...) if !traversalDiags.HasErrors() { - to, toDiags := addrs.ParseTarget(to) + to, toDiags := addrs.ParseMoveEndpoint(to) diags = append(diags, toDiags.ToHCL()...) moved.To = to } @@ -43,11 +43,13 @@ func decodeMovedBlock(block *hcl.Block) (*Moved, hcl.Diagnostics) { // we can only move from a module to a module, resource to resource, etc. if !diags.HasErrors() { - if moved.To.Subject.AddrType() != moved.From.Subject.AddrType() { + if !moved.From.MightUnifyWith(moved.To) { + // We can catch some obviously-wrong combinations early here, + // but we still have other dynamic validation to do at runtime. diags = diags.Append(&hcl.Diagnostic{ Severity: hcl.DiagError, - Summary: "Invalid \"moved\" targets", - Detail: "The \"from\" and \"to\" targets must be the same address type", + Summary: "Invalid \"moved\" addresses", + Detail: "The \"from\" and \"to\" addresses must either both refer to resources or both refer to modules.", Subject: &moved.DeclRange, }) } diff --git a/internal/configs/moved_test.go b/internal/configs/moved_test.go index c662dae95..f2110de1a 100644 --- a/internal/configs/moved_test.go +++ b/internal/configs/moved_test.go @@ -48,8 +48,8 @@ func TestDecodeMovedBlock(t *testing.T) { DefRange: blockRange, }, &Moved{ - From: mustTargetFromExpr(foo_expr), - To: mustTargetFromExpr(bar_expr), + From: mustMoveEndpointFromExpr(foo_expr), + To: mustMoveEndpointFromExpr(bar_expr), DeclRange: blockRange, }, ``, @@ -72,8 +72,8 @@ func TestDecodeMovedBlock(t *testing.T) { DefRange: blockRange, }, &Moved{ - From: mustTargetFromExpr(foo_index_expr), - To: mustTargetFromExpr(bar_index_expr), + From: mustMoveEndpointFromExpr(foo_index_expr), + To: mustMoveEndpointFromExpr(bar_index_expr), DeclRange: blockRange, }, ``, @@ -96,8 +96,8 @@ func TestDecodeMovedBlock(t *testing.T) { DefRange: blockRange, }, &Moved{ - From: mustTargetFromExpr(mod_foo_expr), - To: mustTargetFromExpr(mod_bar_expr), + From: mustMoveEndpointFromExpr(mod_foo_expr), + To: mustMoveEndpointFromExpr(mod_bar_expr), DeclRange: blockRange, }, ``, @@ -116,7 +116,7 @@ func TestDecodeMovedBlock(t *testing.T) { DefRange: blockRange, }, &Moved{ - From: mustTargetFromExpr(foo_expr), + From: mustMoveEndpointFromExpr(foo_expr), DeclRange: blockRange, }, "Missing required argument", @@ -139,11 +139,11 @@ func TestDecodeMovedBlock(t *testing.T) { DefRange: blockRange, }, &Moved{ - To: mustTargetFromExpr(foo_expr), - From: mustTargetFromExpr(mod_foo_expr), + To: mustMoveEndpointFromExpr(foo_expr), + From: mustMoveEndpointFromExpr(mod_foo_expr), DeclRange: blockRange, }, - "Invalid \"moved\" targets", + "Invalid \"moved\" addresses", }, } @@ -162,23 +162,23 @@ func TestDecodeMovedBlock(t *testing.T) { t.Fatal("expected error") } - if !cmp.Equal(got, test.want) { + if !cmp.Equal(got, test.want, cmp.AllowUnexported(addrs.MoveEndpoint{})) { t.Fatalf("wrong result: %s", cmp.Diff(got, test.want)) } }) } } -func mustTargetFromExpr(expr hcl.Expression) *addrs.Target { +func mustMoveEndpointFromExpr(expr hcl.Expression) *addrs.MoveEndpoint { traversal, hcldiags := hcl.AbsTraversalForExpr(expr) if hcldiags.HasErrors() { panic(hcldiags.Errs()) } - target, diags := addrs.ParseTarget(traversal) + ep, diags := addrs.ParseMoveEndpoint(traversal) if diags.HasErrors() { panic(diags.Err()) } - return target + return ep }