diff --git a/decoder/expression_candidates.go b/decoder/expression_candidates.go index 194343ae..ed7a3db8 100644 --- a/decoder/expression_candidates.go +++ b/decoder/expression_candidates.go @@ -335,7 +335,7 @@ func (d *PathDecoder) constraintToCandidates(ctx context.Context, constraint sch candidates = append(candidates, foreachEachCandidate(editRng)...) } - candidates = append(candidates, d.candidatesForTraversalConstraint(c, outerBodyRng, prefixRng, editRng)...) + candidates = append(candidates, d.candidatesForTraversalConstraint(ctx, c, outerBodyRng, prefixRng, editRng)...) case schema.TupleConsExpr: candidates = append(candidates, lang.Candidate{ Label: fmt.Sprintf(`[%s]`, labelForConstraints(c.AnyElem)), @@ -464,7 +464,7 @@ func (d *PathDecoder) constraintToCandidates(ctx context.Context, constraint sch return candidates } -func (d *PathDecoder) candidatesForTraversalConstraint(tc schema.TraversalExpr, outerBodyRng, prefixRng, editRng hcl.Range) []lang.Candidate { +func (d *PathDecoder) candidatesForTraversalConstraint(ctx context.Context, tc schema.TraversalExpr, outermostBodyRng, prefixRng, editRng hcl.Range) []lang.Candidate { candidates := make([]lang.Candidate, 0) if d.pathCtx.ReferenceTargets == nil { @@ -478,14 +478,8 @@ func (d *PathDecoder) candidatesForTraversalConstraint(tc schema.TraversalExpr, prefix, _ := d.bytesFromRange(prefixRng) - d.pathCtx.ReferenceTargets.MatchWalk(tc, string(prefix), editRng, func(target reference.Target) error { - // avoid suggesting references to block's own fields from within (for now) - // TODO: Reflect LocalAddr here - if referenceTargetIsInRange(target, outerBodyRng) { - return nil - } - - address := target.Address().String() + d.pathCtx.ReferenceTargets.MatchWalk(ctx, tc, string(prefix), outermostBodyRng, editRng, func(target reference.Target) error { + address := target.Address(ctx).String() candidates = append(candidates, lang.Candidate{ Label: address, diff --git a/decoder/hover.go b/decoder/hover.go index a9d9ab4b..b708628a 100644 --- a/decoder/hover.go +++ b/decoder/hover.go @@ -283,7 +283,7 @@ func (d *PathDecoder) hoverDataForExpr(ctx context.Context, expr hcl.Expression, tes, ok := constraints.TraversalExprs() if ok { - content, err := d.hoverContentForTraversalExpr(e.AsTraversal(), tes, pos) + content, err := d.hoverContentForTraversalExpr(ctx, e.AsTraversal(), tes, pos) if err != nil { return nil, err } @@ -642,7 +642,7 @@ func stringValFromTemplateExpr(tplExpr *hclsyntax.TemplateExpr) (cty.Value, bool return cty.StringVal(value), true } -func (d *PathDecoder) hoverContentForTraversalExpr(traversal hcl.Traversal, tes []schema.TraversalExpr, pos hcl.Pos) (string, error) { +func (d *PathDecoder) hoverContentForTraversalExpr(ctx context.Context, traversal hcl.Traversal, tes []schema.TraversalExpr, pos hcl.Pos) (string, error) { origins, ok := d.pathCtx.ReferenceOrigins.AtPos(traversal.SourceRange().Filename, pos) if !ok { return "", &reference.NoOriginFound{} @@ -660,14 +660,14 @@ func (d *PathDecoder) hoverContentForTraversalExpr(traversal hcl.Traversal, tes } // TODO: Reflect additional found targets here? - return hoverContentForReferenceTarget(targets[0]) + return hoverContentForReferenceTarget(ctx, targets[0]) } return "", &reference.NoTargetFound{} } -func hoverContentForReferenceTarget(ref reference.Target) (string, error) { - content := fmt.Sprintf("`%s`", ref.Address()) +func hoverContentForReferenceTarget(ctx context.Context, ref reference.Target) (string, error) { + content := fmt.Sprintf("`%s`", ref.Address(ctx)) var friendlyName string if ref.Type != cty.NilType { diff --git a/reference/target.go b/reference/target.go index 1402fdf7..dd9f4a86 100644 --- a/reference/target.go +++ b/reference/target.go @@ -1,6 +1,8 @@ package reference import ( + "context" + "github.com/hashicorp/hcl-lang/lang" "github.com/hashicorp/hcl-lang/schema" "github.com/hashicorp/hcl/v2" @@ -95,9 +97,8 @@ func copyHclRangePtr(rng *hcl.Range) *hcl.Range { } // Address returns any of the two non-empty addresses -// -// TODO: Return address based on context when we have both -func (r Target) Address() lang.Address { +// depending on the provided context +func (r Target) Address(ctx context.Context) lang.Address { addr := r.Addr if len(r.LocalAddr) > 0 { addr = r.LocalAddr diff --git a/reference/targets.go b/reference/targets.go index c11b5c6f..07d32da4 100644 --- a/reference/targets.go +++ b/reference/targets.go @@ -1,6 +1,7 @@ package reference import ( + "context" "errors" "strings" @@ -72,41 +73,92 @@ func (w refTargetDeepWalker) walk(refTargets Targets) { } } -func (refs Targets) MatchWalk(te schema.TraversalExpr, prefix string, originRng hcl.Range, f TargetWalkFunc) { +func (refs Targets) MatchWalk(ctx context.Context, te schema.TraversalExpr, prefix string, outermostBodyRng, originRng hcl.Range, f TargetWalkFunc) { for _, ref := range refs { - if len(ref.LocalAddr) > 0 && strings.HasPrefix(ref.LocalAddr.String(), prefix) { - // Check if origin is inside the targetable range - if ref.TargetableFromRangePtr == nil || rangeOverlaps(*ref.TargetableFromRangePtr, originRng) { - nestedMatches := ref.NestedTargets.containsMatch(te, prefix) - if ref.MatchesConstraint(te) || nestedMatches { - f(ref) - } - } + if localTargetMatches(ctx, ref, te, prefix, outermostBodyRng, originRng) || + absTargetMatches(ctx, ref, te, prefix, outermostBodyRng, originRng) { + f(ref) + continue } - if len(ref.Addr) > 0 && strings.HasPrefix(ref.Addr.String(), prefix) { - nestedMatches := ref.NestedTargets.containsMatch(te, prefix) - if ref.MatchesConstraint(te) || nestedMatches { - f(ref) - continue + + ref.NestedTargets.MatchWalk(ctx, te, prefix, outermostBodyRng, originRng, f) + } +} + +func localTargetMatches(ctx context.Context, target Target, te schema.TraversalExpr, prefix string, outermostBodyRng, originRng hcl.Range) bool { + if len(target.LocalAddr) > 0 && strings.HasPrefix(target.LocalAddr.String(), prefix) { + hasNestedMatches := target.NestedTargets.containsMatch(ctx, te, prefix, outermostBodyRng, originRng) + + // Avoid suggesting cyclical reference to the same attribute + // unless it has nested matches - i.e. still consider reference + // to the outside block/body as valid. + // + // For example, block { foo = self } where "self" refers to the "block" + // is considered valid. The use case this is important for is + // Terraform's self references inside nested block such as "connection". + if target.RangePtr != nil && !hasNestedMatches { + if rangeOverlaps(*target.RangePtr, originRng) { + return false } + // We compare line in case the (incomplete) attribute + // ends w/ whitespace which wouldn't be included in the range + if target.RangePtr.Filename == originRng.Filename && + target.RangePtr.End.Line == originRng.Start.Line { + return false + } + } + + // Reject origins which are outside the targetable range + if target.TargetableFromRangePtr != nil && !rangeOverlaps(*target.TargetableFromRangePtr, originRng) { + return false } - ref.NestedTargets.MatchWalk(te, prefix, originRng, f) + if target.MatchesConstraint(te) || hasNestedMatches { + return true + } } + + return false } -func (refs Targets) containsMatch(te schema.TraversalExpr, prefix string) bool { +func absTargetMatches(ctx context.Context, target Target, te schema.TraversalExpr, prefix string, outermostBodyRng, originRng hcl.Range) bool { + if len(target.Addr) > 0 && strings.HasPrefix(target.Addr.String(), prefix) { + // Reject references to block's own fields from within the body + if referenceTargetIsInRange(target, outermostBodyRng) { + return false + } + + if target.MatchesConstraint(te) || target.NestedTargets.containsMatch(ctx, te, prefix, outermostBodyRng, originRng) { + return true + } + } + return false +} + +func referenceTargetIsInRange(target Target, bodyRange hcl.Range) bool { + return target.RangePtr != nil && + bodyRange.Filename == target.RangePtr.Filename && + (bodyRange.ContainsPos(target.RangePtr.Start) || + posEqual(bodyRange.End, target.RangePtr.End)) +} + +func posEqual(pos, other hcl.Pos) bool { + return pos.Line == other.Line && + pos.Column == other.Column && + pos.Byte == other.Byte +} + +func (refs Targets) containsMatch(ctx context.Context, te schema.TraversalExpr, prefix string, outermostBodyRng, originRng hcl.Range) bool { for _, ref := range refs { - if strings.HasPrefix(ref.LocalAddr.String(), prefix) && - ref.MatchesConstraint(te) { + if localTargetMatches(ctx, ref, te, prefix, outermostBodyRng, originRng) { return true } - if strings.HasPrefix(ref.Addr.String(), prefix) && - ref.MatchesConstraint(te) { + if absTargetMatches(ctx, ref, te, prefix, outermostBodyRng, originRng) { return true } + if len(ref.NestedTargets) > 0 { - if match := ref.NestedTargets.containsMatch(te, prefix); match { + if match := ref.NestedTargets.containsMatch(ctx, te, prefix, outermostBodyRng, originRng); match { return true } } diff --git a/reference/targets_test.go b/reference/targets_test.go index dadc4a5a..72e9b6e3 100644 --- a/reference/targets_test.go +++ b/reference/targets_test.go @@ -1,6 +1,7 @@ package reference import ( + "context" "fmt" "testing" @@ -612,17 +613,29 @@ func TestTargets_OutermostInFile(t *testing.T) { func TestTargets_MatchWalk(t *testing.T) { testCases := []struct { - name string - targets Targets - traversalConst schema.TraversalExpr - prefix string - expectedTargets Targets + name string + targets Targets + traversalConst schema.TraversalExpr + prefix string + outermostBodyRng hcl.Range + originRng hcl.Range + expectedTargets Targets }{ { "no targets", Targets{}, schema.TraversalExpr{}, "test", + hcl.Range{ + Filename: "test.tf", + Start: hcl.InitialPos, + End: hcl.InitialPos, + }, + hcl.Range{ + Filename: "test.tf", + Start: hcl.InitialPos, + End: hcl.InitialPos, + }, Targets{}, }, { @@ -643,6 +656,16 @@ func TestTargets_MatchWalk(t *testing.T) { }, schema.TraversalExpr{}, "", + hcl.Range{ + Filename: "test.tf", + Start: hcl.InitialPos, + End: hcl.InitialPos, + }, + hcl.Range{ + Filename: "test.tf", + Start: hcl.InitialPos, + End: hcl.InitialPos, + }, Targets{ Target{ Addr: lang.Address{ @@ -676,6 +699,16 @@ func TestTargets_MatchWalk(t *testing.T) { }, schema.TraversalExpr{}, "var.f", + hcl.Range{ + Filename: "test.tf", + Start: hcl.InitialPos, + End: hcl.InitialPos, + }, + hcl.Range{ + Filename: "test.tf", + Start: hcl.InitialPos, + End: hcl.InitialPos, + }, Targets{ Target{ Addr: lang.Address{ @@ -707,6 +740,16 @@ func TestTargets_MatchWalk(t *testing.T) { OfType: cty.String, }, "", + hcl.Range{ + Filename: "test.tf", + Start: hcl.InitialPos, + End: hcl.InitialPos, + }, + hcl.Range{ + Filename: "test.tf", + Start: hcl.InitialPos, + End: hcl.InitialPos, + }, Targets{ Target{ Addr: lang.Address{ @@ -739,6 +782,16 @@ func TestTargets_MatchWalk(t *testing.T) { OfScopeId: lang.ScopeId("green"), }, "", + hcl.Range{ + Filename: "test.tf", + Start: hcl.InitialPos, + End: hcl.InitialPos, + }, + hcl.Range{ + Filename: "test.tf", + Start: hcl.InitialPos, + End: hcl.InitialPos, + }, Targets{ Target{ Addr: lang.Address{ @@ -782,6 +835,16 @@ func TestTargets_MatchWalk(t *testing.T) { OfScopeId: lang.ScopeId("blue"), }, "", + hcl.Range{ + Filename: "test.tf", + Start: hcl.InitialPos, + End: hcl.InitialPos, + }, + hcl.Range{ + Filename: "test.tf", + Start: hcl.InitialPos, + End: hcl.InitialPos, + }, Targets{ Target{ Addr: lang.Address{ @@ -798,12 +861,8 @@ func TestTargets_MatchWalk(t *testing.T) { for i, tc := range testCases { t.Run(fmt.Sprintf("%d-%s", i, tc.name), func(t *testing.T) { targets := make(Targets, 0) - rng := hcl.Range{ - Filename: "test.tf", - Start: hcl.InitialPos, - End: hcl.InitialPos, - } - tc.targets.MatchWalk(tc.traversalConst, tc.prefix, rng, func(t Target) error { + ctx := context.Background() + tc.targets.MatchWalk(ctx, tc.traversalConst, tc.prefix, tc.outermostBodyRng, tc.originRng, func(t Target) error { targets = append(targets, t) return nil }) @@ -816,17 +875,29 @@ func TestTargets_MatchWalk(t *testing.T) { func TestTargets_MatchWalk_localRefs(t *testing.T) { testCases := []struct { - name string - targets Targets - traversalConst schema.TraversalExpr - prefix string - expectedTargets Targets + name string + targets Targets + traversalConst schema.TraversalExpr + prefix string + outermostBodyRng hcl.Range + originRng hcl.Range + expectedTargets Targets }{ { "no targets", Targets{}, schema.TraversalExpr{}, "test", + hcl.Range{ + Filename: "test.tf", + Start: hcl.InitialPos, + End: hcl.InitialPos, + }, + hcl.Range{ + Filename: "test.tf", + Start: hcl.InitialPos, + End: hcl.InitialPos, + }, Targets{}, }, { @@ -847,6 +918,16 @@ func TestTargets_MatchWalk_localRefs(t *testing.T) { }, schema.TraversalExpr{}, "co", + hcl.Range{ + Filename: "test.tf", + Start: hcl.InitialPos, + End: hcl.InitialPos, + }, + hcl.Range{ + Filename: "test.tf", + Start: hcl.InitialPos, + End: hcl.InitialPos, + }, Targets{ { LocalAddr: lang.Address{ @@ -857,7 +938,7 @@ func TestTargets_MatchWalk_localRefs(t *testing.T) { }, }, { - "targets with mixed address", + "targets with mixed address and same block", Targets{ { LocalAddr: lang.Address{ @@ -868,6 +949,11 @@ func TestTargets_MatchWalk_localRefs(t *testing.T) { lang.RootStep{Name: "abs"}, lang.AttrStep{Name: "foobar"}, }, + RangePtr: &hcl.Range{ + Filename: "test.tf", + Start: hcl.Pos{Line: 5, Column: 1, Byte: 25}, + End: hcl.Pos{Line: 5, Column: 10, Byte: 35}, + }, }, { LocalAddr: lang.Address{ @@ -881,7 +967,17 @@ func TestTargets_MatchWalk_localRefs(t *testing.T) { }, }, schema.TraversalExpr{}, - "abs", + "local", + hcl.Range{ + Filename: "test.tf", + Start: hcl.Pos{Line: 1, Column: 1, Byte: 0}, + End: hcl.Pos{Line: 10, Column: 1, Byte: 50}, + }, + hcl.Range{ + Filename: "test.tf", + Start: hcl.InitialPos, + End: hcl.InitialPos, + }, Targets{ { LocalAddr: lang.Address{ @@ -892,6 +988,11 @@ func TestTargets_MatchWalk_localRefs(t *testing.T) { lang.RootStep{Name: "abs"}, lang.AttrStep{Name: "foobar"}, }, + RangePtr: &hcl.Range{ + Filename: "test.tf", + Start: hcl.Pos{Line: 5, Column: 1, Byte: 25}, + End: hcl.Pos{Line: 5, Column: 10, Byte: 35}, + }, }, { LocalAddr: lang.Address{ @@ -905,17 +1006,167 @@ func TestTargets_MatchWalk_localRefs(t *testing.T) { }, }, }, + { + "targets matching only the local block", + Targets{ + { + LocalAddr: lang.Address{ + lang.RootStep{Name: "count"}, + lang.AttrStep{Name: "index"}, + }, + TargetableFromRangePtr: &hcl.Range{ + Filename: "test.tf", + Start: hcl.Pos{Line: 1, Column: 1, Byte: 0}, + End: hcl.Pos{Line: 10, Column: 1, Byte: 50}, + }, + }, + { + LocalAddr: lang.Address{ + lang.RootStep{Name: "count"}, + lang.AttrStep{Name: "index"}, + }, + TargetableFromRangePtr: &hcl.Range{ + Filename: "test.tf", + Start: hcl.Pos{Line: 12, Column: 1, Byte: 52}, + End: hcl.Pos{Line: 20, Column: 1, Byte: 80}, + }, + }, + }, + schema.TraversalExpr{}, + "co", + hcl.Range{ + Filename: "test.tf", + Start: hcl.InitialPos, + End: hcl.InitialPos, + }, + hcl.Range{ + Filename: "test.tf", + Start: hcl.Pos{Line: 5, Column: 1, Byte: 25}, + End: hcl.Pos{Line: 5, Column: 10, Byte: 35}, + }, + Targets{ + { + LocalAddr: lang.Address{ + lang.RootStep{Name: "count"}, + lang.AttrStep{Name: "index"}, + }, + TargetableFromRangePtr: &hcl.Range{ + Filename: "test.tf", + Start: hcl.Pos{Line: 1, Column: 1, Byte: 0}, + End: hcl.Pos{Line: 10, Column: 1, Byte: 50}, + }, + }, + }, + }, + { + // ensure that e.g. count = count.index is mismatched, e.g. in + // resource "aws_alb" "test" { + // count = count.index + // } + "target pointing to the same attribute as origin", + Targets{ + { + LocalAddr: lang.Address{ + lang.RootStep{Name: "count"}, + lang.AttrStep{Name: "index"}, + }, + DefRangePtr: &hcl.Range{ + Filename: "test.tf", + Start: hcl.Pos{Line: 2, Column: 3, Byte: 30}, + End: hcl.Pos{Line: 2, Column: 8, Byte: 35}, + }, + RangePtr: &hcl.Range{ + Filename: "test.tf", + Start: hcl.Pos{Line: 2, Column: 3, Byte: 30}, + End: hcl.Pos{Line: 2, Column: 10, Byte: 37}, + }, + TargetableFromRangePtr: &hcl.Range{ + Filename: "test.tf", + Start: hcl.Pos{Line: 1, Column: 28, Byte: 27}, + End: hcl.Pos{Line: 3, Column: 1, Byte: 39}, + }, + }, + }, + schema.TraversalExpr{}, + "", + hcl.Range{ // outermost body range + Filename: "test.tf", + Start: hcl.Pos{Line: 1, Column: 28, Byte: 27}, + End: hcl.Pos{Line: 3, Column: 1, Byte: 39}, + }, + hcl.Range{ // origin range + Filename: "test.tf", + Start: hcl.Pos{Line: 2, Column: 11, Byte: 38}, + End: hcl.Pos{Line: 2, Column: 11, Byte: 38}, + }, + Targets{}, + }, + { + // ensure that e.g. foo = self is matched, e.g. in + // resource "aws_alb" "test" { + // foo = self + // } + "target pointing to outside body of an origin", + Targets{ + { + Addr: lang.Address{ + lang.RootStep{Name: "aws_alb"}, + lang.AttrStep{Name: "test"}, + }, + LocalAddr: lang.Address{ + lang.RootStep{Name: "self"}, + }, + RangePtr: &hcl.Range{ + Filename: "test.tf", + Start: hcl.Pos{Line: 1, Column: 28, Byte: 27}, + End: hcl.Pos{Line: 3, Column: 1, Byte: 37}, + }, + TargetableFromRangePtr: &hcl.Range{ + Filename: "test.tf", + Start: hcl.Pos{Line: 1, Column: 28, Byte: 27}, + End: hcl.Pos{Line: 3, Column: 1, Byte: 37}, + }, + NestedTargets: Targets{ + { + LocalAddr: lang.Address{ + lang.RootStep{Name: "self"}, + lang.AttrStep{Name: "bar"}, + }, + RangePtr: &hcl.Range{ + Filename: "test.tf", + Start: hcl.Pos{Line: 1, Column: 28, Byte: 27}, + End: hcl.Pos{Line: 3, Column: 1, Byte: 37}, + }, + TargetableFromRangePtr: &hcl.Range{ + Filename: "test.tf", + Start: hcl.Pos{Line: 1, Column: 28, Byte: 27}, + End: hcl.Pos{Line: 3, Column: 1, Byte: 37}, + }, + }, + }, + }, + }, + schema.TraversalExpr{}, + "", + hcl.Range{ // outermost body range + Filename: "test.tf", + Start: hcl.Pos{Line: 1, Column: 28, Byte: 27}, + End: hcl.Pos{Line: 3, Column: 1, Byte: 37}, + }, + hcl.Range{ // origin range + Filename: "test.tf", + Start: hcl.Pos{Line: 2, Column: 9, Byte: 36}, + End: hcl.Pos{Line: 2, Column: 9, Byte: 36}, + }, + Targets{}, + }, } for i, tc := range testCases { t.Run(fmt.Sprintf("%d-%s", i, tc.name), func(t *testing.T) { targets := make(Targets, 0) - rng := hcl.Range{ - Filename: "test.tf", - Start: hcl.InitialPos, - End: hcl.InitialPos, - } - tc.targets.MatchWalk(tc.traversalConst, tc.prefix, rng, func(t Target) error { + ctx := context.Background() + tc.targets.MatchWalk(ctx, tc.traversalConst, tc.prefix, tc.outermostBodyRng, tc.originRng, func(t Target) error { targets = append(targets, t) return nil })