allow map[string]interface{}, and other such things, as scalars

generate/imports.go

@@ -1,7 +1,9 @@
 package generate
 
 import (
+	"fmt"
 	"go/types"
+	"regexp"
 	"strconv"
 	"strings"
 )
@@ -35,24 +37,49 @@ func (g *generator) ref(fullyQualifiedName string) (qualifiedName string, err er
 	return g.getRef(fullyQualifiedName, false)
 }
 
+var _sliceOrMapPrefixRegexp = regexp.MustCompile(`^(\*|\[\d*\]|map\[string\])*`)
+
 func (g *generator) getRef(fullyQualifiedName string, addImport bool) (qualifiedName string, err error) {
-	i := strings.LastIndex(fullyQualifiedName, ".")
+	// Ideally, we want to allow a reference to basically an arbitrary symbol.
+	// But that's very hard, because it might be quite complicated, like
+	//	struct{ F []map[mypkg.K]otherpkg.V }
+	// Now in practice, using an unnamed struct is not a great idea, but we do
+	// want to allow as much as we can that encoding/json knows how to work
+	// with, since you would reasonably expect us to accept, say,
+	// map[string][]interface{}.  So we allow:
+	// - any named type (mypkg.T)
+	// - any predeclared basic type (string, int, etc.)
+	// - interface{}
+	// - for any allowed type T, *T, []T, [N]T, and map[string]T
+	// which effectively excludes:
+	// - unnamed struct types
+	// - map[K]V where K is a named type wrapping string
+	// - any nonstandard spelling of those (interface {/* hi */},
+	//	 map[  string      ]T)
+	// TODO: document that somewhere visible
+
+	errorMsg := `invalid type-name "%v" (%v); expected a builtin, ` +
+		`path/to/package.Name, interface{}, or a slice, map, or pointer of those`
+
+	if strings.Contains(fullyQualifiedName, " ") {
+		// TODO: pass in pos here and below
+		return "", errorf(nil, errorMsg, fullyQualifiedName, "contains spaces")
+	}
+
+	prefix := _sliceOrMapPrefixRegexp.FindString(fullyQualifiedName)
+	nameToImport := fullyQualifiedName[len(prefix):]
+
+	i := strings.LastIndex(nameToImport, ".")
 	if i == -1 {
-		// We allow any builtin type, or interface{}.  In principle it would be
-		// fine to allow any interface or struct, but (1) they might refer to a
-		// type that needs an import, and (2) that just honestly seems
-		// confusing, why would you want it.  But the empty interface,
-		// specifically, is useful.
-		if fullyQualifiedName != "interface{}" && types.Universe.Lookup(fullyQualifiedName) == nil {
-			// TODO: pass in pos here
-			return "", errorf(nil,
-				`unknown name "%v"; expected a builtin or path/to/package.Name`, fullyQualifiedName)
+		if nameToImport != "interface{}" && types.Universe.Lookup(nameToImport) == nil {
+			return "", errorf(nil, errorMsg, fullyQualifiedName,
+				fmt.Sprintf(`unknown type-name "%v"`, nameToImport))
 		}
 		return fullyQualifiedName, nil
 	}
 
-	pkgPath := fullyQualifiedName[:i]
-	localName := fullyQualifiedName[i+1:]
+	pkgPath := nameToImport[:i]
+	localName := nameToImport[i+1:]
 	var alias string
 	if addImport {
 		alias = g.addImportFor(pkgPath)
@@ -60,10 +87,11 @@ func (g *generator) getRef(fullyQualifiedName string, addImport bool) (qualified
 		var ok bool
 		alias, ok = g.imports[pkgPath]
 		if !ok {
+			// This is an internal error, not a user error.
 			return "", errorf(nil, `no alias defined for package "%v"`, pkgPath)
 		}
 	}
-	return alias + "." + localName, nil
+	return prefix + alias + "." + localName, nil
 }
 
 // Returns the import-clause to use in the generated code.