Support non-hashable Generators in genall

It was previous possible for a generator implementation to become
non-hashable by adding non-hashable fields, and since interface values
compare by deferencing to their underlying values, this was a problem.

This changes it to refer to everything by a *pointer* to a Generator,
which just compares the pointer value.  Since we're shipping around
unique instances anyway, this is fine.

Author: DirectXMan12

pkg/genall/genall.go

@@ -90,7 +90,8 @@ type GenerationContext struct {
 }
 
 // WriteYAML writes the given objects out, serialized as YAML, using the
-// context's OutputRule.
+// context's OutputRule.  Objects are written as separate documents, separated
+// from each other by `---` (as per the YAML spec).
 func (g GenerationContext) WriteYAML(itemPath string, objs ...interface{}) error {
 	out, err := g.Open(nil, itemPath)
 	if err != nil {
@@ -161,10 +162,11 @@ func (r *Runtime) Run() bool {
 		return true
 	}
 
-	for _, gen := range r.Generators {
+	for i := range r.Generators {
+		gen := &r.Generators[i]    // don't take a reference to the loop variable
 		ctx := r.GenerationContext // make a shallow copy
 		ctx.OutputRule = r.OutputRules.ForGenerator(gen)
-		if err := gen.Generate(&ctx); err != nil {
+		if err := (*gen).Generate(&ctx); err != nil {
 			fmt.Fprintln(os.Stderr, err)
 		}
 	}

pkg/genall/options.go

@@ -108,14 +108,14 @@ func FromOptions(optionsRegistry *markers.Registry, options []string) (*Runtime,
 func protoFromOptions(optionsRegistry *markers.Registry, options []string) (protoRuntime, error) {
 	var gens Generators
 	rules := OutputRules{
-		ByGenerator: make(map[Generator]OutputRule),
+		ByGenerator: make(map[*Generator]OutputRule),
 	}
 	var paths []string
 
 	// collect the generators first, so that we can key the output on the actual
 	// generator, which matters if there's settings in the gen object and it's not a pointer.
 	outputByGen := make(map[string]OutputRule)
-	gensByName := make(map[string]Generator)
+	gensByName := make(map[string]*Generator)
 
 	for _, rawOpt := range options {
 		if rawOpt[0] != '+' {
@@ -134,7 +134,7 @@ func protoFromOptions(optionsRegistry *markers.Registry, options []string) (prot
 		switch val := val.(type) {
 		case Generator:
 			gens = append(gens, val)
-			gensByName[defn.Name] = val
+			gensByName[defn.Name] = &val
 		case OutputRule:
 			_, genName := splitOutputRuleOption(defn.Name)
 			if genName == "" {
@@ -176,7 +176,7 @@ type protoRuntime struct {
 	Paths            []string
 	Generators       Generators
 	OutputRules      OutputRules
-	GeneratorsByName map[string]Generator
+	GeneratorsByName map[string]*Generator
 }
 
 // splitOutputRuleOption splits a marker name of "output:rule:gen" or "output:rule"

pkg/genall/output.go

@@ -39,10 +39,10 @@ func (n nopCloser) Close() error {
 // DirectoryPerGenerator produces output rules mapping output to a different subdirectory
 // of the given base directory for each generator (with each subdirectory specified as
 // the key in the input map).
-func DirectoryPerGenerator(base string, generators map[string]Generator) OutputRules {
+func DirectoryPerGenerator(base string, generators map[string]*Generator) OutputRules {
 	rules := OutputRules{
 		Default:     OutputArtifacts{Config: OutputToDirectory(base)},
-		ByGenerator: make(map[Generator]OutputRule, len(generators)),
+		ByGenerator: make(map[*Generator]OutputRule, len(generators)),
 	}
 
 	for name, gen := range generators {
@@ -59,12 +59,15 @@ type OutputRules struct {
 	// Default is the output rule used when no specific per-generator overrides match.
 	Default OutputRule
 	// ByGenerator contains specific per-generator overrides.
-	ByGenerator map[Generator]OutputRule
+	// NB(directxman12): this is a pointer to avoid issues if a given Generator becomes unhashable
+	// (interface values compare by "dereferencing" their internal pointer first, whereas pointers
+	// compare by the actual pointer itself).
+	ByGenerator map[*Generator]OutputRule
 }
 
 // ForGenerator returns the output rule that should be used
 // by the given Generator.
-func (o OutputRules) ForGenerator(gen Generator) OutputRule {
+func (o OutputRules) ForGenerator(gen *Generator) OutputRule {
 	if forGen, specific := o.ByGenerator[gen]; specific {
 		return forGen
 	}