From ae3408a096b667203df9b01684bdb9b1fa99fa94 Mon Sep 17 00:00:00 2001 From: Eric Rakestraw Date: Sun, 25 Oct 2026 21:22:01 +0000 Subject: [PATCH] Distinguish finite slice views from cycles --- docs/roadmap/implementation.md | 3 + internal/engine/slice_view_cycle_test.go | 79 ++++++++++++++++++++++++ internal/engine/value_operation.go | 4 ++ 3 files changed, 86 insertions(+) create mode 100644 internal/engine/slice_view_cycle_test.go diff --git a/docs/roadmap/implementation.md b/docs/roadmap/implementation.md index 7bd91c3..c5df6fb 100644 --- a/docs/roadmap/implementation.md +++ b/docs/roadmap/implementation.md @@ -139,6 +139,9 @@ Focused check: `go test ./internal/engine -run 'Dynamic|Projection|Clone|Resolut Finding: **A03**. Owners: `internal/engine/value_operation.go`, `value.go`, `clone.go`, and tests. +Status: **complete** (A03). Prior stage commits: `2736ddf`, `deb9aa5`. +Validation passed: `GOTOOLCHAIN=local GOPROXY=off GOTELEMETRY=off go test ./internal/engine -run 'Mapping|Value|Projection|Clone|Override|Cycle|FiniteSlice|SliceCycles' -count=1` and `GOTOOLCHAIN=local GOPROXY=off GOTELEMETRY=off go test ./internal/engine -count=1`. + Add visible slice length to `valueVisit` identity in `visitFor`. Keep type, kind, and starting pointer, existing empty/nil handling, and active-path removal on unwinding. Capacity is not part of the visible traversed graph. Maps and pointers diff --git a/internal/engine/slice_view_cycle_test.go b/internal/engine/slice_view_cycle_test.go new file mode 100644 index 0000000..c6ce364 --- /dev/null +++ b/internal/engine/slice_view_cycle_test.go @@ -0,0 +1,79 @@ +package engine + +import ( + "reflect" + "testing" +) + +type sliceViewConfig struct { + Value []any `yaml:"value"` +} + +func finiteSliceView() []any { + shared := []any{"leaf", nil} + shared[1] = shared[:1] + return shared +} + +func TestFiniteSliceViewsAreAcceptedAcrossValueBoundaries(t *testing.T) { + want := []any{"leaf", []any{"leaf"}} + + mappingSource := Mapping("mapping.yml", map[string]any{"value": finiteSliceView()}) + mapping, err := testLoad[sliceViewConfig](mappingSource) + if err != nil || !reflect.DeepEqual(mapping.Config.Value, want) { + t.Fatalf("mapping value = %#v, error = %v", mapping, err) + } + + defaults, err := testLoad[sliceViewConfig](NoSource(), Options[sliceViewConfig]{Definition: Definition[sliceViewConfig]{ + Defaults: func() sliceViewConfig { return sliceViewConfig{Value: finiteSliceView()} }, + }}) + if err != nil || !reflect.DeepEqual(defaults.Config.Value, want) { + t.Fatalf("typed default value = %#v, error = %v", defaults, err) + } + + override, err := testLoad[sliceViewConfig](NoSource(), Options[sliceViewConfig]{Overrides: []Override{{ + Path: Keys("value"), Value: finiteSliceView(), Flag: "--value", + }}}) + if err != nil || !reflect.DeepEqual(override.Config.Value, want) { + t.Fatalf("CLI override value = %#v, error = %v", override, err) + } + + original := sliceViewConfig{Value: finiteSliceView()} + schema := mustSchemaFor[sliceViewConfig](t) + projected, issue := projectConfiguration(schema, reflect.ValueOf(original), defaultProjectionLimits()) + if issue != nil { + t.Fatalf("finite view projection issue = %#v", issue) + } + cloned, issue := cloneConfiguration(schema, reflect.ValueOf(original), defaultProjectionLimits()) + if issue != nil { + t.Fatalf("finite view clone issue = %#v", issue) + } + if !reflect.DeepEqual(cloned.Interface(), original) || !reflect.DeepEqual(projected, mappingSource.mapping) { + t.Fatalf("equivalent finite values differ: clone=%#v projected=%#v mapping=%#v", cloned.Interface(), projected, mappingSource.mapping) + } + cloned.FieldByName("Value").Index(1).Interface().([]any)[0] = "changed" + if original.Value[0] != "leaf" || original.Value[1].([]any)[0] != "leaf" { + t.Fatal("mutating a cloned descendant changed the original value") + } +} + +func TestSliceCyclesRemainRejected(t *testing.T) { + schema := mustSchemaFor[sliceViewConfig](t) + self := make([]any, 1) + self[0] = self + first := make([]any, 1) + second := make([]any, 1) + first[0], second[0] = second, first + + for name, value := range map[string][]any{"self": self, "mutual": first} { + t.Run(name, func(t *testing.T) { + configuration := sliceViewConfig{Value: value} + if _, issue := projectConfiguration(schema, reflect.ValueOf(configuration), defaultProjectionLimits()); issue == nil || issue.code != CodeSchemaCycle { + t.Fatalf("projection cycle issue = %#v", issue) + } + if _, issue := cloneConfiguration(schema, reflect.ValueOf(configuration), defaultProjectionLimits()); issue == nil || issue.code != CodeSchemaCycle { + t.Fatalf("clone cycle issue = %#v", issue) + } + }) + } +} diff --git a/internal/engine/value_operation.go b/internal/engine/value_operation.go index acdd437..77cfcf1 100644 --- a/internal/engine/value_operation.go +++ b/internal/engine/value_operation.go @@ -21,6 +21,7 @@ type valueVisit struct { typeOf reflect.Type kind reflect.Kind ptr uintptr + length int } // valueOperation owns one traversal's cycle and budget state and reuses the @@ -107,6 +108,9 @@ func visitFor(value reflect.Value) (valueVisit, bool) { return valueVisit{}, false } visit.ptr = value.Pointer() + if value.Kind() == reflect.Slice { + visit.length = value.Len() + } case reflect.Map: if value.Len() == 0 { return valueVisit{}, false