cmd/cloner, cmd/viewer: handle named map/slice types with Clone/View methods

The cloner and viewer code generators didn't handle named types
with basic underlying types (map/slice) that have their own Clone
or View methods. For example, a type like:

    type Map map[string]any
    func (m Map) Clone() Map { ... }
    func (m Map) View() MapView { ... }

When used as a struct field, the cloner would descend into the
underlying map[string]any and fail because it can't clone the any
(interface{}) value type. Similarly, the viewer would try to create
a MapFnOf view and fail.

Fix the cloner to check for a Clone method on the named type
before falling through to the underlying type handling.

Fix the viewer to check for a View method on named map/slice types,
so the type author can provide a purpose-built safe view that
doesn't leak raw any values. Named map/slice types without a View
method fall through to normal handling, which correctly rejects
types like map[string]any as unsupported.

Updates tailscale/corp#39502 (needed by tailscale/corp#39594)

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

cmd/cloner/cloner.go

@@ -129,6 +129,12 @@ func gen(buf *bytes.Buffer, it *codegen.ImportTracker, typ *types.Named) {
 				}
 				continue
 			}
+			// Named types with basic underlying types (map/slice) that
+			// have their own Clone method should use it directly.
+			if methodResultType(ft, "Clone") != nil {
+				writef("dst.%s = src.%s.Clone()", fname, fname)
+				continue
+			}
 		}
 		switch ft := ft.Underlying().(type) {
 		case *types.Slice:

cmd/cloner/cloner_test.go

@@ -154,6 +154,34 @@ func TestMapWithPointers(t *testing.T) {
 	}
 }
 
+func TestNamedMapContainer(t *testing.T) {
+	orig := &clonerex.NamedMapContainer{
+		Attrs: clonerex.NamedMap{
+			"str":  "hello",
+			"num":  int64(42),
+			"bool": true,
+		},
+	}
+
+	cloned := orig.Clone()
+	if !reflect.DeepEqual(orig, cloned) {
+		t.Errorf("Clone() = %v, want %v", cloned, orig)
+	}
+
+	// Mutate the cloned map to verify no aliasing.
+	cloned.Attrs["str"] = "modified"
+	if orig.Attrs["str"] == "modified" {
+		t.Errorf("Clone() aliased memory in Attrs: original was modified")
+	}
+
+	// Verify nil handling.
+	nilContainer := &clonerex.NamedMapContainer{}
+	nilClone := nilContainer.Clone()
+	if !reflect.DeepEqual(nilContainer, nilClone) {
+		t.Errorf("Clone() of nil Attrs = %v, want %v", nilClone, nilContainer)
+	}
+}
+
 func TestDeeplyNestedMap(t *testing.T) {
 	num := 123
 	orig := &clonerex.DeeplyNestedMap{

cmd/cloner/clonerex/clonerex.go

@@ -1,7 +1,7 @@
 // Copyright (c) Tailscale Inc & contributors
 // SPDX-License-Identifier: BSD-3-Clause
 
-//go:generate go run tailscale.com/cmd/cloner  -clonefunc=true -type SliceContainer,InterfaceContainer,MapWithPointers,DeeplyNestedMap
+//go:generate go run tailscale.com/cmd/cloner  -clonefunc=true -type SliceContainer,InterfaceContainer,MapWithPointers,DeeplyNestedMap,NamedMapContainer
 
 // Package clonerex is an example package for the cloner tool.
 package clonerex
@@ -39,6 +39,27 @@ type MapWithPointers struct {
 	CloneInterface  map[string]Cloneable
 }
 
+// NamedMap is a named map type with its own Clone method.
+// This tests that the cloner uses the type's Clone method
+// rather than trying to descend into the map's value type.
+type NamedMap map[string]any
+
+func (m NamedMap) Clone() NamedMap {
+	if m == nil {
+		return nil
+	}
+	m2 := make(NamedMap, len(m))
+	for k, v := range m {
+		m2[k] = v
+	}
+	return m2
+}
+
+// NamedMapContainer has a field whose type is a named map with a Clone method.
+type NamedMapContainer struct {
+	Attrs NamedMap
+}
+
 // DeeplyNestedMap tests arbitrary depth of map nesting (3+ levels)
 type DeeplyNestedMap struct {
 	ThreeLevels map[string]map[string]map[string]int

cmd/cloner/clonerex/clonerex_clone.go

@@ -159,9 +159,26 @@ var _DeeplyNestedMapCloneNeedsRegeneration = DeeplyNestedMap(struct {
 	FourLevels  map[string]map[string]map[string]map[string]*SliceContainer
 }{})
 
+// Clone makes a deep copy of NamedMapContainer.
+// The result aliases no memory with the original.
+func (src *NamedMapContainer) Clone() *NamedMapContainer {
+	if src == nil {
+		return nil
+	}
+	dst := new(NamedMapContainer)
+	*dst = *src
+	dst.Attrs = src.Attrs.Clone()
+	return dst
+}
+
+// A compilation failure here means this code must be regenerated, with the command at the top of this file.
+var _NamedMapContainerCloneNeedsRegeneration = NamedMapContainer(struct {
+	Attrs NamedMap
+}{})
+
 // Clone duplicates src into dst and reports whether it succeeded.
 // To succeed, <src, dst> must be of types <*T, *T> or <*T, **T>,
-// where T is one of SliceContainer,InterfaceContainer,MapWithPointers,DeeplyNestedMap.
+// where T is one of SliceContainer,InterfaceContainer,MapWithPointers,DeeplyNestedMap,NamedMapContainer.
 func Clone(dst, src any) bool {
 	switch src := src.(type) {
 	case *SliceContainer:
@@ -200,6 +217,15 @@ func Clone(dst, src any) bool {
 			*dst = src.Clone()
 			return true
 		}
+	case *NamedMapContainer:
+		switch dst := dst.(type) {
+		case *NamedMapContainer:
+			*dst = *src.Clone()
+			return true
+		case **NamedMapContainer:
+			*dst = src.Clone()
+			return true
+		}
 	}
 	return false
 }

cmd/viewer/tests/tests.go

@@ -12,7 +12,7 @@ import (
 	"tailscale.com/types/views"
 )
 
-//go:generate go run tailscale.com/cmd/viewer --type=StructWithPtrs,StructWithoutPtrs,Map,StructWithSlices,OnlyGetClone,StructWithEmbedded,GenericIntStruct,GenericNoPtrsStruct,GenericCloneableStruct,StructWithContainers,StructWithTypeAliasFields,GenericTypeAliasStruct,StructWithMapOfViews --clone-only-type=OnlyGetClone
+//go:generate go run tailscale.com/cmd/viewer --type=StructWithPtrs,StructWithoutPtrs,Map,StructWithSlices,OnlyGetClone,StructWithEmbedded,GenericIntStruct,GenericNoPtrsStruct,GenericCloneableStruct,StructWithContainers,StructWithTypeAliasFields,GenericTypeAliasStruct,StructWithMapOfViews,StructWithNamedMap,StructWithNamedSlice --clone-only-type=OnlyGetClone
 
 type StructWithoutPtrs struct {
 	Int int
@@ -241,3 +241,61 @@ type GenericTypeAliasStruct[T integer, T2 views.ViewCloner[T2, V2], V2 views.Str
 type StructWithMapOfViews struct {
 	MapOfViews map[string]StructWithoutPtrsView
 }
+
+// NamedMap is a named map type with its own Clone and View methods.
+// This tests that the viewer calls View() on named map types rather
+// than trying to generate a view of the underlying map[string]any.
+type NamedMap map[string]any
+
+func (m NamedMap) Clone() NamedMap {
+	if m == nil {
+		return nil
+	}
+	m2 := make(NamedMap, len(m))
+	for k, v := range m {
+		m2[k] = v
+	}
+	return m2
+}
+
+// NamedMapView is a read-only view of NamedMap.
+type NamedMapView struct {
+	ж NamedMap
+}
+
+func (m NamedMap) View() NamedMapView { return NamedMapView{m} }
+
+func (v NamedMapView) Get(k string) (any, bool) { val, ok := v.ж[k]; return val, ok }
+func (v NamedMapView) Len() int                 { return len(v.ж) }
+
+type StructWithNamedMap struct {
+	Attrs NamedMap
+}
+
+// NamedSlice is a named slice type with its own Clone and View methods.
+// This tests that the viewer calls View() on named slice types rather
+// than trying to generate a view of the underlying []any.
+type NamedSlice []any
+
+func (s NamedSlice) Clone() NamedSlice {
+	if s == nil {
+		return nil
+	}
+	s2 := make(NamedSlice, len(s))
+	copy(s2, s)
+	return s2
+}
+
+// NamedSliceView is a read-only view of NamedSlice.
+type NamedSliceView struct {
+	ж NamedSlice
+}
+
+func (s NamedSlice) View() NamedSliceView { return NamedSliceView{s} }
+
+func (v NamedSliceView) At(i int) any { return v.ж[i] }
+func (v NamedSliceView) Len() int     { return len(v.ж) }
+
+type StructWithNamedSlice struct {
+	Items NamedSlice
+}

cmd/viewer/tests/tests_clone.go

@@ -563,3 +563,37 @@ func (src *StructWithMapOfViews) Clone() *StructWithMapOfViews {
 var _StructWithMapOfViewsCloneNeedsRegeneration = StructWithMapOfViews(struct {
 	MapOfViews map[string]StructWithoutPtrsView
 }{})
+
+// Clone makes a deep copy of StructWithNamedMap.
+// The result aliases no memory with the original.
+func (src *StructWithNamedMap) Clone() *StructWithNamedMap {
+	if src == nil {
+		return nil
+	}
+	dst := new(StructWithNamedMap)
+	*dst = *src
+	dst.Attrs = src.Attrs.Clone()
+	return dst
+}
+
+// A compilation failure here means this code must be regenerated, with the command at the top of this file.
+var _StructWithNamedMapCloneNeedsRegeneration = StructWithNamedMap(struct {
+	Attrs NamedMap
+}{})
+
+// Clone makes a deep copy of StructWithNamedSlice.
+// The result aliases no memory with the original.
+func (src *StructWithNamedSlice) Clone() *StructWithNamedSlice {
+	if src == nil {
+		return nil
+	}
+	dst := new(StructWithNamedSlice)
+	*dst = *src
+	dst.Items = src.Items.Clone()
+	return dst
+}
+
+// A compilation failure here means this code must be regenerated, with the command at the top of this file.
+var _StructWithNamedSliceCloneNeedsRegeneration = StructWithNamedSlice(struct {
+	Items NamedSlice
+}{})

cmd/viewer/tests/tests_view.go

@@ -16,7 +16,7 @@ import (
 	"tailscale.com/types/views"
 )
 
-//go:generate go run tailscale.com/cmd/cloner  -clonefunc=false -type=StructWithPtrs,StructWithoutPtrs,Map,StructWithSlices,OnlyGetClone,StructWithEmbedded,GenericIntStruct,GenericNoPtrsStruct,GenericCloneableStruct,StructWithContainers,StructWithTypeAliasFields,GenericTypeAliasStruct,StructWithMapOfViews
+//go:generate go run tailscale.com/cmd/cloner  -clonefunc=false -type=StructWithPtrs,StructWithoutPtrs,Map,StructWithSlices,OnlyGetClone,StructWithEmbedded,GenericIntStruct,GenericNoPtrsStruct,GenericCloneableStruct,StructWithContainers,StructWithTypeAliasFields,GenericTypeAliasStruct,StructWithMapOfViews,StructWithNamedMap,StructWithNamedSlice
 
 // View returns a read-only view of StructWithPtrs.
 func (p *StructWithPtrs) View() StructWithPtrsView {
@@ -1129,3 +1129,151 @@ func (v StructWithMapOfViewsView) MapOfViews() views.Map[string, StructWithoutPt
 var _StructWithMapOfViewsViewNeedsRegeneration = StructWithMapOfViews(struct {
 	MapOfViews map[string]StructWithoutPtrsView
 }{})
+
+// View returns a read-only view of StructWithNamedMap.
+func (p *StructWithNamedMap) View() StructWithNamedMapView {
+	return StructWithNamedMapView{ж: p}
+}
+
+// StructWithNamedMapView provides a read-only view over StructWithNamedMap.
+//
+// Its methods should only be called if `Valid()` returns true.
+type StructWithNamedMapView struct {
+	// ж is the underlying mutable value, named with a hard-to-type
+	// character that looks pointy like a pointer.
+	// It is named distinctively to make you think of how dangerous it is to escape
+	// to callers. You must not let callers be able to mutate it.
+	ж *StructWithNamedMap
+}
+
+// Valid reports whether v's underlying value is non-nil.
+func (v StructWithNamedMapView) Valid() bool { return v.ж != nil }
+
+// AsStruct returns a clone of the underlying value which aliases no memory with
+// the original.
+func (v StructWithNamedMapView) AsStruct() *StructWithNamedMap {
+	if v.ж == nil {
+		return nil
+	}
+	return v.ж.Clone()
+}
+
+// MarshalJSON implements [jsonv1.Marshaler].
+func (v StructWithNamedMapView) MarshalJSON() ([]byte, error) {
+	return jsonv1.Marshal(v.ж)
+}
+
+// MarshalJSONTo implements [jsonv2.MarshalerTo].
+func (v StructWithNamedMapView) MarshalJSONTo(enc *jsontext.Encoder) error {
+	return jsonv2.MarshalEncode(enc, v.ж)
+}
+
+// UnmarshalJSON implements [jsonv1.Unmarshaler].
+func (v *StructWithNamedMapView) UnmarshalJSON(b []byte) error {
+	if v.ж != nil {
+		return errors.New("already initialized")
+	}
+	if len(b) == 0 {
+		return nil
+	}
+	var x StructWithNamedMap
+	if err := jsonv1.Unmarshal(b, &x); err != nil {
+		return err
+	}
+	v.ж = &x
+	return nil
+}
+
+// UnmarshalJSONFrom implements [jsonv2.UnmarshalerFrom].
+func (v *StructWithNamedMapView) UnmarshalJSONFrom(dec *jsontext.Decoder) error {
+	if v.ж != nil {
+		return errors.New("already initialized")
+	}
+	var x StructWithNamedMap
+	if err := jsonv2.UnmarshalDecode(dec, &x); err != nil {
+		return err
+	}
+	v.ж = &x
+	return nil
+}
+
+func (v StructWithNamedMapView) Attrs() NamedMapView { return v.ж.Attrs.View() }
+
+// A compilation failure here means this code must be regenerated, with the command at the top of this file.
+var _StructWithNamedMapViewNeedsRegeneration = StructWithNamedMap(struct {
+	Attrs NamedMap
+}{})
+
+// View returns a read-only view of StructWithNamedSlice.
+func (p *StructWithNamedSlice) View() StructWithNamedSliceView {
+	return StructWithNamedSliceView{ж: p}
+}
+
+// StructWithNamedSliceView provides a read-only view over StructWithNamedSlice.
+//
+// Its methods should only be called if `Valid()` returns true.
+type StructWithNamedSliceView struct {
+	// ж is the underlying mutable value, named with a hard-to-type
+	// character that looks pointy like a pointer.
+	// It is named distinctively to make you think of how dangerous it is to escape
+	// to callers. You must not let callers be able to mutate it.
+	ж *StructWithNamedSlice
+}
+
+// Valid reports whether v's underlying value is non-nil.
+func (v StructWithNamedSliceView) Valid() bool { return v.ж != nil }
+
+// AsStruct returns a clone of the underlying value which aliases no memory with
+// the original.
+func (v StructWithNamedSliceView) AsStruct() *StructWithNamedSlice {
+	if v.ж == nil {
+		return nil
+	}
+	return v.ж.Clone()
+}
+
+// MarshalJSON implements [jsonv1.Marshaler].
+func (v StructWithNamedSliceView) MarshalJSON() ([]byte, error) {
+	return jsonv1.Marshal(v.ж)
+}
+
+// MarshalJSONTo implements [jsonv2.MarshalerTo].
+func (v StructWithNamedSliceView) MarshalJSONTo(enc *jsontext.Encoder) error {
+	return jsonv2.MarshalEncode(enc, v.ж)
+}
+
+// UnmarshalJSON implements [jsonv1.Unmarshaler].
+func (v *StructWithNamedSliceView) UnmarshalJSON(b []byte) error {
+	if v.ж != nil {
+		return errors.New("already initialized")
+	}
+	if len(b) == 0 {
+		return nil
+	}
+	var x StructWithNamedSlice
+	if err := jsonv1.Unmarshal(b, &x); err != nil {
+		return err
+	}
+	v.ж = &x
+	return nil
+}
+
+// UnmarshalJSONFrom implements [jsonv2.UnmarshalerFrom].
+func (v *StructWithNamedSliceView) UnmarshalJSONFrom(dec *jsontext.Decoder) error {
+	if v.ж != nil {
+		return errors.New("already initialized")
+	}
+	var x StructWithNamedSlice
+	if err := jsonv2.UnmarshalDecode(dec, &x); err != nil {
+		return err
+	}
+	v.ж = &x
+	return nil
+}
+
+func (v StructWithNamedSliceView) Items() NamedSliceView { return v.ж.Items.View() }
+
+// A compilation failure here means this code must be regenerated, with the command at the top of this file.
+var _StructWithNamedSliceViewNeedsRegeneration = StructWithNamedSlice(struct {
+	Items NamedSlice
+}{})

cmd/viewer/viewer.go

@@ -282,6 +282,22 @@ func genView(buf *bytes.Buffer, it *codegen.ImportTracker, typ *types.Named, fie
 			writeTemplateWithComment("valueField", fname)
 			continue
 		}
+		// Named map/slice types whose element type is opaque (e.g. any)
+		// can't be safely wrapped in views.Map/views.Slice because the
+		// accessor would leak the raw element.  If the type provides its
+		// own View() method the author can return a purpose-built safe
+		// view; use it.  Otherwise fall through to the normal handling,
+		// which will reject the type as unsupported.
+		if named, _ := codegen.NamedTypeOf(fieldType); named != nil {
+			switch fieldType.Underlying().(type) {
+			case *types.Map, *types.Slice:
+				if viewType := viewTypeForValueType(fieldType); viewType != nil {
+					args.FieldViewName = it.QualifiedName(viewType)
+					writeTemplateWithComment("viewField", fname)
+					continue
+				}
+			}
+		}
 		switch underlying := fieldType.Underlying().(type) {
 		case *types.Slice:
 			slice := underlying

cmd/viewer/viewer_test.go

@@ -10,11 +10,109 @@ import (
 	"go/parser"
 	"go/token"
 	"go/types"
+	"strings"
 	"testing"
 
 	"tailscale.com/util/codegen"
 )
 
+// TestNamedMapWithView tests that a named map type with a user-supplied
+// View() method causes the generated view accessor to call .View() and
+// return the user-defined view type. Without the View() method the
+// generator should reject the field as unsupported.
+func TestNamedMapWithView(t *testing.T) {
+	const src = `
+package test
+
+// AttrMap is a named map whose values are opaque (any).
+// It provides its own Clone and View methods.
+type AttrMap map[string]any
+
+func (m AttrMap) Clone() AttrMap {
+	m2 := make(AttrMap, len(m))
+	for k, v := range m { m2[k] = v }
+	return m2
+}
+
+// AttrMapView is a hand-written read-only view of AttrMap.
+type AttrMapView struct{ m AttrMap }
+
+func (m AttrMap) View() AttrMapView { return AttrMapView{m} }
+
+// Container holds an AttrMap field.
+type Container struct {
+	Attrs AttrMap
+}
+`
+	output := genViewOutput(t, src, "Container")
+
+	// The generated accessor must call .View() and return the
+	// user-defined AttrMapView, not views.Map or the raw AttrMap.
+	const want = "func (v ContainerView) Attrs() AttrMapView { return v.ж.Attrs.View() }"
+	if !strings.Contains(output, want) {
+		t.Errorf("generated output missing expected accessor\nwant: %s\ngot:\n%s", want, output)
+	}
+}
+
+// TestNamedMapWithoutView tests that a named map[string]any WITHOUT a
+// View() method does NOT generate an accessor that calls .View().
+func TestNamedMapWithoutView(t *testing.T) {
+	const src = `
+package test
+
+type AttrMap map[string]any
+
+func (m AttrMap) Clone() AttrMap {
+	m2 := make(AttrMap, len(m))
+	for k, v := range m { m2[k] = v }
+	return m2
+}
+
+type Container struct {
+	Attrs AttrMap
+}
+`
+	output := genViewOutput(t, src, "Container")
+
+	// Must not generate an accessor that calls .Attrs.View(),
+	// since AttrMap doesn't have a View() method.
+	if strings.Contains(output, "Attrs.View()") {
+		t.Errorf("generated code calls .Attrs.View() but AttrMap has no View method:\n%s", output)
+	}
+	// Must not return AttrMapView (which doesn't exist).
+	if strings.Contains(output, "AttrMapView") {
+		t.Errorf("generated code references AttrMapView but it doesn't exist:\n%s", output)
+	}
+}
+
+// genViewOutput parses src, runs genView on the named type, and returns
+// the generated Go source.
+func genViewOutput(t *testing.T, src string, typeName string) string {
+	t.Helper()
+	fset := token.NewFileSet()
+	f, err := parser.ParseFile(fset, "test.go", src, 0)
+	if err != nil {
+		t.Fatal(err)
+	}
+	conf := types.Config{}
+	pkg, err := conf.Check("test", fset, []*ast.File{f}, nil)
+	if err != nil {
+		t.Fatal(err)
+	}
+	obj := pkg.Scope().Lookup(typeName)
+	if obj == nil {
+		t.Fatalf("type %q not found", typeName)
+	}
+	named, ok := obj.(*types.TypeName).Type().(*types.Named)
+	if !ok {
+		t.Fatalf("%q is not a named type", typeName)
+	}
+	var buf bytes.Buffer
+	tracker := codegen.NewImportTracker(pkg)
+	genView(&buf, tracker, named, nil)
+	return buf.String()
+}
+
 func TestViewerImports(t *testing.T) {
 	tests := []struct {
 		name        string

net/dns/dns_clone.go

@@ -6,7 +6,6 @@
 package dns
 
 import (
-	"maps"
 	"net/netip"
 
 	"tailscale.com/types/dnstype"
@@ -45,7 +44,7 @@ func (src *Config) Clone() *Config {
 			dst.Hosts[k] = append([]netip.Addr{}, src.Hosts[k]...)
 		}
 	}
-	dst.SubdomainHosts = maps.Clone(src.SubdomainHosts)
+	dst.SubdomainHosts = src.SubdomainHosts.Clone()
 	return dst
 }