Skip to content

Commit 1fa09f1

Browse files
aledbfclaude
andcommitted
refactor(features): hold jsObject's index order and truthiness in tests
Array-index properties are ordered by comparing their canonical decimal forms instead of parsing them again, and isArrayIndex reports only whether a key is one. The tests now hold the 2^32-1 bound with 4294967294 and 4294967296, and the truthiness of a number in the deprecated-property move. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
1 parent af4909b commit 1fa09f1

3 files changed

Lines changed: 14 additions & 13 deletions

File tree

‎internal/features/jsobject.go‎

Lines changed: 9 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import (
88
"io"
99
"slices"
1010
"strconv"
11+
"strings"
1112
"unicode/utf16"
1213
)
1314

@@ -56,29 +57,27 @@ func (o *jsObject) spread(src *jsObject) *jsObject {
5657
return out
5758
}
5859

59-
// arrayIndex reports whether k is an array index, which JS orders first.
60-
func arrayIndex(k string) (uint64, bool) {
60+
// isArrayIndex reports whether k is an array index (a canonical integer
61+
// below 2^32-1), which JS orders first.
62+
func isArrayIndex(k string) bool {
6163
n, err := strconv.ParseUint(k, 10, 32)
62-
if err != nil || n == 1<<32-1 || strconv.FormatUint(n, 10) != k {
63-
return 0, false
64-
}
65-
return n, true
64+
return err == nil && n != 1<<32-1 && strconv.FormatUint(n, 10) == k
6665
}
6766

6867
// order is the order JSON.stringify writes o's properties in.
6968
func (o *jsObject) order() []string {
7069
var idx, rest []string
7170
for _, k := range o.keys {
72-
if _, ok := arrayIndex(k); ok {
71+
if isArrayIndex(k) {
7372
idx = append(idx, k)
7473
} else {
7574
rest = append(rest, k)
7675
}
7776
}
77+
// Array indexes are canonical: shorter is smaller, equal lengths compare
78+
// as strings.
7879
slices.SortFunc(idx, func(a, b string) int {
79-
x, _ := arrayIndex(a)
80-
y, _ := arrayIndex(b)
81-
return cmp.Compare(x, y)
80+
return cmp.Or(cmp.Compare(len(a), len(b)), strings.Compare(a, b))
8281
})
8382
return append(idx, rest...)
8483
}

‎internal/features/jsobject_test.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,15 +7,15 @@ import (
77

88
// JSON.parse then JSON.stringify: a repeated property keeps its first
99
// position and its last value, array-index properties come first, ascending
10-
// (4294967295, "01" and "-1" are not indexes), the rest in insertion order.
10+
// (4294967295 and above, "01" and "-1" are not indexes), the rest in insertion order.
1111
func TestJSObjectRoundTripsAsJavaScript(t *testing.T) {
12-
src := `{"b":1,"4294967295":2,"10":3,"01":4,"2":5,"b":6,"-1":7,"a":[{"z":1,"y":[]}],"n":1.50,"e":1e21}`
12+
src := `{"b":1,"4294967295":2,"10":3,"01":4,"2":5,"b":6,"-1":7,"4294967296":8,"9":9,"4294967294":10,"a":[{"z":1,"y":[]}],"n":1.50,"e":1e21}`
1313
v, err := parseJS([]byte(src))
1414
if err != nil {
1515
t.Fatal(err)
1616
}
1717
got, err := json.Marshal(v)
18-
want := `{"2":5,"10":3,"b":6,"4294967295":2,"01":4,"-1":7,"a":[{"z":1,"y":[]}],"n":1.5,"e":1e+21}`
18+
want := `{"2":5,"9":9,"10":3,"4294967294":10,"b":6,"4294967295":2,"01":4,"-1":7,"4294967296":8,"a":[{"z":1,"y":[]}],"n":1.5,"e":1e+21}`
1919
if err != nil || string(got) != want {
2020
t.Errorf("got %s, %v\nwant %s", got, err, want)
2121
}

‎internal/features/types_order_test.go‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,8 @@ func TestFeatureMarshalJSONMovesDeprecatedProperties(t *testing.T) {
7979
`{"id":"c","customizations":{"vscode":{"extensions":"basex,1"}},"included":true,"value":null}`},
8080
{`{"id":"d","extensions":[],"settings":"ab"}`,
8181
`{"id":"d","included":true,"value":null,"customizations":{"vscode":{"extensions":[],"settings":{"0":"a","1":"b"}}}}`},
82+
{`{"id":"g","extensions":0,"settings":1}`,
83+
`{"id":"g","extensions":0,"included":true,"value":null,"customizations":{"vscode":{"settings":{}}}}`},
8284
{`{"id":"e","extensions":"","settings":null,"devPort":1}`,
8385
`{"id":"e","extensions":"","settings":null,"devPort":1,"included":true,"value":null}`},
8486
} {

0 commit comments

Comments
 (0)