Skip to content

Commit 9360685

Browse files
Simplify suggest.go: remove hasFieldName, use package-level varPath
Replace FindStructFieldByKeyType + hasFieldName fallback with slices.Contains on the candidates list. Move varPath to a package-level var and remove it from function signatures. Thread varPrefixCorrected through suggestPath's initialHasFix param to eliminate the special case. Co-authored-by: Isaac
1 parent 316b897 commit 9360685

3 files changed

Lines changed: 24 additions & 98 deletions

File tree

bundle/config/mutator/resolve_variable_references.go

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@ var defaultPrefixes = []string{
4646
var (
4747
artifactPath = dyn.MustPathFromString("artifacts")
4848
resourcesPath = dyn.MustPathFromString("resources")
49+
varPath = dyn.NewPath(dyn.Key("var"))
4950
)
5051

5152
type resolveVariableReferences struct {
@@ -149,15 +150,11 @@ func (m *resolveVariableReferences) Apply(ctx context.Context, b *bundle.Bundle)
149150
prefixes[i] = dyn.MustPathFromString(prefix)
150151
}
151152

152-
// The path ${var.foo} is a shorthand for ${variables.foo.value}.
153-
// We rewrite it here to make the resolution logic simpler.
154-
varPath := dyn.NewPath(dyn.Key("var"))
155-
156153
var diags diag.Diagnostics
157154
maxRounds := 1 + m.extraRounds
158155

159156
for round := range maxRounds {
160-
hasUpdates, newDiags := m.resolveOnce(b, prefixes, varPath)
157+
hasUpdates, newDiags := m.resolveOnce(b, prefixes)
161158

162159
diags = diags.Extend(newDiags)
163160

@@ -186,7 +183,7 @@ func (m *resolveVariableReferences) Apply(ctx context.Context, b *bundle.Bundle)
186183
return diags
187184
}
188185

189-
func (m *resolveVariableReferences) resolveOnce(b *bundle.Bundle, prefixes []dyn.Path, varPath dyn.Path) (bool, diag.Diagnostics) {
186+
func (m *resolveVariableReferences) resolveOnce(b *bundle.Bundle, prefixes []dyn.Path) (bool, diag.Diagnostics) {
190187
var diags diag.Diagnostics
191188
hasUpdates := false
192189
err := m.selectivelyMutate(b, func(root dyn.Value) (dyn.Value, error) {
@@ -205,7 +202,7 @@ func (m *resolveVariableReferences) resolveOnce(b *bundle.Bundle, prefixes []dyn
205202
//
206203
normalized, _ := convert.Normalize(b.Config, root, convert.IncludeMissingFields)
207204

208-
suggestFn := m.makeSuggestFn(normalized, prefixes, varPath)
205+
suggestFn := m.makeSuggestFn(normalized)
209206

210207
// If the pattern is nil, we resolve references in the entire configuration.
211208
root, err := dyn.MapByPattern(root, m.pattern, func(p dyn.Path, v dyn.Value) (dyn.Value, error) {

bundle/config/mutator/suggest.go

Lines changed: 9 additions & 71 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package mutator
33
import (
44
"math"
55
"reflect"
6+
"slices"
67
"strings"
78

89
"github.com/databricks/cli/bundle/config"
@@ -125,19 +126,15 @@ func rewriteToVarShorthand(key string) string {
125126
// - Map types: candidates come from the actual runtime dyn.Value keys.
126127
func (m *resolveVariableReferences) makeSuggestFn(
127128
normalized dyn.Value,
128-
prefixes []dyn.Path,
129-
varPath dyn.Path,
130129
) dynvar.SuggestFn {
131130
return func(key string) string {
132-
return m.suggest(key, normalized, prefixes, varPath)
131+
return m.suggest(key, normalized)
133132
}
134133
}
135134

136135
func (m *resolveVariableReferences) suggest(
137136
key string,
138137
normalized dyn.Value,
139-
prefixes []dyn.Path,
140-
varPath dyn.Path,
141138
) string {
142139
// Parse the key into path segments.
143140
path, err := dyn.NewPathFromString(key)
@@ -170,16 +167,7 @@ func (m *resolveVariableReferences) suggest(
170167
segments[i] = c.Key()
171168
}
172169

173-
suggestion := suggestPath(segments, normalized)
174-
175-
// If suggestPath found no deeper fixes but the var prefix itself was
176-
// corrected, verify the rewritten path is valid and use it.
177-
if suggestion == "" && varPrefixCorrected {
178-
if _, err := dyn.GetByPath(normalized, path); err == nil {
179-
suggestion = path.String()
180-
}
181-
}
182-
170+
suggestion := suggestPath(segments, normalized, varPrefixCorrected)
183171
if suggestion == "" {
184172
return ""
185173
}
@@ -193,11 +181,11 @@ func (m *resolveVariableReferences) suggest(
193181
// suggestPath walks the config type tree and dyn value tree segment by segment,
194182
// greedily correcting each wrong segment. Returns the corrected dot-separated
195183
// path if any corrections were made, or "" if no suggestion can be produced.
196-
func suggestPath(segments []string, normalized dyn.Value) string {
184+
func suggestPath(segments []string, normalized dyn.Value, initialHasFix bool) string {
197185
currentType := reflect.TypeOf(config.Root{})
198186
currentDyn := normalized
199187
corrected := make([]string, len(segments))
200-
hasFix := false
188+
hasFix := initialHasFix
201189

202190
for i, seg := range segments {
203191
// Dereference pointer types.
@@ -216,18 +204,9 @@ func suggestPath(segments []string, normalized dyn.Value) string {
216204
case reflect.Struct:
217205
candidates = structFieldNames(currentType)
218206

219-
// Try exact match first.
220-
sf, _, ok := structaccess.FindStructFieldByKeyType(currentType, seg)
221-
if ok {
207+
if slices.Contains(candidates, seg) {
222208
corrected[i] = seg
223-
nextType = sf.Type
224-
} else {
225-
// Exact match failed. Also check readonly fields that
226-
// FindStructFieldByKeyType skips.
227-
if hasFieldName(currentType, seg) {
228-
corrected[i] = seg
229-
nextType = fieldTypeByName(currentType, seg)
230-
}
209+
nextType = fieldTypeByName(currentType, seg)
231210
}
232211

233212
case reflect.Map:
@@ -259,12 +238,7 @@ func suggestPath(segments []string, normalized dyn.Value) string {
259238

260239
// Advance type using the corrected segment.
261240
if currentType.Kind() == reflect.Struct {
262-
sf, _, ok := structaccess.FindStructFieldByKeyType(currentType, best)
263-
if ok {
264-
nextType = sf.Type
265-
} else {
266-
nextType = fieldTypeByName(currentType, best)
267-
}
241+
nextType = fieldTypeByName(currentType, best)
268242
}
269243
// For maps, nextType is already set to the map element type.
270244
}
@@ -289,44 +263,8 @@ func suggestPath(segments []string, normalized dyn.Value) string {
289263
return strings.Join(corrected, ".")
290264
}
291265

292-
// hasFieldName checks if a struct type has a field with the given JSON name,
293-
// including readonly fields (which FindStructFieldByKeyType skips).
294-
func hasFieldName(t reflect.Type, name string) bool {
295-
for t.Kind() == reflect.Pointer {
296-
t = t.Elem()
297-
}
298-
if t.Kind() != reflect.Struct {
299-
return false
300-
}
301-
for i := range t.NumField() {
302-
sf := t.Field(i)
303-
if sf.PkgPath != "" {
304-
continue
305-
}
306-
if sf.Anonymous && sf.Tag.Get("json") == "" {
307-
ft := sf.Type
308-
for ft.Kind() == reflect.Pointer {
309-
ft = ft.Elem()
310-
}
311-
if hasFieldName(ft, name) {
312-
return true
313-
}
314-
continue
315-
}
316-
jsonName := structtag.JSONTag(sf.Tag.Get("json")).Name()
317-
if jsonName == name {
318-
btag := structtag.BundleTag(sf.Tag.Get("bundle"))
319-
if btag.Internal() {
320-
continue
321-
}
322-
return true
323-
}
324-
}
325-
return false
326-
}
327-
328266
// fieldTypeByName returns the reflect.Type of a struct field identified by its
329-
// JSON name, searching embedded structs. Includes readonly fields.
267+
// JSON name, searching embedded structs.
330268
func fieldTypeByName(t reflect.Type, name string) reflect.Type {
331269
for t.Kind() == reflect.Pointer {
332270
t = t.Elem()

bundle/config/mutator/suggest_test.go

Lines changed: 11 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -126,7 +126,7 @@ func TestSuggestPathSingleSegmentFix(t *testing.T) {
126126
}),
127127
})
128128

129-
result := suggestPath([]string{"bundle", "nme"}, normalized)
129+
result := suggestPath([]string{"bundle", "nme"}, normalized, false)
130130
assert.Equal(t, "bundle.name", result)
131131
}
132132

@@ -138,7 +138,7 @@ func TestSuggestPathMultiSegmentFix(t *testing.T) {
138138
}),
139139
})
140140

141-
result := suggestPath([]string{"bundl", "nme"}, normalized)
141+
result := suggestPath([]string{"bundl", "nme"}, normalized, false)
142142
assert.Equal(t, "bundle.name", result)
143143
}
144144

@@ -150,7 +150,7 @@ func TestSuggestPathAllCorrect(t *testing.T) {
150150
})
151151

152152
// All segments match exactly → no suggestion needed.
153-
result := suggestPath([]string{"bundle", "name"}, normalized)
153+
result := suggestPath([]string{"bundle", "name"}, normalized, false)
154154
assert.Equal(t, "", result)
155155
}
156156

@@ -162,7 +162,7 @@ func TestSuggestPathNoMatch(t *testing.T) {
162162
})
163163

164164
// "zzzzz" is too far from any candidate.
165-
result := suggestPath([]string{"bundle", "zzzzz"}, normalized)
165+
result := suggestPath([]string{"bundle", "zzzzz"}, normalized, false)
166166
assert.Equal(t, "", result)
167167
}
168168

@@ -176,7 +176,7 @@ func TestSuggestPathMapKey(t *testing.T) {
176176
}),
177177
})
178178

179-
result := suggestPath([]string{"variables", "my_clster", "value"}, normalized)
179+
result := suggestPath([]string{"variables", "my_clster", "value"}, normalized, false)
180180
assert.Equal(t, "variables.my_cluster.value", result)
181181
}
182182

@@ -194,7 +194,7 @@ func TestSuggestPathResourceField(t *testing.T) {
194194
}),
195195
})
196196

197-
result := suggestPath([]string{"resources", "jobs", "my_job", "nme"}, normalized)
197+
result := suggestPath([]string{"resources", "jobs", "my_job", "nme"}, normalized, false)
198198
assert.Equal(t, "resources.jobs.my_job.name", result)
199199
}
200200

@@ -211,10 +211,7 @@ func TestSuggestVarRewriting(t *testing.T) {
211211
prefixes: defaultPrefixes,
212212
}
213213

214-
prefixes := []dyn.Path{dyn.MustPathFromString("variables")}
215-
varPath := dyn.NewPath(dyn.Key("var"))
216-
217-
suggestion := m.suggest("var.my_clster_id", normalized, prefixes, varPath)
214+
suggestion := m.suggest("var.my_clster_id", normalized)
218215
require.Equal(t, "var.my_cluster_id", suggestion)
219216
}
220217

@@ -231,17 +228,14 @@ func TestSuggestVarPrefixTypo(t *testing.T) {
231228
prefixes: defaultPrefixes,
232229
}
233230

234-
prefixes := []dyn.Path{dyn.MustPathFromString("variables")}
235-
varPath := dyn.NewPath(dyn.Key("var"))
236-
237231
// Typo in var prefix only, variable name is correct.
238-
assert.Equal(t, "var.my_cluster_id", m.suggest("vr.my_cluster_id", normalized, prefixes, varPath))
232+
assert.Equal(t, "var.my_cluster_id", m.suggest("vr.my_cluster_id", normalized))
239233

240234
// Typo in var prefix AND variable name.
241-
assert.Equal(t, "var.my_cluster_id", m.suggest("vr.my_clster_id", normalized, prefixes, varPath))
235+
assert.Equal(t, "var.my_cluster_id", m.suggest("vr.my_clster_id", normalized))
242236

243237
// Var prefix typo but no matching variable.
244-
assert.Equal(t, "", m.suggest("vr.nonexistent", normalized, prefixes, varPath))
238+
assert.Equal(t, "", m.suggest("vr.nonexistent", normalized))
245239
}
246240

247241
func TestSuggestNoSuggestionForCorrectPath(t *testing.T) {
@@ -257,9 +251,6 @@ func TestSuggestNoSuggestionForCorrectPath(t *testing.T) {
257251
prefixes: defaultPrefixes,
258252
}
259253

260-
prefixes := []dyn.Path{dyn.MustPathFromString("variables")}
261-
varPath := dyn.NewPath(dyn.Key("var"))
262-
263-
suggestion := m.suggest("var.my_cluster_id", normalized, prefixes, varPath)
254+
suggestion := m.suggest("var.my_cluster_id", normalized)
264255
assert.Equal(t, "", suggestion)
265256
}

0 commit comments

Comments
 (0)