Skip to content

Commit 1fd0df4

Browse files
Simplify prefix typo detection to use full suggestFn
Instead of only correcting the first path segment for prefix typos, use the full multi-segment suggestFn. This produces better suggestions (e.g., ${bundl.nme} → ${bundle.name}) and removes dead code. Co-authored-by: Isaac
1 parent d25423f commit 1fd0df4

3 files changed

Lines changed: 18 additions & 48 deletions

File tree

bundle/config/mutator/resolve_variable_references.go

Lines changed: 14 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -43,8 +43,10 @@ var defaultPrefixes = []string{
4343
"variables",
4444
}
4545

46-
var artifactPath = dyn.MustPathFromString("artifacts")
47-
var resourcesPath = dyn.MustPathFromString("resources")
46+
var (
47+
artifactPath = dyn.MustPathFromString("artifacts")
48+
resourcesPath = dyn.MustPathFromString("resources")
49+
)
4850

4951
type resolveVariableReferences struct {
5052
prefixes []string
@@ -259,25 +261,16 @@ func (m *resolveVariableReferences) resolveOnce(b *bundle.Bundle, prefixes []dyn
259261
return dyn.InvalidValue, dynvar.ErrSkipResolution
260262
}
261263

262-
// Check for prefix typos before skipping. If the first
263-
// component is close to a valid prefix, emit a warning
264-
// with a suggestion. The reference is left unresolved to
265-
// avoid breaking existing behavior.
266-
if len(path) > 0 {
267-
firstKey := path[0].Key()
268-
prefixNames := m.suggestPrefixNames(prefixes)
269-
best, dist := closestMatch(firstKey, prefixNames)
270-
if best != "" && dist > 0 {
271-
corrected := make(dyn.Path, len(path))
272-
copy(corrected, path)
273-
corrected[0] = dyn.Key(best)
274-
suggestion := rewriteToVarShorthand(corrected.String())
275-
key := rewriteToVarShorthand(path.String())
276-
diags = diags.Append(diag.Diagnostic{
277-
Severity: diag.Warning,
278-
Summary: fmt.Sprintf("reference does not exist: ${%s}. Did you mean ${%s}?", key, suggestion),
279-
})
280-
}
264+
// Check for prefix typos before skipping. Use the full
265+
// suggestFn to correct all segments (not just the prefix).
266+
// The reference is left unresolved to avoid breaking
267+
// existing behavior.
268+
key := rewriteToVarShorthand(path.String())
269+
if suggestion := suggestFn(key); suggestion != "" {
270+
diags = diags.Append(diag.Diagnostic{
271+
Severity: diag.Warning,
272+
Summary: fmt.Sprintf("reference does not exist: ${%s}. Did you mean ${%s}?", key, suggestion),
273+
})
281274
}
282275

283276
return dyn.InvalidValue, dynvar.ErrSkipResolution

bundle/config/mutator/suggest.go

Lines changed: 0 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -112,29 +112,6 @@ func rewriteToVarShorthand(key string) string {
112112
return key
113113
}
114114

115-
// suggestPrefixNames returns the list of prefix names used for suggestion
116-
// matching. This includes the resolution prefixes plus additional well-known
117-
// prefixes like "var" and "resources" that users commonly reference.
118-
func (m *resolveVariableReferences) suggestPrefixNames(prefixes []dyn.Path) []string {
119-
seen := map[string]bool{}
120-
var names []string
121-
for _, p := range prefixes {
122-
key := p[0].Key()
123-
if !seen[key] {
124-
seen[key] = true
125-
names = append(names, key)
126-
}
127-
}
128-
// Add well-known prefixes that may not be in the resolution set.
129-
for _, extra := range []string{"var", "resources"} {
130-
if !seen[extra] {
131-
seen[extra] = true
132-
names = append(names, extra)
133-
}
134-
}
135-
return names
136-
}
137-
138115
// makeSuggestFn builds a SuggestFn that uses Go type information and runtime
139116
// dyn values to generate suggestions for unresolved variable references.
140117
//

bundle/config/mutator/suggest_test.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,7 @@ func TestStructFieldNames(t *testing.T) {
7676

7777
assert.Contains(t, names, "embedded_field")
7878
assert.Contains(t, names, "name")
79-
assert.Contains(t, names, "id") // readonly should be included
79+
assert.Contains(t, names, "id") // readonly should be included
8080
assert.NotContains(t, names, "internal_field") // internal should be excluded
8181
assert.NotContains(t, names, "-")
8282
assert.NotContains(t, names, "NoTag") // No json tag → excluded
@@ -109,9 +109,9 @@ func TestRewriteToVarShorthand(t *testing.T) {
109109
}{
110110
{"variables.my_cluster.value", "var.my_cluster"},
111111
{"variables.x.value", "var.x"},
112-
{"bundle.name", "bundle.name"}, // not a variables path
113-
{"variables.foo.bar.value", "variables.foo.bar.value"}, // nested - don't rewrite
114-
{"variables.foo.default", "variables.foo.default"}, // not .value suffix
112+
{"bundle.name", "bundle.name"}, // not a variables path
113+
{"variables.foo.bar.value", "variables.foo.bar.value"}, // nested - don't rewrite
114+
{"variables.foo.default", "variables.foo.default"}, // not .value suffix
115115
}
116116
for _, tt := range tests {
117117
assert.Equal(t, tt.want, rewriteToVarShorthand(tt.in), "in=%q", tt.in)

0 commit comments

Comments
 (0)