Skip to content

Commit 0f2e32b

Browse files
authored
reference: refactor & fix matching of cyclical refs (#159)
* reference: refactor tests & introduce ctx * reference: refactor & fix matching (cyclical refs)
1 parent e72d293 commit 0f2e32b

File tree

5 files changed

+361
-63
lines changed

5 files changed

+361
-63
lines changed

decoder/expression_candidates.go

Lines changed: 4 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -335,7 +335,7 @@ func (d *PathDecoder) constraintToCandidates(ctx context.Context, constraint sch
335335
candidates = append(candidates, foreachEachCandidate(editRng)...)
336336
}
337337

338-
candidates = append(candidates, d.candidatesForTraversalConstraint(c, outerBodyRng, prefixRng, editRng)...)
338+
candidates = append(candidates, d.candidatesForTraversalConstraint(ctx, c, outerBodyRng, prefixRng, editRng)...)
339339
case schema.TupleConsExpr:
340340
candidates = append(candidates, lang.Candidate{
341341
Label: fmt.Sprintf(`[%s]`, labelForConstraints(c.AnyElem)),
@@ -464,7 +464,7 @@ func (d *PathDecoder) constraintToCandidates(ctx context.Context, constraint sch
464464
return candidates
465465
}
466466

467-
func (d *PathDecoder) candidatesForTraversalConstraint(tc schema.TraversalExpr, outerBodyRng, prefixRng, editRng hcl.Range) []lang.Candidate {
467+
func (d *PathDecoder) candidatesForTraversalConstraint(ctx context.Context, tc schema.TraversalExpr, outermostBodyRng, prefixRng, editRng hcl.Range) []lang.Candidate {
468468
candidates := make([]lang.Candidate, 0)
469469

470470
if d.pathCtx.ReferenceTargets == nil {
@@ -478,14 +478,8 @@ func (d *PathDecoder) candidatesForTraversalConstraint(tc schema.TraversalExpr,
478478

479479
prefix, _ := d.bytesFromRange(prefixRng)
480480

481-
d.pathCtx.ReferenceTargets.MatchWalk(tc, string(prefix), editRng, func(target reference.Target) error {
482-
// avoid suggesting references to block's own fields from within (for now)
483-
// TODO: Reflect LocalAddr here
484-
if referenceTargetIsInRange(target, outerBodyRng) {
485-
return nil
486-
}
487-
488-
address := target.Address().String()
481+
d.pathCtx.ReferenceTargets.MatchWalk(ctx, tc, string(prefix), outermostBodyRng, editRng, func(target reference.Target) error {
482+
address := target.Address(ctx).String()
489483

490484
candidates = append(candidates, lang.Candidate{
491485
Label: address,

decoder/hover.go

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -283,7 +283,7 @@ func (d *PathDecoder) hoverDataForExpr(ctx context.Context, expr hcl.Expression,
283283

284284
tes, ok := constraints.TraversalExprs()
285285
if ok {
286-
content, err := d.hoverContentForTraversalExpr(e.AsTraversal(), tes, pos)
286+
content, err := d.hoverContentForTraversalExpr(ctx, e.AsTraversal(), tes, pos)
287287
if err != nil {
288288
return nil, err
289289
}
@@ -642,7 +642,7 @@ func stringValFromTemplateExpr(tplExpr *hclsyntax.TemplateExpr) (cty.Value, bool
642642
return cty.StringVal(value), true
643643
}
644644

645-
func (d *PathDecoder) hoverContentForTraversalExpr(traversal hcl.Traversal, tes []schema.TraversalExpr, pos hcl.Pos) (string, error) {
645+
func (d *PathDecoder) hoverContentForTraversalExpr(ctx context.Context, traversal hcl.Traversal, tes []schema.TraversalExpr, pos hcl.Pos) (string, error) {
646646
origins, ok := d.pathCtx.ReferenceOrigins.AtPos(traversal.SourceRange().Filename, pos)
647647
if !ok {
648648
return "", &reference.NoOriginFound{}
@@ -660,14 +660,14 @@ func (d *PathDecoder) hoverContentForTraversalExpr(traversal hcl.Traversal, tes
660660
}
661661

662662
// TODO: Reflect additional found targets here?
663-
return hoverContentForReferenceTarget(targets[0])
663+
return hoverContentForReferenceTarget(ctx, targets[0])
664664
}
665665

666666
return "", &reference.NoTargetFound{}
667667
}
668668

669-
func hoverContentForReferenceTarget(ref reference.Target) (string, error) {
670-
content := fmt.Sprintf("`%s`", ref.Address())
669+
func hoverContentForReferenceTarget(ctx context.Context, ref reference.Target) (string, error) {
670+
content := fmt.Sprintf("`%s`", ref.Address(ctx))
671671

672672
var friendlyName string
673673
if ref.Type != cty.NilType {

reference/target.go

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
package reference
22

33
import (
4+
"context"
5+
46
"github.com/hashicorp/hcl-lang/lang"
57
"github.com/hashicorp/hcl-lang/schema"
68
"github.com/hashicorp/hcl/v2"
@@ -95,9 +97,8 @@ func copyHclRangePtr(rng *hcl.Range) *hcl.Range {
9597
}
9698

9799
// Address returns any of the two non-empty addresses
98-
//
99-
// TODO: Return address based on context when we have both
100-
func (r Target) Address() lang.Address {
100+
// depending on the provided context
101+
func (r Target) Address(ctx context.Context) lang.Address {
101102
addr := r.Addr
102103
if len(r.LocalAddr) > 0 {
103104
addr = r.LocalAddr

reference/targets.go

Lines changed: 73 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package reference
22

33
import (
4+
"context"
45
"errors"
56
"strings"
67

@@ -72,41 +73,92 @@ func (w refTargetDeepWalker) walk(refTargets Targets) {
7273
}
7374
}
7475

75-
func (refs Targets) MatchWalk(te schema.TraversalExpr, prefix string, originRng hcl.Range, f TargetWalkFunc) {
76+
func (refs Targets) MatchWalk(ctx context.Context, te schema.TraversalExpr, prefix string, outermostBodyRng, originRng hcl.Range, f TargetWalkFunc) {
7677
for _, ref := range refs {
77-
if len(ref.LocalAddr) > 0 && strings.HasPrefix(ref.LocalAddr.String(), prefix) {
78-
// Check if origin is inside the targetable range
79-
if ref.TargetableFromRangePtr == nil || rangeOverlaps(*ref.TargetableFromRangePtr, originRng) {
80-
nestedMatches := ref.NestedTargets.containsMatch(te, prefix)
81-
if ref.MatchesConstraint(te) || nestedMatches {
82-
f(ref)
83-
}
84-
}
78+
if localTargetMatches(ctx, ref, te, prefix, outermostBodyRng, originRng) ||
79+
absTargetMatches(ctx, ref, te, prefix, outermostBodyRng, originRng) {
80+
f(ref)
81+
continue
8582
}
86-
if len(ref.Addr) > 0 && strings.HasPrefix(ref.Addr.String(), prefix) {
87-
nestedMatches := ref.NestedTargets.containsMatch(te, prefix)
88-
if ref.MatchesConstraint(te) || nestedMatches {
89-
f(ref)
90-
continue
83+
84+
ref.NestedTargets.MatchWalk(ctx, te, prefix, outermostBodyRng, originRng, f)
85+
}
86+
}
87+
88+
func localTargetMatches(ctx context.Context, target Target, te schema.TraversalExpr, prefix string, outermostBodyRng, originRng hcl.Range) bool {
89+
if len(target.LocalAddr) > 0 && strings.HasPrefix(target.LocalAddr.String(), prefix) {
90+
hasNestedMatches := target.NestedTargets.containsMatch(ctx, te, prefix, outermostBodyRng, originRng)
91+
92+
// Avoid suggesting cyclical reference to the same attribute
93+
// unless it has nested matches - i.e. still consider reference
94+
// to the outside block/body as valid.
95+
//
96+
// For example, block { foo = self } where "self" refers to the "block"
97+
// is considered valid. The use case this is important for is
98+
// Terraform's self references inside nested block such as "connection".
99+
if target.RangePtr != nil && !hasNestedMatches {
100+
if rangeOverlaps(*target.RangePtr, originRng) {
101+
return false
91102
}
103+
// We compare line in case the (incomplete) attribute
104+
// ends w/ whitespace which wouldn't be included in the range
105+
if target.RangePtr.Filename == originRng.Filename &&
106+
target.RangePtr.End.Line == originRng.Start.Line {
107+
return false
108+
}
109+
}
110+
111+
// Reject origins which are outside the targetable range
112+
if target.TargetableFromRangePtr != nil && !rangeOverlaps(*target.TargetableFromRangePtr, originRng) {
113+
return false
92114
}
93115

94-
ref.NestedTargets.MatchWalk(te, prefix, originRng, f)
116+
if target.MatchesConstraint(te) || hasNestedMatches {
117+
return true
118+
}
95119
}
120+
121+
return false
96122
}
97123

98-
func (refs Targets) containsMatch(te schema.TraversalExpr, prefix string) bool {
124+
func absTargetMatches(ctx context.Context, target Target, te schema.TraversalExpr, prefix string, outermostBodyRng, originRng hcl.Range) bool {
125+
if len(target.Addr) > 0 && strings.HasPrefix(target.Addr.String(), prefix) {
126+
// Reject references to block's own fields from within the body
127+
if referenceTargetIsInRange(target, outermostBodyRng) {
128+
return false
129+
}
130+
131+
if target.MatchesConstraint(te) || target.NestedTargets.containsMatch(ctx, te, prefix, outermostBodyRng, originRng) {
132+
return true
133+
}
134+
}
135+
return false
136+
}
137+
138+
func referenceTargetIsInRange(target Target, bodyRange hcl.Range) bool {
139+
return target.RangePtr != nil &&
140+
bodyRange.Filename == target.RangePtr.Filename &&
141+
(bodyRange.ContainsPos(target.RangePtr.Start) ||
142+
posEqual(bodyRange.End, target.RangePtr.End))
143+
}
144+
145+
func posEqual(pos, other hcl.Pos) bool {
146+
return pos.Line == other.Line &&
147+
pos.Column == other.Column &&
148+
pos.Byte == other.Byte
149+
}
150+
151+
func (refs Targets) containsMatch(ctx context.Context, te schema.TraversalExpr, prefix string, outermostBodyRng, originRng hcl.Range) bool {
99152
for _, ref := range refs {
100-
if strings.HasPrefix(ref.LocalAddr.String(), prefix) &&
101-
ref.MatchesConstraint(te) {
153+
if localTargetMatches(ctx, ref, te, prefix, outermostBodyRng, originRng) {
102154
return true
103155
}
104-
if strings.HasPrefix(ref.Addr.String(), prefix) &&
105-
ref.MatchesConstraint(te) {
156+
if absTargetMatches(ctx, ref, te, prefix, outermostBodyRng, originRng) {
106157
return true
107158
}
159+
108160
if len(ref.NestedTargets) > 0 {
109-
if match := ref.NestedTargets.containsMatch(te, prefix); match {
161+
if match := ref.NestedTargets.containsMatch(ctx, te, prefix, outermostBodyRng, originRng); match {
110162
return true
111163
}
112164
}

0 commit comments

Comments
 (0)