From e145ff5201411b972c5f873324ec96174ba42034 Mon Sep 17 00:00:00 2001
From: Radek Simko <radek.simko@gmail.com>
Date: Wed, 14 Apr 2021 19:52:49 +0100
Subject: [PATCH] Avoid completing duplicate label candidates

---
 decoder/candidates_test.go  | 92 +++++++++++++++++++++++++++++++++++++
 decoder/label_candidates.go | 23 +++++++++-
 2 files changed, 114 insertions(+), 1 deletion(-)

diff --git a/decoder/candidates_test.go b/decoder/candidates_test.go
index 8361c219..a58ff996 100644
--- a/decoder/candidates_test.go
+++ b/decoder/candidates_test.go
@@ -643,6 +643,98 @@ func TestDecoder_CandidatesAtPos_emptyLabel(t *testing.T) {
 	}
 }
 
+func TestDecoder_CandidatesAtPos_emptyLabel_duplicateDepKeys(t *testing.T) {
+	resourceLabelSchema := []*schema.LabelSchema{
+		{Name: "type", IsDepKey: true},
+		{Name: "name"},
+	}
+	resourceSchema := &schema.BlockSchema{
+		Labels: resourceLabelSchema,
+		Body: &schema.BodySchema{
+			Attributes: map[string]*schema.AttributeSchema{
+				"count": {Expr: schema.LiteralTypeOnly(cty.Number)},
+			},
+		},
+		DependentBody: map[schema.SchemaKey]*schema.BodySchema{
+			schema.NewSchemaKey(schema.DependencyKeys{
+				Labels: []schema.LabelDependent{
+					{Index: 0, Value: "azurerm_subnet"},
+				},
+			}): {
+				Attributes: map[string]*schema.AttributeSchema{
+					"one":   {Expr: schema.LiteralTypeOnly(cty.String), IsRequired: true},
+					"two":   {Expr: schema.LiteralTypeOnly(cty.Number)},
+					"three": {Expr: schema.LiteralTypeOnly(cty.Bool)},
+				},
+			},
+			schema.NewSchemaKey(schema.DependencyKeys{
+				Labels: []schema.LabelDependent{
+					{Index: 0, Value: "azurerm_subnet"},
+				},
+				Attributes: []schema.AttributeDependent{
+					{
+						Name: "provider",
+						Expr: schema.ExpressionValue{
+							Address: lang.Address{
+								lang.RootStep{Name: "azurerm"},
+							},
+						},
+					},
+				},
+			}): {
+				Attributes: map[string]*schema.AttributeSchema{
+					"one":   {Expr: schema.LiteralTypeOnly(cty.String), IsRequired: true},
+					"two":   {Expr: schema.LiteralTypeOnly(cty.Number)},
+					"three": {Expr: schema.LiteralTypeOnly(cty.Bool)},
+				},
+			},
+		},
+	}
+	bodySchema := &schema.BodySchema{
+		Blocks: map[string]*schema.BlockSchema{
+			"resource": resourceSchema,
+		},
+	}
+
+	cfg := []byte(`resource "" "" {
+}
+`)
+
+	d := NewDecoder()
+	d.SetSchema(bodySchema)
+	f, pDiags := hclsyntax.ParseConfig([]byte(cfg), "test.tf", hcl.InitialPos)
+	if len(pDiags) > 0 {
+		t.Fatal(pDiags)
+	}
+	err := d.LoadFile("test.tf", f)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	candidates, err := d.CandidatesAtPos("test.tf", hcl.Pos{Line: 1, Column: 11, Byte: 10})
+	if err != nil {
+		t.Fatal(err)
+	}
+	expectedCandidates := lang.CompleteCandidates([]lang.Candidate{
+		{
+			Label: "azurerm_subnet",
+			TextEdit: lang.TextEdit{
+				Range: hcl.Range{
+					Filename: "test.tf",
+					Start:    hcl.Pos{Line: 1, Column: 11, Byte: 10},
+					End:      hcl.Pos{Line: 1, Column: 11, Byte: 10},
+				},
+				NewText: "azurerm_subnet",
+				Snippet: "azurerm_subnet",
+			},
+			Kind: lang.LabelCandidateKind,
+		},
+	})
+	if diff := cmp.Diff(expectedCandidates, candidates); diff != "" {
+		t.Fatalf("unexpected candidates: %s", diff)
+	}
+}
+
 func TestDecoder_CandidatesAtPos_basic(t *testing.T) {
 	resourceLabelSchema := []*schema.LabelSchema{
 		{Name: "type", IsDepKey: true},
diff --git a/decoder/label_candidates.go b/decoder/label_candidates.go
index 460fd838..52c33585 100644
--- a/decoder/label_candidates.go
+++ b/decoder/label_candidates.go
@@ -14,6 +14,8 @@ func (d *Decoder) labelCandidatesFromDependentSchema(idx int, db map[schema.Sche
 	candidates := lang.NewCandidates()
 	count := 0
 
+	foundCandidateNames := make(map[string]bool, 0)
+
 	prefix, _ := d.bytesFromRange(prefixRng)
 
 	for schemaKey, bodySchema := range db {
@@ -32,6 +34,22 @@ func (d *Decoder) labelCandidatesFromDependentSchema(idx int, db map[schema.Sche
 				if len(prefix) > 0 && !strings.HasPrefix(label.Value, string(prefix)) {
 					continue
 				}
+
+				// Dependent keys may be duplicated where one
+				// key is labels-only and other one contains
+				// labels + attributes.
+				//
+				// Specifically in Terraform this applies to
+				// a resource type depending on 'provider' attribute.
+				//
+				// We do need such dependent keys elsewhere
+				// to know how to do completion within a block
+				// but this doesn't matter when completing the label itself
+				// unless/until we're also completing the dependent attributes.
+				if _, ok := foundCandidateNames[label.Value]; ok {
+					continue
+				}
+
 				candidates.List = append(candidates.List, lang.Candidate{
 					Label:        label.Value,
 					Kind:         lang.LabelCandidateKind,
@@ -41,10 +59,13 @@ func (d *Decoder) labelCandidatesFromDependentSchema(idx int, db map[schema.Sche
 						Snippet: label.Value,
 						Range:   editRng,
 					},
-					// TODO: AdditionalTextEdits (required fields if body is empty)
+					// TODO: AdditionalTextEdits:
+					// - prefill required fields if body is empty
+					// - prefill dependent attribute(s)
 					Detail:      bodySchema.Detail,
 					Description: bodySchema.Description,
 				})
+				foundCandidateNames[label.Value] = true
 			}
 		}
 	}