diff --git a/docs/roadmap/implementation.md b/docs/roadmap/implementation.md index d265b7f..6ee5946 100644 --- a/docs/roadmap/implementation.md +++ b/docs/roadmap/implementation.md @@ -179,7 +179,7 @@ authorized. No feature code or current behavior contract changed. ## Stage 2 — Add private destination normalization and wrapping -Status: planned. Scope: pure private mechanics with bounded unit tests. +Status: complete. Scope: pure private mechanics with bounded unit tests. 1. Add a small engine helper file for normalized mapping-only destinations and constructing placed document bodies. Follow decisions 2–7; keep helpers @@ -195,8 +195,11 @@ Status: planned. Scope: pure private mechanics with bounded unit tests. unchanged body coordinates, and bounded wrapper checks. Do not count helper invocations or fix an allocation strategy in tests. -Validate: focused destination/path tests and the everyday check. Done: the -mechanism is usable privately; no loader accepts a new public field yet. +Validated with `go test ./internal/engine -run 'Destination|PlaceDocument'` +and `./scripts/check.sh` (go1.26.8 linux/amd64); both passed. The private +helpers normalize mapping-only destinations, preserve authored nodes and source +coordinates, and check per-document depth/node bounds before allocating wrappers. +No loader accepts a new public field yet. ## Stage 3 — Integrate private non-import input placement diff --git a/internal/engine/destination.go b/internal/engine/destination.go new file mode 100644 index 0000000..459bf44 --- /dev/null +++ b/internal/engine/destination.go @@ -0,0 +1,108 @@ +package engine + +import ( + "errors" + + "gitea.maximumdirect.net/eric/ConfigKit/internal/document" +) + +var ( + errInvalidDestination = errors.New("configkit: destination must contain only non-reserved mapping keys") + errDestinationLimit = errors.New("configkit: destination exceeds document limits") +) + +func normalizeDestination(path Path) (Path, error) { + if !path.valid() { + return Path{}, errInvalidDestination + } + + segments := make([]pathSegment, 0, len(path.segments)) + for _, segment := range path.segments { + if segment.kind != pathSegmentKey && segment.kind != pathSegmentParsed { + return Path{}, errInvalidDestination + } + if segment.key == "configkit" || segment.key == "extends" { + return Path{}, errInvalidDestination + } + segments = append(segments, pathSegment{kind: pathSegmentKey, key: segment.key}) + } + return Path{segments: segments}, nil +} + +// placeDocument checks all per-document bounds before allocating destination +// wrappers. Aggregate accounting remains the caller's responsibility. +func placeDocument(body *document.Node, destination Path, sourceRoot document.Location, maxDepth, maxNodes int) (Path, *document.Node, error) { + normalized, err := normalizeDestination(destination) + if err != nil { + return Path{}, nil, err + } + if body == nil { + return normalized, nil, nil + } + if !destinationWithinLimits(body, normalized, maxDepth, maxNodes) { + return normalized, nil, errDestinationLimit + } + + placed := body + for index := len(normalized.segments) - 1; index >= 0; index-- { + key := normalized.segments[index].key + placed = &document.Node{ + Kind: document.Mapping, + Location: sourceRoot, + Mapping: []document.Entry{{ + Key: key, + KeyLocation: sourceRoot, + ValueLocation: sourceRoot, + Value: placed, + }}, + } + } + return normalized, placed, nil +} + +// destinationWithinLimits expects a normalized mapping-key destination. +func destinationWithinLimits(body *document.Node, destination Path, maxDepth, maxNodes int) bool { + if body == nil { + return true + } + if maxDepth < 0 || maxNodes < 0 || !destination.valid() { + return false + } + segments := len(destination.segments) + if segments > maxDepth || segments > maxNodes/2 { + return false + } + for _, segment := range destination.segments { + if segment.kind != pathSegmentKey { + return false + } + } + if documentDepth(body) > maxDepth-segments { + return false + } + return countInputNodes(body) <= maxNodes-2*segments +} + +func documentDepth(node *document.Node) int { + if node == nil { + return -1 + } + depth := 0 + var visit func(*document.Node, int) + visit = func(current *document.Node, currentDepth int) { + if current == nil { + return + } + if currentDepth > depth { + depth = currentDepth + } + for _, entry := range current.Mapping { + visit(entry.Value, currentDepth+1) + } + for _, child := range current.Sequence { + visit(child, currentDepth+1) + } + } + visit(node, 0) + return depth +} diff --git a/internal/engine/destination_test.go b/internal/engine/destination_test.go new file mode 100644 index 0000000..c67e056 --- /dev/null +++ b/internal/engine/destination_test.go @@ -0,0 +1,178 @@ +package engine + +import ( + "errors" + "testing" + + "gitea.maximumdirect.net/eric/ConfigKit/internal/document" +) + +func TestNormalizeDestinationUsesLiteralMappingKeys(t *testing.T) { + t.Parallel() + + parsed, err := ParsePath("/a~1b/0/~0") + if err != nil { + t.Fatal(err) + } + normalized, err := normalizeDestination(parsed) + if err != nil { + t.Fatal(err) + } + if got, want := normalized.String(), "/a~1b/0/~0"; got != want { + t.Fatalf("normalized path = %q, want %q", got, want) + } + for index, want := range []string{"a/b", "0", "~"} { + segment := normalized.segments[index] + if segment.kind != pathSegmentKey || segment.key != want { + t.Fatalf("segment %d = %#v, want key %q", index, segment, want) + } + } + + original := Keys("owned", "path") + owned, err := normalizeDestination(original) + if err != nil { + t.Fatal(err) + } + owned.segments[0].key = "changed" + if got := original.String(); got != "/owned/path" { + t.Fatalf("normalizing shared path storage mutated its input: %q", got) + } +} + +func TestNormalizeDestinationRejectsIndicesAndReservedKeys(t *testing.T) { + t.Parallel() + + for name, path := range map[string]Path{ + "explicit index": Keys("items").Index(0), + "negative index": Keys("items").Index(-1), + "invalid path": Path{invalid: true}, + "configkit": Keys("configkit"), + "extends": Keys("extends"), + "nested configkit": Keys("outer", "configkit"), + "nested extends": Keys("outer", "extends"), + } { + t.Run(name, func(t *testing.T) { + t.Parallel() + if _, err := normalizeDestination(path); !errors.Is(err, errInvalidDestination) { + t.Fatalf("normalizeDestination(%q) error = %v, want invalid destination", path.String(), err) + } + }) + } + + for _, key := range []string{"myextends", "configkit2", "", ".", "..", "../x", "a.b", "a/b", "~"} { + t.Run("ordinary key "+key, func(t *testing.T) { + t.Parallel() + if _, err := normalizeDestination(Keys(key)); err != nil { + t.Fatalf("normalizeDestination(%q): %v", key, err) + } + }) + } +} + +func TestPlaceDocumentWrapsEmptyMappingAndPreservesSourceCoordinates(t *testing.T) { + t.Parallel() + + rootLocation := document.Location{SourceID: "source-a", Line: 1, Column: 1} + body := &document.Node{ + Kind: document.Mapping, + Location: document.Location{SourceID: "source-a", Line: 4, Column: 1}, + } + path, placed, err := placeDocument(body, Keys("a", "b"), rootLocation, 2, 5) + if err != nil { + t.Fatal(err) + } + if got, want := path.String(), "/a/b"; got != want { + t.Fatalf("normalized destination = %q, want %q", got, want) + } + if placed == body || len(placed.Mapping) != 1 || placed.Mapping[0].Key != "a" { + t.Fatalf("outer destination mapping was not created: %#v", placed) + } + inner := placed.Mapping[0].Value + if inner.Kind != document.Mapping || len(inner.Mapping) != 1 || inner.Mapping[0].Key != "b" { + t.Fatalf("inner destination mapping was not created: %#v", inner) + } + if inner.Mapping[0].Value != body || len(body.Mapping) != 0 { + t.Fatal("placement mutated or copied the authored body") + } + if placed.Location != rootLocation || inner.Location != rootLocation || placed.Mapping[0].KeyLocation != rootLocation || placed.Mapping[0].ValueLocation != rootLocation { + t.Fatal("introduced structure did not use the source-root coordinate fallback") + } + if body.Location != (document.Location{SourceID: "source-a", Line: 4, Column: 1}) { + t.Fatalf("authored body location changed: %#v", body.Location) + } + if got, want := countInputNodes(placed), countInputNodes(body)+4; got != want { + t.Fatalf("placed node count = %d, want body count plus four synthetic nodes (%d)", got, want) + } +} + +func TestPlaceDocumentPreservesAuthoredEntryCoordinates(t *testing.T) { + t.Parallel() + + rootLocation := document.Location{SourceID: "source-b", Line: 1, Column: 1} + bodyLocation := document.Location{SourceID: "source-b", Line: 3, Column: 1} + keyLocation := document.Location{SourceID: "source-b", Line: 3, Column: 3} + valueLocation := document.Location{SourceID: "source-b", Line: 4, Column: 5} + value := &document.Node{Kind: document.String, Location: valueLocation, Text: "value"} + body := &document.Node{ + Kind: document.Mapping, + Location: bodyLocation, + Mapping: []document.Entry{{ + Key: "leaf", + KeyLocation: keyLocation, + ValueLocation: valueLocation, + Value: value, + }}, + } + _, placed, err := placeDocument(body, Keys("mounted"), rootLocation, 2, 5) + if err != nil { + t.Fatal(err) + } + if placed.Mapping[0].KeyLocation != rootLocation || placed.Mapping[0].ValueLocation != rootLocation { + t.Fatal("introduced destination entry did not use the source-root fallback") + } + if placed.Mapping[0].Value != body || body.Location != bodyLocation { + t.Fatal("placement changed the authored body node or location") + } + entry := body.Mapping[0] + if entry.KeyLocation != keyLocation || entry.ValueLocation != valueLocation || entry.Value != value || value.Location != valueLocation { + t.Fatal("placement changed authored entry or value coordinates") + } +} + +func TestPlaceDocumentRootAndBounds(t *testing.T) { + t.Parallel() + + body := &document.Node{ + Kind: document.Mapping, + Mapping: []document.Entry{{ + Key: "leaf", + Value: &document.Node{Kind: document.String, Text: "value"}, + }}, + } + path, placed, err := placeDocument(body, Path{}, body.Location, 1, 3) + if err != nil { + t.Fatal(err) + } + if path.String() != "" || placed != body { + t.Fatal("root placement must preserve the existing body") + } + + destination := Keys("a", "b") + if !destinationWithinLimits(body, destination, 3, 7) { + t.Fatal("exact depth and node limits should admit the placed document") + } + if destinationWithinLimits(body, destination, 2, 7) { + t.Fatal("placement exceeding the depth limit was accepted") + } + if destinationWithinLimits(body, destination, 3, 6) { + t.Fatal("placement exceeding the node limit was accepted") + } + _, placed, err = placeDocument(body, destination, body.Location, 3, 6) + if !errors.Is(err, errDestinationLimit) || placed != nil { + t.Fatalf("bounded placement returned node %p and error %v", placed, err) + } + _, absent, err := placeDocument(nil, destination, document.Location{}, 0, 0) + if err != nil || absent != nil { + t.Fatalf("absent body should not consume placement limits: node %p, error %v", absent, err) + } +}