diff --git a/builtins.go b/builtins.go index 9cf0f8be..cebabe31 100644 --- a/builtins.go +++ b/builtins.go @@ -1531,6 +1531,7 @@ func builtinUglyObjectFlatMerge(i *interpreter, x value) (value, error) { return nil, err } newFields := make(simpleObjectFieldMap) + var newOrder []string for _, elem := range objarr.elements { obj, err := i.evaluateObject(elem) if err != nil { @@ -1545,7 +1546,8 @@ func builtinUglyObjectFlatMerge(i *interpreter, x value) (value, error) { } // there is only one field, really - for fieldName, fieldVal := range simpleObj.fields { + for _, fieldName := range simpleObj.fieldOrder { + fieldVal := simpleObj.fields[fieldName] if _, alreadyExists := newFields[fieldName]; alreadyExists { return nil, i.Error(duplicateFieldNameErrMsg(fieldName)) } @@ -1557,12 +1559,14 @@ func builtinUglyObjectFlatMerge(i *interpreter, x value) (value, error) { bindings: simpleObj.upValues, }, } + newOrder = append(newOrder, fieldName) } } return makeValueSimpleObject( nil, newFields, + newOrder, []unboundField{}, // No asserts allowed nil, ), nil diff --git a/cmd/jsonnet/cmd.go b/cmd/jsonnet/cmd.go index ada9adf6..30429821 100644 --- a/cmd/jsonnet/cmd.go +++ b/cmd/jsonnet/cmd.go @@ -258,6 +258,8 @@ func processArgs(givenArgs []string, config *config, vm *jsonnet.VM) (processArg config.evalStream = true } else if arg == "-S" || arg == "--string" { vm.StringOutput = true + } else if arg == "--preserve-field-order" { + vm.PreserveFieldOrder = true } else if arg == "--no-trailing-newline" { vm.OutputNewline = false } else if len(arg) > 1 && arg[0] == '-' { diff --git a/interpreter.go b/interpreter.go index 26df98a2..13bbeccb 100644 --- a/interpreter.go +++ b/interpreter.go @@ -269,6 +269,9 @@ type interpreter struct { // Native functions nativeFuncs map[string]*NativeFunction + // When true, object fields are output in declaration order rather than sorted + preserveFieldOrder bool + // A part of std object common to all files baseStd *valueObject @@ -418,6 +421,7 @@ func (i *interpreter) rawevaluate(a ast.Node, tc tailCallStatus) (value, error) case *ast.DesugaredObject: // Evaluate all the field names. Check for null, dups, etc. fields := make(simpleObjectFieldMap, len(node.Fields)) + var fieldOrder []string for _, field := range node.Fields { fieldNameValue, err := i.evaluate(field.Name, nonTailCall) if err != nil { @@ -442,6 +446,7 @@ func (i *interpreter) rawevaluate(a ast.Node, tc tailCallStatus) (value, error) f = &plusSuperUnboundField{f} } fields[fieldName] = simpleObjectField{f, field.Hide} + fieldOrder = append(fieldOrder, fieldName) } var asserts []unboundField for _, assert := range node.Asserts { @@ -452,7 +457,7 @@ func (i *interpreter) rawevaluate(a ast.Node, tc tailCallStatus) (value, error) locals = append(locals, objectLocal{name: local.Variable, node: local.Body}) } upValues := i.stack.capture(node.FreeVariables()) - return makeValueSimpleObject(upValues, fields, asserts, locals), nil + return makeValueSimpleObject(upValues, fields, fieldOrder, asserts, locals), nil case *ast.Error: msgVal, err := i.evaluate(node.Expr, nonTailCall) @@ -680,6 +685,12 @@ func unparseNumber(v float64) string { return fmt.Sprintf("%.17g", v) } +// jsonOrderedObject is an intermediate representation of a JSON object that preserves field order. +type jsonOrderedObject struct { + keys []string + fields map[string]interface{} +} + // manifestJSON converts to standard JSON representation as in "encoding/json" package func (i *interpreter) manifestJSON(v value) (interface{}, error) { // TODO(sbarzowski) Add nice stack traces indicating the part of the code which @@ -737,8 +748,7 @@ func (i *interpreter) manifestJSON(v value) (interface{}, error) { return result, nil case *valueObject: - fieldNames := objectFields(v, withoutHidden) - sort.Strings(fieldNames) + fieldNames := objectFieldsOrdered(v, withoutHidden) msg := ast.MakeLocationRangeMessage("Checking object assertions") i.stack.setCurrentTrace(traceElement{ @@ -751,7 +761,10 @@ func (i *interpreter) manifestJSON(v value) (interface{}, error) { } i.stack.clearCurrentTrace() - result := make(map[string]interface{}, len(fieldNames)) + result := jsonOrderedObject{ + keys: fieldNames, + fields: make(map[string]interface{}, len(fieldNames)), + } for _, fieldName := range fieldNames { msg := ast.MakeLocationRangeMessage(fmt.Sprintf("Field %#v", fieldName)) @@ -769,7 +782,7 @@ func (i *interpreter) manifestJSON(v value) (interface{}, error) { i.stack.clearCurrentTrace() return nil, err } - result[fieldName] = field + result.fields[fieldName] = field i.stack.clearCurrentTrace() } @@ -784,7 +797,7 @@ func (i *interpreter) manifestJSON(v value) (interface{}, error) { } } -func serializeJSON(v interface{}, multiline bool, indent string, buf *bytes.Buffer) { +func serializeJSON(v interface{}, multiline bool, indent string, buf *bytes.Buffer, preserveFieldOrder bool) { switch v := v.(type) { case nil: buf.WriteString("null") @@ -805,7 +818,7 @@ func serializeJSON(v interface{}, multiline bool, indent string, buf *bytes.Buff for _, elem := range v { buf.WriteString(prefix) buf.WriteString(indent2) - serializeJSON(elem, multiline, indent2, buf) + serializeJSON(elem, multiline, indent2, buf, preserveFieldOrder) if multiline { prefix = ",\n" } else { @@ -829,14 +842,15 @@ func serializeJSON(v interface{}, multiline bool, indent string, buf *bytes.Buff case float64: buf.WriteString(unparseNumber(v)) - case map[string]interface{}: - fieldNames := make([]string, 0, len(v)) - for name := range v { - fieldNames = append(fieldNames, name) + case jsonOrderedObject: + keys := v.keys + if !preserveFieldOrder { + sorted := make([]string, len(keys)) + copy(sorted, keys) + sort.Strings(sorted) + keys = sorted } - sort.Strings(fieldNames) - - if len(fieldNames) == 0 { + if len(keys) == 0 { buf.WriteString("{ }") } else { var prefix string @@ -848,8 +862,8 @@ func serializeJSON(v interface{}, multiline bool, indent string, buf *bytes.Buff prefix = "{" indent2 = indent } - for _, fieldName := range fieldNames { - fieldVal := v[fieldName] + for _, fieldName := range keys { + fieldVal := v.fields[fieldName] buf.WriteString(prefix) buf.WriteString(indent2) @@ -857,7 +871,7 @@ func serializeJSON(v interface{}, multiline bool, indent string, buf *bytes.Buff buf.WriteString(unparseString(fieldName)) buf.WriteString(": ") - serializeJSON(fieldVal, multiline, indent2, buf) + serializeJSON(fieldVal, multiline, indent2, buf, preserveFieldOrder) if multiline { prefix = ",\n" @@ -887,7 +901,7 @@ func (i *interpreter) manifestAndSerializeJSON( if err != nil { return err } - serializeJSON(manifested, multiline, indent, buf) + serializeJSON(manifested, multiline, indent, buf, i.preserveFieldOrder) return nil } @@ -909,8 +923,9 @@ func (i *interpreter) manifestAndSerializeMulti(v value, stringOutputMode bool, return r, err } switch json := json.(type) { - case map[string]interface{}: - for filename, fileJSON := range json { + case jsonOrderedObject: + for _, filename := range json.keys { + fileJSON := json.fields[filename] var buf bytes.Buffer if stringOutputMode { switch val := fileJSON.(type) { @@ -922,7 +937,7 @@ func (i *interpreter) manifestAndSerializeMulti(v value, stringOutputMode bool, return r, makeRuntimeError(msg, i.getCurrentStackTrace()) } } else { - serializeJSON(fileJSON, true, "", &buf) + serializeJSON(fileJSON, true, "", &buf, i.preserveFieldOrder) } if outputNewline { buf.WriteString("\n") @@ -948,7 +963,7 @@ func (i *interpreter) manifestAndSerializeYAMLStream(v value) (r []string, err e case []interface{}: for _, doc := range json { var buf bytes.Buffer - serializeJSON(doc, true, "", &buf) + serializeJSON(doc, true, "", &buf, i.preserveFieldOrder) buf.WriteString("\n") r = append(r, buf.String()) } @@ -961,6 +976,27 @@ func (i *interpreter) manifestAndSerializeYAMLStream(v value) (r []string, err e return } +// flattenJSONForNative converts jsonOrderedObject to map[string]interface{} recursively, +// for passing to native functions which expect standard Go JSON types. +func flattenJSONForNative(v interface{}) interface{} { + switch v := v.(type) { + case jsonOrderedObject: + m := make(map[string]interface{}, len(v.keys)) + for k, val := range v.fields { + m[k] = flattenJSONForNative(val) + } + return m + case []interface{}: + result := make([]interface{}, len(v)) + for idx, elem := range v { + result[idx] = flattenJSONForNative(elem) + } + return result + default: + return v + } +} + func jsonToValue(i *interpreter, v interface{}) (value, error) { switch v := v.(type) { case nil: @@ -1269,19 +1305,23 @@ func prepareExtVars(i *interpreter, ext vmExtMap, kind string) map[string]*cache func buildObject(hide ast.ObjectFieldHide, fields map[string]value) *valueObject { fieldMap := simpleObjectFieldMap{} + fieldOrder := make([]string, 0, len(fields)) for name, v := range fields { fieldMap[name] = simpleObjectField{&readyValue{v}, hide} + fieldOrder = append(fieldOrder, name) } - return makeValueSimpleObject(bindingFrame{}, fieldMap, nil, nil) + sort.Strings(fieldOrder) + return makeValueSimpleObject(bindingFrame{}, fieldMap, fieldOrder, nil, nil) } -func buildInterpreter(ext vmExtMap, nativeFuncs map[string]*NativeFunction, maxStack int, ic *importCache, traceOut io.Writer, evalHook EvalHook) (*interpreter, error) { +func buildInterpreter(ext vmExtMap, nativeFuncs map[string]*NativeFunction, maxStack int, ic *importCache, traceOut io.Writer, evalHook EvalHook, preserveFieldOrder bool) (*interpreter, error) { i := interpreter{ - stack: makeCallStack(maxStack), - importCache: ic, - traceOut: traceOut, - nativeFuncs: nativeFuncs, - evalHook: evalHook, + stack: makeCallStack(maxStack), + importCache: ic, + traceOut: traceOut, + nativeFuncs: nativeFuncs, + evalHook: evalHook, + preserveFieldOrder: preserveFieldOrder, } stdObj, err := buildStdObject(&i) diff --git a/jsonnet_test.go b/jsonnet_test.go index 4a864376..c07d748b 100644 --- a/jsonnet_test.go +++ b/jsonnet_test.go @@ -242,7 +242,7 @@ func TestExtReset(t *testing.T) { t.Fatalf("unexpected error %v", err) } vm.ExtReset() - _, err = vm.EvaluateAnonymousSnippet("test.jsonnet", `{ str: std.extVat('fooString'), code: std.extVar('fooCode') }`) + _, err = vm.EvaluateAnonymousSnippet("test.jsonnet", `{ str: std.extVar('fooString'), code: std.extVar('fooCode') }`) if err == nil { t.Fatalf("expected error, got nil") } @@ -251,6 +251,31 @@ func TestExtReset(t *testing.T) { } } +func TestPreserveFieldOrder(t *testing.T) { + input := `{ z: 1, a: 2, m: 3 }` + + vm := MakeVM() + sorted, err := vm.EvaluateAnonymousSnippet("test.jsonnet", input) + if err != nil { + t.Fatalf("unexpected error %v", err) + } + if strings.Contains(sorted, `"z": 1`) && strings.Index(sorted, `"a"`) < strings.Index(sorted, `"z"`) { + // "a" comes before "z" — correct alphabetical order + } else if strings.Index(sorted, `"z"`) < strings.Index(sorted, `"a"`) { + t.Errorf("expected alphabetical order by default, got: %s", sorted) + } + + vm2 := MakeVM() + vm2.PreserveFieldOrder = true + ordered, err := vm2.EvaluateAnonymousSnippet("test.jsonnet", input) + if err != nil { + t.Fatalf("unexpected error %v", err) + } + if strings.Index(ordered, `"z"`) > strings.Index(ordered, `"a"`) { + t.Errorf("expected declaration order with PreserveFieldOrder=true, got: %s", ordered) + } +} + func TestTLAReset(t *testing.T) { vm := MakeVM() vm.TLAVar("fooString", "bar") diff --git a/main_test.go b/main_test.go index b14f0726..a97ad96e 100644 --- a/main_test.go +++ b/main_test.go @@ -104,13 +104,14 @@ var nativePanic = &NativeFunction{ } type jsonnetInput struct { - name string - input []byte - eKind evalKind - stringOutputMode bool - noNewline bool // not nice to have a negative flag, but it gives the more relevant default - extVars map[string]string - extCode map[string]string + name string + input []byte + eKind evalKind + stringOutputMode bool + noNewline bool // not nice to have a negative flag, but it gives the more relevant default + preserveFieldOrder bool + extVars map[string]string + extCode map[string]string } type jsonnetResult struct { @@ -136,6 +137,7 @@ func runInternalJsonnet(i jsonnetInput) jsonnetResult { vm.StringOutput = i.stringOutputMode vm.OutputNewline = !i.noNewline + vm.PreserveFieldOrder = i.preserveFieldOrder for name, value := range i.extVars { vm.ExtVar(name, value) } @@ -337,8 +339,9 @@ func runTest(t *testing.T, test *mainTest) { name: test.name, input: input, eKind: eKind, - stringOutputMode: strings.HasSuffix(test.golden, "_string_output.golden"), - noNewline: strings.Contains(test.golden, "_no_newline"), + stringOutputMode: strings.HasSuffix(test.golden, "_string_output.golden"), + noNewline: strings.Contains(test.golden, "_no_newline"), + preserveFieldOrder: strings.Contains(test.golden, "_preserve_field_order"), extVars: test.meta.extVars, extCode: test.meta.extCode, }) diff --git a/testdata/field_order.golden b/testdata/field_order.golden new file mode 100644 index 00000000..b74521ef --- /dev/null +++ b/testdata/field_order.golden @@ -0,0 +1,23 @@ +{ + "extended": { + "a": 2, + "b": 4, + "m": 3, + "z": 1 + }, + "nested": { + "a": { + "c": 40, + "q": 30 + }, + "z": { + "b": 20, + "y": 10 + } + }, + "plain": { + "a": 2, + "m": 3, + "z": 1 + } +} diff --git a/testdata/field_order.jsonnet b/testdata/field_order.jsonnet new file mode 100644 index 00000000..71cdd262 --- /dev/null +++ b/testdata/field_order.jsonnet @@ -0,0 +1,10 @@ +// Fields are declared in z, a, m order; output should reflect that +// when --preserve-field-order is set, or be sorted alphabetically otherwise. +local obj = { z: 1, a: 2, m: 3 }; +local nested = { z: { y: 10, b: 20 }, a: { q: 30, c: 40 } }; +local extended = { z: 1, a: 2 } + { m: 3, b: 4 }; +{ + plain: obj, + nested: nested, + extended: extended, +} diff --git a/testdata/field_order.linter.golden b/testdata/field_order.linter.golden new file mode 100644 index 00000000..e69de29b diff --git a/testdata/field_order_preserve_field_order.golden b/testdata/field_order_preserve_field_order.golden new file mode 100644 index 00000000..5d370fd3 --- /dev/null +++ b/testdata/field_order_preserve_field_order.golden @@ -0,0 +1,23 @@ +{ + "plain": { + "z": 1, + "a": 2, + "m": 3 + }, + "nested": { + "z": { + "y": 10, + "b": 20 + }, + "a": { + "q": 30, + "c": 40 + } + }, + "extended": { + "z": 1, + "a": 2, + "m": 3, + "b": 4 + } +} diff --git a/thunks.go b/thunks.go index e98c487c..f513a4ff 100644 --- a/thunks.go +++ b/thunks.go @@ -273,7 +273,7 @@ func (native *NativeFunction) evalCall(arguments callArguments, i *interpreter) if err != nil { return nil, err } - nativeArgs = append(nativeArgs, json) + nativeArgs = append(nativeArgs, flattenJSONForNative(json)) } call := func() (resultJSON interface{}, err error) { defer func() { diff --git a/value.go b/value.go index d5a65716..9383e9aa 100644 --- a/value.go +++ b/value.go @@ -539,10 +539,11 @@ type objectLocal struct { // Let a = {x: 42} and b = {y: self.x}. Evaluating b.y is an error, // but (a+b).y evaluates to 42. type simpleObject struct { - upValues bindingFrame - fields simpleObjectFieldMap - asserts []unboundField - locals []objectLocal + upValues bindingFrame + fields simpleObjectFieldMap + fieldOrder []string // tracks definition order of fields + asserts []unboundField + locals []objectLocal } func checkAssertionsHelper(i *interpreter, obj *valueObject, curr uncachedObject, superDepth int) error { @@ -592,14 +593,15 @@ func (*simpleObject) inheritanceSize() int { return 1 } -func makeValueSimpleObject(b bindingFrame, fields simpleObjectFieldMap, asserts []unboundField, locals []objectLocal) *valueObject { +func makeValueSimpleObject(b bindingFrame, fields simpleObjectFieldMap, fieldOrder []string, asserts []unboundField, locals []objectLocal) *valueObject { return &valueObject{ cache: make(map[objectCacheKey]value), uncached: &simpleObject{ - upValues: b, - fields: fields, - asserts: asserts, - locals: locals, + upValues: b, + fields: fields, + fieldOrder: fieldOrder, + asserts: asserts, + locals: locals, }, } } @@ -659,6 +661,7 @@ func makeValueExtendedObject(left, right *valueObject) *valueObject { type restrictedObject struct { obj uncachedObject retainedFields fieldHideMap + retainedOrder []string // tracks field order after restriction } func (o *restrictedObject) inheritanceSize() int { @@ -666,11 +669,20 @@ func (o *restrictedObject) inheritanceSize() int { } func makeValueRestrictedObject(obj *valueObject) *valueObject { + retained := objectFieldsVisibility(obj) + fullOrder := uncachedObjectFieldsOrder(obj.uncached) + retainedOrder := make([]string, 0, len(fullOrder)) + for _, f := range fullOrder { + if _, ok := retained[f]; ok { + retainedOrder = append(retainedOrder, f) + } + } return &valueObject{ cache: make(map[objectCacheKey]value), uncached: &restrictedObject{ obj: obj.uncached, - retainedFields: objectFieldsVisibility(obj), + retainedFields: retained, + retainedOrder: retainedOrder, }, } } @@ -769,6 +781,50 @@ func objectHasField(sb selfBinding, fieldName string) bool { type fieldHideMap map[string]ast.ObjectFieldHide +// uncachedObjectFieldsOrder returns field names in definition order, without filtering by visibility. +// For extended objects, left fields come first, then new fields from the right. +func uncachedObjectFieldsOrder(obj uncachedObject) []string { + switch obj := obj.(type) { + case *extendedObject: + leftOrder := uncachedObjectFieldsOrder(obj.left) + rightOrder := uncachedObjectFieldsOrder(obj.right) + result := make([]string, 0, len(leftOrder)+len(rightOrder)) + seen := make(map[string]bool, len(leftOrder)) + for _, f := range leftOrder { + result = append(result, f) + seen[f] = true + } + for _, f := range rightOrder { + if !seen[f] { + result = append(result, f) + } + } + return result + case *restrictedObject: + return obj.retainedOrder + case *simpleObject: + return obj.fieldOrder + } + return nil +} + +// objectFieldsOrdered returns visible field names in definition order. +func objectFieldsOrdered(obj *valueObject, h hidden) []string { + visibility := objectFieldsVisibility(obj) + allKeys := uncachedObjectFieldsOrder(obj.uncached) + result := make([]string, 0, len(allKeys)) + for _, key := range allKeys { + hide, exists := visibility[key] + if !exists { + continue + } + if h == withHidden || hide != ast.ObjectFieldHidden { + result = append(result, key) + } + } + return result +} + func uncachedObjectFieldsVisibility(obj uncachedObject) fieldHideMap { r := make(fieldHideMap) switch obj := obj.(type) { diff --git a/vm.go b/vm.go index 4a07b068..539ed3f3 100644 --- a/vm.go +++ b/vm.go @@ -43,8 +43,9 @@ type VM struct { //nolint:govet nativeFuncs map[string]*NativeFunction importer Importer ErrorFormatter ErrorFormatter - StringOutput bool // expect to evaluate to a string, and output that string directly - OutputNewline bool // add a trailing newline (default true) + StringOutput bool // expect to evaluate to a string, and output that string directly + OutputNewline bool // add a trailing newline (default true) + PreserveFieldOrder bool // preserve field declaration order instead of sorting alphabetically importCache *importCache traceOut io.Writer EvalHook EvalHook @@ -178,10 +179,10 @@ const ( ) // version is the current gojsonnet's version -const version = "v0.22.0" +const version = "v0.22.1-fork.1" func (vm *VM) buildConfiguredInterpreter() (*interpreter, error) { - return buildInterpreter(vm.ext, vm.nativeFuncs, vm.MaxStack, vm.importCache, vm.traceOut, vm.EvalHook) + return buildInterpreter(vm.ext, vm.nativeFuncs, vm.MaxStack, vm.importCache, vm.traceOut, vm.EvalHook, vm.PreserveFieldOrder) } // Evaluate evaluates a Jsonnet program given by an Abstract Syntax Tree