self review

Author: bahrmichael

cmd/src/abc_variables_set.go

@@ -1,6 +1,7 @@
 package main
 
 import (
+	"bytes"
 	"context"
 	"encoding/json"
 	"flag"
@@ -99,15 +100,18 @@ Values are interpreted as JSON literals when valid. Otherwise they are sent as p
 }
 
 func marshalABCVariableValue(raw string) (value string, remove bool, err error) {
-	var parsed any
-	if err := json.Unmarshal([]byte(raw), &parsed); err != nil {
-		parsed = raw
+	// Try to compact valid JSON literals first. This allows us to send null to delete values, as well as raw numbers.
+	// If we that doesn't work for the given value, fall back to string encoding.
+	var compact bytes.Buffer
+	if err := json.Compact(&compact, []byte(raw)); err == nil {
+		value := compact.String()
+		return value, value == "null", nil
 	}
 
-	encoded, err := json.Marshal(parsed)
+	encoded, err := json.Marshal(raw)
 	if err != nil {
 		return "", false, err
 	}
 
-	return string(encoded), parsed == nil, nil
+	return string(encoded), false, nil
 }

cmd/src/abc_variables_set_test.go

@@ -0,0 +1,33 @@
+package main
+
+import "testing"
+
+func TestMarshalABCVariableValuePreservesLargeIntegerLiteral(t *testing.T) {
+	t.Parallel()
+
+	value, remove, err := marshalABCVariableValue("9007199254740993")
+	if err != nil {
+		t.Fatalf("marshalABCVariableValue returned error: %v", err)
+	}
+	if remove {
+		t.Fatal("marshalABCVariableValue unexpectedly marked value for removal")
+	}
+	if value != "9007199254740993" {
+		t.Fatalf("marshalABCVariableValue = %q, want %q", value, "9007199254740993")
+	}
+}
+
+func TestMarshalABCVariableValueTreatsNullAsRemoval(t *testing.T) {
+	t.Parallel()
+
+	value, remove, err := marshalABCVariableValue("null")
+	if err != nil {
+		t.Fatalf("marshalABCVariableValue returned error: %v", err)
+	}
+	if !remove {
+		t.Fatal("marshalABCVariableValue did not mark null for removal")
+	}
+	if value != "null" {
+		t.Fatalf("marshalABCVariableValue = %q, want %q", value, "null")
+	}
+}