Skip to content

Commit

Permalink
reference: Update type matching to align w/ Terraform's (#243)
Browse files Browse the repository at this point in the history
Terraform itself basically doesn't use cty's TestConformance, but instead uses the convert.Convert function.
This makes the type comparison a lot more lenient, where it accepts basically any primitive type for any other.
It does however reject mismatch between complex types.

Other products / language servers may benefit from the stricter comparison (as implemented by TestConformance)
but we do not have enough data to understand this problem space and so we optimise for Terraform for now.
This should not hurt other products unless they have a lot of references to filter. At worst they'll get
*more* completion items and hover/semtokens for references which are not relevant, but should still get
the ones which are relevant.
  • Loading branch information
radeksimko authored Apr 4, 2023
1 parent 795a8fa commit 31f5cd3
Show file tree
Hide file tree
Showing 6 changed files with 53 additions and 17 deletions.
27 changes: 24 additions & 3 deletions decoder/expr_reference_completion_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,13 @@ func TestCompletionAtPos_exprReference(t *testing.T) {
lang.RootStep{Name: "local"},
lang.AttrStep{Name: "bar"},
},
Type: cty.List(cty.Number),
},
{
Addr: lang.Address{
lang.RootStep{Name: "local"},
lang.AttrStep{Name: "baz"},
},
Type: cty.Number,
},
},
Expand All @@ -68,6 +75,20 @@ func TestCompletionAtPos_exprReference(t *testing.T) {
},
},
},
{
Label: "local.baz",
Detail: "number",
Kind: lang.TraversalCandidateKind,
TextEdit: lang.TextEdit{
NewText: "local.baz",
Snippet: "local.baz",
Range: hcl.Range{
Filename: "test.tf",
Start: hcl.Pos{Line: 1, Column: 8, Byte: 7},
End: hcl.Pos{Line: 1, Column: 8, Byte: 7},
},
},
},
}),
},
{
Expand All @@ -85,7 +106,7 @@ func TestCompletionAtPos_exprReference(t *testing.T) {
lang.RootStep{Name: "local"},
lang.AttrStep{Name: "foo"},
},
Type: cty.String,
Type: cty.List(cty.String),
},
{
Addr: lang.Address{
Expand Down Expand Up @@ -143,7 +164,7 @@ func TestCompletionAtPos_exprReference(t *testing.T) {
lang.RootStep{Name: "local"},
lang.AttrStep{Name: "bar"},
},
Type: cty.Number,
Type: cty.List(cty.Number),
},
{
Addr: lang.Address{
Expand Down Expand Up @@ -194,7 +215,7 @@ func TestCompletionAtPos_exprReference(t *testing.T) {
lang.RootStep{Name: "local"},
lang.AttrStep{Name: "bar"},
},
Type: cty.Number,
Type: cty.List(cty.Number),
},
{
Addr: lang.Address{
Expand Down
4 changes: 2 additions & 2 deletions decoder/expression_candidates_legacy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1478,7 +1478,7 @@ func TestLegacyDecoder_CandidateAtPos_traversalExpressions(t *testing.T) {
Attributes: map[string]*schema.AttributeSchema{
"attr": {
Expr: schema.ExprConstraints{
schema.TraversalExpr{OfType: cty.String},
schema.TraversalExpr{OfType: cty.List(cty.String)},
},
},
},
Expand Down Expand Up @@ -1521,7 +1521,7 @@ func TestLegacyDecoder_CandidateAtPos_traversalExpressions(t *testing.T) {
lang.RootStep{Name: "var"},
lang.AttrStep{Name: "first"},
},
Type: cty.Bool,
Type: cty.List(cty.Number),
},
reference.Target{
Addr: lang.Address{
Expand Down
2 changes: 1 addition & 1 deletion decoder/hover_expressions_legacy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1410,7 +1410,7 @@ func TestLegacyDecoder_HoverAtPos_traversalExpressions(t *testing.T) {
lang.RootStep{Name: "var"},
lang.AttrStep{Name: "blah"},
},
Type: cty.Bool,
Type: cty.List(cty.Bool),
},
},
reference.Origins{
Expand Down
2 changes: 1 addition & 1 deletion decoder/semantic_tokens_expr_legacy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1481,7 +1481,7 @@ func TestLegacyDecoder_SemanticTokensInFile_traversalExpression(t *testing.T) {
lang.RootStep{Name: "var"},
lang.AttrStep{Name: "blah"},
},
Type: cty.Bool,
Type: cty.List(cty.Bool),
},
},
reference.Origins{
Expand Down
21 changes: 11 additions & 10 deletions reference/target.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"github.com/hashicorp/hcl-lang/schema"
"github.com/hashicorp/hcl/v2"
"github.com/zclconf/go-cty/cty"
"github.com/zclconf/go-cty/cty/convert"
)

type Target struct {
Expand Down Expand Up @@ -141,30 +142,30 @@ func (r Target) TargetRange() (hcl.Range, bool) {
}

func (target Target) MatchesConstraint(ref schema.Reference) bool {
return target.MatchesScopeId(ref.OfScopeId) && target.ConformsToType(ref.OfType)
return target.MatchesScopeId(ref.OfScopeId) && target.IsConvertibleToType(ref.OfType)
}

func (ref Target) LegacyMatchesConstraint(te schema.TraversalExpr) bool {
return ref.MatchesScopeId(te.OfScopeId) && ref.ConformsToType(te.OfType)
return ref.MatchesScopeId(te.OfScopeId) && ref.IsConvertibleToType(te.OfType)
}

func (ref Target) MatchesScopeId(scopeId lang.ScopeId) bool {
return scopeId == "" || ref.ScopeId == scopeId
}

func (ref Target) ConformsToType(typ cty.Type) bool {
conformsToType := false
func (ref Target) IsConvertibleToType(typ cty.Type) bool {
isConvertible := false
if typ != cty.NilType && ref.Type != cty.NilType {
if ref.Type == cty.DynamicPseudoType {
// anything conforms with dynamic
conformsToType = true
// anything is convertible to dynamic
isConvertible = true
}
if errs := ref.Type.TestConformance(typ); len(errs) == 0 {
conformsToType = true
if _, err := convert.Convert(cty.UnknownVal(ref.Type), typ); err == nil {
isConvertible = true
}
}

return conformsToType || (typ == cty.NilType && ref.Type == cty.NilType)
return isConvertible || (typ == cty.NilType && ref.Type == cty.NilType)
}

func (target Target) Matches(origin MatchableOrigin) bool {
Expand Down Expand Up @@ -202,7 +203,7 @@ func (target Target) Matches(origin MatchableOrigin) bool {
matchesCons = true
continue
}
if cons.OfType != cty.NilType && target.ConformsToType(cons.OfType) {
if cons.OfType != cty.NilType && target.IsConvertibleToType(cons.OfType) {
matchesCons = true
}
if cons.OfType == cty.NilType && target.Type == cty.NilType {
Expand Down
14 changes: 14 additions & 0 deletions reference/targets_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -738,6 +738,13 @@ func TestTargets_LegacyMatchWalk(t *testing.T) {
},
Type: cty.String,
},
Target{
Addr: lang.Address{
lang.RootStep{Name: "var"},
lang.AttrStep{Name: "another"},
},
Type: cty.List(cty.String),
},
},
schema.TraversalExpr{
OfType: cty.String,
Expand All @@ -754,6 +761,13 @@ func TestTargets_LegacyMatchWalk(t *testing.T) {
End: hcl.InitialPos,
},
Targets{
Target{
Addr: lang.Address{
lang.RootStep{Name: "var"},
lang.AttrStep{Name: "first"},
},
Type: cty.Bool,
},
Target{
Addr: lang.Address{
lang.RootStep{Name: "var"},
Expand Down

0 comments on commit 31f5cd3

Please sign in to comment.