diff --git a/decoder/expression_candidates.go b/decoder/expression_candidates.go index f5118f15..194343ae 100644 --- a/decoder/expression_candidates.go +++ b/decoder/expression_candidates.go @@ -478,22 +478,23 @@ func (d *PathDecoder) candidatesForTraversalConstraint(tc schema.TraversalExpr, prefix, _ := d.bytesFromRange(prefixRng) - d.pathCtx.ReferenceTargets.MatchWalk(tc, string(prefix), func(ref reference.Target) error { + 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) - if ref.RangePtr != nil && outerBodyRng.Filename == ref.RangePtr.Filename && - (outerBodyRng.ContainsPos(ref.RangePtr.Start) || - posEqual(outerBodyRng.End, ref.RangePtr.End)) { + // TODO: Reflect LocalAddr here + if referenceTargetIsInRange(target, outerBodyRng) { return nil } + address := target.Address().String() + candidates = append(candidates, lang.Candidate{ - Label: ref.Addr.String(), - Detail: ref.FriendlyName(), - Description: ref.Description, + Label: address, + Detail: target.FriendlyName(), + Description: target.Description, Kind: lang.TraversalCandidateKind, TextEdit: lang.TextEdit{ - NewText: ref.Addr.String(), - Snippet: ref.Addr.String(), + NewText: address, + Snippet: address, Range: editRng, }, }) @@ -503,6 +504,13 @@ func (d *PathDecoder) candidatesForTraversalConstraint(tc schema.TraversalExpr, return candidates } +func referenceTargetIsInRange(target reference.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 newTextForConstraints(cons schema.ExprConstraints, isNested bool) string { for _, constraint := range cons { switch c := constraint.(type) { diff --git a/decoder/hover.go b/decoder/hover.go index 4109cded..77afb0d8 100644 --- a/decoder/hover.go +++ b/decoder/hover.go @@ -638,7 +638,7 @@ func (d *PathDecoder) hoverContentForTraversalExpr(traversal hcl.Traversal, tes return "", nil } - targets, ok := d.pathCtx.ReferenceTargets.Match(origin.Address(), origin.OriginConstraints()) + targets, ok := d.pathCtx.ReferenceTargets.Match(origin) if !ok { return "", &reference.NoTargetFound{} } @@ -648,7 +648,7 @@ func (d *PathDecoder) hoverContentForTraversalExpr(traversal hcl.Traversal, tes } func hoverContentForReferenceTarget(ref reference.Target) (string, error) { - content := fmt.Sprintf("`%s`", ref.Addr.String()) + content := fmt.Sprintf("`%s`", ref.Address()) var friendlyName string if ref.Type != cty.NilType { diff --git a/decoder/reference_targets.go b/decoder/reference_targets.go index e87862d0..09d5cd5b 100644 --- a/decoder/reference_targets.go +++ b/decoder/reference_targets.go @@ -55,7 +55,7 @@ func (d *Decoder) ReferenceTargetsForOriginAtPos(path lang.Path, file string, po if !ok { continue } - targets, ok := targetCtx.ReferenceTargets.Match(matchableOrigin.Address(), matchableOrigin.OriginConstraints()) + targets, ok := targetCtx.ReferenceTargets.Match(matchableOrigin) if !ok { // target not found continue diff --git a/decoder/semantic_tokens.go b/decoder/semantic_tokens.go index 29eb2a86..c6e6d996 100644 --- a/decoder/semantic_tokens.go +++ b/decoder/semantic_tokens.go @@ -207,7 +207,7 @@ func (d *PathDecoder) tokensForExpression(ctx context.Context, expr hclsyntax.Ex return tokens } - _, targetFound := d.pathCtx.ReferenceTargets.Match(origin.Address(), origin.OriginConstraints()) + _, targetFound := d.pathCtx.ReferenceTargets.Match(origin) if !targetFound { return tokens } diff --git a/lang/address.go b/lang/address.go index bfa07b95..c4097b45 100644 --- a/lang/address.go +++ b/lang/address.go @@ -9,6 +9,13 @@ import ( type Address []AddressStep func (a Address) Equals(addr Address) bool { + // Empty address may come up in context where there are + // two addresses for the same target and only is declared + // (LocalAddr / Addr) in which case we don't want the empty + // one to be treated as a match. + if len(a) == 0 && len(addr) == 0 { + return false + } if len(a) != len(addr) { return false } diff --git a/reference/origins.go b/reference/origins.go index 21de9559..6b328593 100644 --- a/reference/origins.go +++ b/reference/origins.go @@ -37,11 +37,11 @@ func (ro Origins) Match(localPath lang.Path, target Target, targetPath lang.Path for _, refOrigin := range ro { switch origin := refOrigin.(type) { case LocalOrigin: - if localPath.Equals(targetPath) && target.Matches(origin.Address(), origin.OriginConstraints()) { + if localPath.Equals(targetPath) && target.Matches(origin) { origins = append(origins, refOrigin) } case PathOrigin: - if origin.TargetPath.Equals(targetPath) && target.Matches(origin.Address(), origin.OriginConstraints()) { + if origin.TargetPath.Equals(targetPath) && target.Matches(origin) { origins = append(origins, refOrigin) } } diff --git a/reference/target.go b/reference/target.go index fef915b6..b1b3ddc6 100644 --- a/reference/target.go +++ b/reference/target.go @@ -8,7 +8,28 @@ import ( ) type Target struct { - Addr lang.Address + // Addr represents the address of the target, as available + // elsewhere in the configuration + Addr lang.Address + + // LocalAddr represents the address of the target + // as available *locally* (e.g. self.attr_name) + LocalAddr lang.Address + + // TargetableFromRangePtr defines where the target is targetable from. + // This is considered when matching the target against origin. + // + // e.g. count.index is only available within the body of the block + // where count is declared (and extension enabled) + TargetableFromRangePtr *hcl.Range + + // ScopeId provides scope for matching/filtering + // (in addition to Type & Addr/LocalAddr). + // + // There should never be two targets with the same Type & address, + // but there are contexts (e.g. completion) where we don't filter + // by address and may not have type either (e.g. because targets + // are type-unaware). ScopeId lang.ScopeId // RangePtr represents range of the whole attribute or block @@ -31,16 +52,38 @@ type Target struct { NestedTargets Targets } +// rangeOverlaps is a copy of hcl.Range.Overlaps +// https://github.com/hashicorp/hcl/blob/v2.14.1/pos.go#L195-L212 +// which accounts for empty ranges that are common in the context of LS +func rangeOverlaps(one, other hcl.Range) bool { + switch { + case one.Filename != other.Filename: + // If the ranges are in different files then they can't possibly overlap + return false + case one.Empty() && other.Empty(): + // Empty ranges can never overlap + return false + case one.ContainsOffset(other.Start.Byte) || one.ContainsOffset(other.End.Byte): + return true + case other.ContainsOffset(one.Start.Byte) || other.ContainsOffset(one.End.Byte): + return true + default: + return false + } +} + func (ref Target) Copy() Target { return Target{ - Addr: ref.Addr, - ScopeId: ref.ScopeId, - RangePtr: copyHclRangePtr(ref.RangePtr), - DefRangePtr: copyHclRangePtr(ref.DefRangePtr), - Type: ref.Type, // cty.Type is immutable by design - Name: ref.Name, - Description: ref.Description, - NestedTargets: ref.NestedTargets.Copy(), + Addr: ref.Addr, + LocalAddr: ref.LocalAddr, + TargetableFromRangePtr: copyHclRangePtr(ref.TargetableFromRangePtr), + ScopeId: ref.ScopeId, + RangePtr: copyHclRangePtr(ref.RangePtr), + DefRangePtr: copyHclRangePtr(ref.DefRangePtr), + Type: ref.Type, // cty.Type is immutable by design + Name: ref.Name, + Description: ref.Description, + NestedTargets: ref.NestedTargets.Copy(), } } @@ -51,8 +94,16 @@ func copyHclRangePtr(rng *hcl.Range) *hcl.Range { return rng.Ptr() } +// 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 { - return r.Addr + addr := r.Addr + if len(r.LocalAddr) > 0 { + addr = r.LocalAddr + } + + return addr } func (r Target) FriendlyName() string { @@ -98,26 +149,32 @@ func (ref Target) ConformsToType(typ cty.Type) bool { return conformsToType || (typ == cty.NilType && ref.Type == cty.NilType) } -func (target Target) Matches(addr lang.Address, cons OriginConstraints) bool { - if len(target.Addr) > len(addr) { +func (target Target) Matches(origin MatchableOrigin) bool { + if len(target.LocalAddr) > len(origin.Address()) && len(target.Addr) > len(origin.Address()) { return false } - originAddr := addr + originAddr, localOriginAddr := origin.Address(), origin.Address() matchesCons := false - if len(cons) == 0 && target.Type != cty.NilType { - matchesCons = true + // Unconstrained origins should be uncommon, but they match any target + if len(origin.OriginConstraints()) == 0 { + // As long as the target is type-aware. Type-unaware targets + // generally don't have Type, so we avoid false positive here. + if target.Type != cty.NilType { + matchesCons = true + } } - for _, cons := range cons { + for _, cons := range origin.OriginConstraints() { if !target.MatchesScopeId(cons.OfScopeId) { continue } if target.Type == cty.DynamicPseudoType { - originAddr = addr.FirstSteps(uint(len(target.Addr))) + originAddr = origin.Address().FirstSteps(uint(len(target.Addr))) + localOriginAddr = origin.Address().FirstSteps(uint(len(target.LocalAddr))) matchesCons = true continue } @@ -130,5 +187,10 @@ func (target Target) Matches(addr lang.Address, cons OriginConstraints) bool { } } - return target.Addr.Equals(originAddr) && matchesCons + // Reject origin if it's outside the targetable range + if target.TargetableFromRangePtr != nil && !rangeOverlaps(*target.TargetableFromRangePtr, origin.OriginRange()) { + return false + } + + return (target.LocalAddr.Equals(localOriginAddr) || target.Addr.Equals(originAddr)) && matchesCons } diff --git a/reference/targets.go b/reference/targets.go index 609390f0..c11b5c6f 100644 --- a/reference/targets.go +++ b/reference/targets.go @@ -4,7 +4,6 @@ import ( "errors" "strings" - "github.com/hashicorp/hcl-lang/lang" "github.com/hashicorp/hcl-lang/schema" "github.com/hashicorp/hcl/v2" ) @@ -29,7 +28,8 @@ func (r Targets) Len() int { } func (r Targets) Less(i, j int) bool { - return r[i].Addr.String() < r[j].Addr.String() + return r[i].LocalAddr.String() < r[j].LocalAddr.String() || + r[i].Addr.String() < r[j].Addr.String() } func (r Targets) Swap(i, j int) { @@ -72,9 +72,18 @@ func (w refTargetDeepWalker) walk(refTargets Targets) { } } -func (refs Targets) MatchWalk(te schema.TraversalExpr, prefix string, f TargetWalkFunc) { +func (refs Targets) MatchWalk(te schema.TraversalExpr, prefix string, originRng hcl.Range, f TargetWalkFunc) { for _, ref := range refs { - if strings.HasPrefix(ref.Addr.String(), string(prefix)) { + 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 len(ref.Addr) > 0 && strings.HasPrefix(ref.Addr.String(), prefix) { nestedMatches := ref.NestedTargets.containsMatch(te, prefix) if ref.MatchesConstraint(te) || nestedMatches { f(ref) @@ -82,13 +91,17 @@ func (refs Targets) MatchWalk(te schema.TraversalExpr, prefix string, f TargetWa } } - ref.NestedTargets.MatchWalk(te, prefix, f) + ref.NestedTargets.MatchWalk(te, prefix, originRng, f) } } func (refs Targets) containsMatch(te schema.TraversalExpr, prefix string) bool { for _, ref := range refs { - if strings.HasPrefix(ref.Addr.String(), string(prefix)) && + if strings.HasPrefix(ref.LocalAddr.String(), prefix) && + ref.MatchesConstraint(te) { + return true + } + if strings.HasPrefix(ref.Addr.String(), prefix) && ref.MatchesConstraint(te) { return true } @@ -101,13 +114,14 @@ func (refs Targets) containsMatch(te schema.TraversalExpr, prefix string) bool { return false } -func (refs Targets) Match(addr lang.Address, cons OriginConstraints) (Targets, bool) { +func (refs Targets) Match(origin MatchableOrigin) (Targets, bool) { matchingReferences := make(Targets, 0) refs.deepWalk(func(ref Target) error { - if ref.Matches(addr, cons) { + if ref.Matches(origin) { matchingReferences = append(matchingReferences, ref) } + return nil }, InfiniteDepth) diff --git a/reference/targets_test.go b/reference/targets_test.go index 62f878dc..af83a59a 100644 --- a/reference/targets_test.go +++ b/reference/targets_test.go @@ -196,7 +196,7 @@ func TestTargets_Match(t *testing.T) { } for i, tc := range testCases { t.Run(fmt.Sprintf("%d-%s", i, tc.name), func(t *testing.T) { - refTarget, ok := tc.targets.Match(tc.origin.Address(), tc.origin.OriginConstraints()) + refTarget, ok := tc.targets.Match(tc.origin) if !ok && tc.expectedFound { t.Fatalf("expected targetable to be found") } @@ -208,6 +208,192 @@ func TestTargets_Match(t *testing.T) { } } +func TestTargets_Match_localRefs(t *testing.T) { + testCases := []struct { + name string + targets Targets + origin MatchableOrigin + expectedTargets Targets + expectedMatch bool + }{ + { + "no targets", + Targets{}, + LocalOrigin{ + Addr: lang.Address{ + lang.RootStep{Name: "count"}, + lang.AttrStep{Name: "index"}, + }, + }, + Targets{}, + false, + }, + { + "local address", + Targets{ + { + LocalAddr: lang.Address{ + lang.RootStep{Name: "count"}, + lang.AttrStep{Name: "index"}, + }, + Type: cty.Number, + }, + }, + LocalOrigin{ + Addr: lang.Address{ + lang.RootStep{Name: "count"}, + lang.AttrStep{Name: "index"}, + }, + }, + Targets{ + { + LocalAddr: lang.Address{ + lang.RootStep{Name: "count"}, + lang.AttrStep{Name: "index"}, + }, + Type: cty.Number, + }, + }, + true, + }, + { + "local address with Constraint", + Targets{ + { + LocalAddr: lang.Address{ + lang.RootStep{Name: "count"}, + lang.AttrStep{Name: "index"}, + }, + Type: cty.Number, + }, + }, + LocalOrigin{ + Addr: lang.Address{ + lang.RootStep{Name: "count"}, + lang.AttrStep{Name: "index"}, + }, + Constraints: OriginConstraints{ + {OfType: cty.Number}, + }, + }, + Targets{ + { + LocalAddr: lang.Address{ + lang.RootStep{Name: "count"}, + lang.AttrStep{Name: "index"}, + }, + Type: cty.Number, + }, + }, + true, + }, + { + "local address with Type and TargetableFromRange positive", + Targets{ + { + LocalAddr: lang.Address{ + lang.RootStep{Name: "count"}, + lang.AttrStep{Name: "index"}, + }, + Type: cty.Number, + TargetableFromRangePtr: &hcl.Range{ + Filename: "test.tf", + Start: hcl.Pos{Line: 1, Column: 1, Byte: 0}, + End: hcl.Pos{Line: 5, Column: 1, Byte: 50}, + }, + }, + }, + LocalOrigin{ + Addr: lang.Address{ + lang.RootStep{Name: "count"}, + lang.AttrStep{Name: "index"}, + }, + Range: hcl.Range{ + Filename: "test.tf", + Start: hcl.Pos{Line: 2, Column: 1, Byte: 20}, + End: hcl.Pos{Line: 2, Column: 10, Byte: 29}, + }, + Constraints: OriginConstraints{ + {OfType: cty.Number}, + }, + }, + Targets{ + { + LocalAddr: lang.Address{ + lang.RootStep{Name: "count"}, + lang.AttrStep{Name: "index"}, + }, + Type: cty.Number, + TargetableFromRangePtr: &hcl.Range{ + Filename: "test.tf", + Start: hcl.Pos{Line: 1, Column: 1, Byte: 0}, + End: hcl.Pos{Line: 5, Column: 1, Byte: 50}, + }, + }, + }, + true, + }, + { + "local address with Type and TargetableFromRange negative", + Targets{ + { + LocalAddr: lang.Address{ + lang.RootStep{Name: "count"}, + lang.AttrStep{Name: "index"}, + }, + Type: cty.Number, + TargetableFromRangePtr: &hcl.Range{ + Filename: "test.tf", + Start: hcl.Pos{Line: 1, Column: 1, Byte: 0}, + End: hcl.Pos{Line: 5, Column: 1, Byte: 20}, + }, + }, + }, + LocalOrigin{ + Addr: lang.Address{ + lang.RootStep{Name: "count"}, + lang.AttrStep{Name: "index"}, + }, + Range: hcl.Range{ + Filename: "test.tf", + Start: hcl.Pos{Line: 2, Column: 1, Byte: 20}, + End: hcl.Pos{Line: 2, Column: 10, Byte: 29}, + }, + Constraints: OriginConstraints{ + {OfType: cty.Number}, + }, + }, + Targets{ + { + LocalAddr: lang.Address{ + lang.RootStep{Name: "count"}, + lang.AttrStep{Name: "index"}, + }, + Type: cty.Number, + TargetableFromRangePtr: &hcl.Range{ + Filename: "test.tf", + Start: hcl.Pos{Line: 1, Column: 1, Byte: 0}, + End: hcl.Pos{Line: 5, Column: 1, Byte: 20}, + }, + }, + }, + true, + }, + } + for i, tc := range testCases { + t.Run(fmt.Sprintf("%d-%s", i, tc.name), func(t *testing.T) { + refTarget, ok := tc.targets.Match(tc.origin) + if !ok && tc.expectedMatch { + t.Fatalf("expected target to be matched") + } + + if diff := cmp.Diff(tc.expectedTargets, refTarget, ctydebug.CmpOptions); diff != "" { + t.Fatalf("mismatch of reference target: %s", diff) + } + }) + } +} + func TestTargets_OutermostInFile(t *testing.T) { testCases := []struct { name string @@ -512,7 +698,124 @@ 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) - tc.targets.MatchWalk(tc.traversalConst, tc.prefix, func(t Target) error { + rng := hcl.Range{ + Filename: "test.tf", + Start: hcl.InitialPos, + End: hcl.InitialPos, + } + tc.targets.MatchWalk(tc.traversalConst, tc.prefix, rng, func(t Target) error { + targets = append(targets, t) + return nil + }) + if diff := cmp.Diff(tc.expectedTargets, targets, ctydebug.CmpOptions); diff != "" { + t.Fatalf("mismatch of targets: %s", diff) + } + }) + } +} + +func TestTargets_MatchWalk_localRefs(t *testing.T) { + testCases := []struct { + name string + targets Targets + traversalConst schema.TraversalExpr + prefix string + expectedTargets Targets + }{ + { + "no targets", + Targets{}, + schema.TraversalExpr{}, + "test", + Targets{}, + }, + { + "targets with local address only", + Targets{ + { + LocalAddr: lang.Address{ + lang.RootStep{Name: "count"}, + lang.AttrStep{Name: "index"}, + }, + }, + { + LocalAddr: lang.Address{ + lang.RootStep{Name: "foo"}, + lang.AttrStep{Name: "bar"}, + }, + }, + }, + schema.TraversalExpr{}, + "co", + Targets{ + { + LocalAddr: lang.Address{ + lang.RootStep{Name: "count"}, + lang.AttrStep{Name: "index"}, + }, + }, + }, + }, + { + "targets with mixed address", + Targets{ + { + LocalAddr: lang.Address{ + lang.RootStep{Name: "local"}, + lang.AttrStep{Name: "foobar"}, + }, + Addr: lang.Address{ + lang.RootStep{Name: "abs"}, + lang.AttrStep{Name: "foobar"}, + }, + }, + { + LocalAddr: lang.Address{ + lang.RootStep{Name: "local"}, + lang.AttrStep{Name: "boo"}, + }, + Addr: lang.Address{ + lang.RootStep{Name: "abs"}, + lang.AttrStep{Name: "boo"}, + }, + }, + }, + schema.TraversalExpr{}, + "abs", + Targets{ + { + LocalAddr: lang.Address{ + lang.RootStep{Name: "local"}, + lang.AttrStep{Name: "foobar"}, + }, + Addr: lang.Address{ + lang.RootStep{Name: "abs"}, + lang.AttrStep{Name: "foobar"}, + }, + }, + { + LocalAddr: lang.Address{ + lang.RootStep{Name: "local"}, + lang.AttrStep{Name: "boo"}, + }, + Addr: lang.Address{ + lang.RootStep{Name: "abs"}, + lang.AttrStep{Name: "boo"}, + }, + }, + }, + }, + } + + 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 { targets = append(targets, t) return nil })