util/mak: deprecate NonNil, add type-safe NonNilSliceForJSON, NonNilMapForJSON

And put the rationale in the name too to save the callers the need for a comment.

Change-Id: I090f51b749a5a0641897ee89a8fb2e2080c8b782
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>

ipn/localapi/localapi.go

@@ -18,7 +18,6 @@ import (
 	"net/http/httputil"
 	"net/netip"
 	"net/url"
-	"reflect"
 	"runtime"
 	"strconv"
 	"strings"
@@ -34,6 +33,7 @@ import (
 	"tailscale.com/tka"
 	"tailscale.com/types/logger"
 	"tailscale.com/util/clientmetric"
+	"tailscale.com/util/mak"
 	"tailscale.com/version"
 )
 
@@ -527,7 +527,7 @@ func (h *Handler) serveFileTargets(w http.ResponseWriter, r *http.Request) {
 		writeErrorJSON(w, err)
 		return
 	}
-	makeNonNil(&fts)
+	mak.NonNilSliceForJSON(&fts)
 	w.Header().Set("Content-Type", "application/json")
 	json.NewEncoder(w).Encode(fts)
 }
@@ -858,30 +858,3 @@ func defBool(a string, def bool) bool {
 	}
 	return v
 }
-
-// makeNonNil takes a pointer to a Go data structure
-// (currently only a slice or a map) and makes sure it's non-nil for
-// JSON serialization. (In particular, JavaScript clients usually want
-// the field to be defined after they decode the JSON.)
-func makeNonNil(ptr any) {
-	if ptr == nil {
-		panic("nil interface")
-	}
-	rv := reflect.ValueOf(ptr)
-	if rv.Kind() != reflect.Ptr {
-		panic(fmt.Sprintf("kind %v, not Ptr", rv.Kind()))
-	}
-	if rv.Pointer() == 0 {
-		panic("nil pointer")
-	}
-	rv = rv.Elem()
-	if rv.Pointer() != 0 {
-		return
-	}
-	switch rv.Type().Kind() {
-	case reflect.Slice:
-		rv.Set(reflect.MakeSlice(rv.Type(), 0, 0))
-	case reflect.Map:
-		rv.Set(reflect.MakeMap(rv.Type()))
-	}
-}

util/mak/mak.go

@@ -25,10 +25,8 @@ func Set[K comparable, V any, T ~map[K]V](m *T, k K, v V) {
 // (currently only a slice or a map) and makes sure it's non-nil for
 // JSON serialization. (In particular, JavaScript clients usually want
 // the field to be defined after they decode the JSON.)
-// MakeNonNil takes a pointer to a Go data structure
+//
-// (currently only a slice or a map) and makes sure it's non-nil for
+// Deprecated: use NonNilSliceForJSON or NonNilMapForJSON instead.
-// JSON serialization. (In particular, JavaScript clients usually want
-// the field to be defined after they decode the JSON.)
 func NonNil(ptr interface{}) {
 	if ptr == nil {
 		panic("nil interface")
@@ -51,3 +49,23 @@ func NonNil(ptr interface{}) {
 		rv.Set(reflect.MakeMap(rv.Type()))
 	}
 }
+
+// NonNilSliceForJSON makes sure that *slicePtr is non-nil so it will
+// won't be omitted from JSON serialization and possibly confuse JavaScript
+// clients expecting it to be preesnt.
+func NonNilSliceForJSON[T any, S ~[]T](slicePtr *S) {
+	if *slicePtr != nil {
+		return
+	}
+	*slicePtr = make([]T, 0)
+}
+
+// NonNilMapForJSON makes sure that *slicePtr is non-nil so it will
+// won't be omitted from JSON serialization and possibly confuse JavaScript
+// clients expecting it to be preesnt.
+func NonNilMapForJSON[K comparable, V any, M ~map[K]V](mapPtr *M) {
+	if *mapPtr != nil {
+		return
+	}
+	*mapPtr = make(M)
+}

util/mak/mak_test.go

@@ -69,3 +69,21 @@ func TestNonNil(t *testing.T) {
 		t.Error("map still nil")
 	}
 }
+
+func TestNonNilMapForJSON(t *testing.T) {
+	type M map[string]int
+	var m M
+	NonNilMapForJSON(&m)
+	if m == nil {
+		t.Fatal("still nil")
+	}
+}
+
+func TestNonNilSliceForJSON(t *testing.T) {
+	type S []int
+	var s S
+	NonNilSliceForJSON(&s)
+	if s == nil {
+		t.Fatal("still nil")
+	}
+}