fix(codegen): dont return data from GetModuleHook in first pass

Stencil doesn't provide any guarantees around template rendering order.
This was a concious decision made to prevent overly complex templating
usecases where templates aren't the best place to encapsulate said
logic. However, in order for templates to be able to use module hooks,
we still needed the notion of an "add to hook" and "get hook" state.
This brought the two-pass render system that we have today. The first
render pass is for adding to module hooks while the second is for
retrieving module hook data. Only the results of the second pass are
used for writing to files on the filesystem.

However, due to an oversight in the original implementation, we never
actually limited the `GetModuleHook` function to return data just during
the second pass. It returns data even during the first pass, which means
that it's possible to access data in a non-determisitic fashion during
the first pass. This commit changes that behaviour to have
`GetModuleHook` always return `[]any` during the first pass. This
ensures that there is never any order dependent module hook data being
returned.

Also as part of this, to prevent further accidental leakage of file
ordering, we now sort templates randomly on each stencil run. This
mirrors mentality behind the Go maps to help prevent developers from
relying on a certain key ordering.

As a reminder for reviewers, module hooks only write to the module hook
in-memory store during the first pass. They noop during the second pass.

internal/codegen/stencil.go

@@ -6,11 +6,13 @@ package codegen
 import (
 	"context"
 	"io"
+	"math/rand"
 	"os"
 	"os/exec"
 	"path"
 	"path/filepath"
 	"sort"
+	"time"
 
 	"github.com/getoutreach/gobox/pkg/app"
 	"github.com/getoutreach/stencil/internal/modules"
@@ -338,6 +340,13 @@ func (s *Stencil) getTemplates(ctx context.Context, log logrus.FieldLogger) ([]*
 
 	log.Debug("Finished discovering templates")
 
+	// Shuffle the templates to prevent accidental file order guarantees
+	// from being relied upon.
+	//nolint:gosec // Why: We don't need that much entropy.
+	rand.New(rand.NewSource(time.Now().UnixNano())).Shuffle(len(tpls), func(i, j int) {
+		tpls[i], tpls[j] = tpls[j], tpls[i]
+	})
+
 	return tpls, nil
 }
 

internal/codegen/stencil_test.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"fmt"
 	"io"
+	"strings"
 	"testing"
 
 	"github.com/getoutreach/gobox/pkg/app"
@@ -92,7 +93,7 @@ func TestModuleHookRender(t *testing.T) {
 	assert.NilError(t, err, "expected Render() to not fail")
 	assert.Equal(t, len(tpls), 2, "expected Render() to return a single template")
 	assert.Equal(t, len(tpls[1].Files), 1, "expected Render() template to return a single file")
-	assert.Equal(t, tpls[1].Files[0].String(), "a", "expected Render() to return correct output")
+	assert.Equal(t, strings.TrimSpace(tpls[1].Files[0].String()), "a", "expected Render() to return correct output")
 }
 
 func ExampleStencil_PostRun() {

internal/codegen/testdata/module-hook/m2.tpl

@@ -1 +1,2 @@
-{{ index (stencil.GetModuleHook "coolthing") 0 }}
+{{- $mh := stencil.GetModuleHook "coolthing" }}
+{{ if $mh }}{{ index $mh 0 }}{{ end }}

internal/codegen/tpl_stencil.go

@@ -36,14 +36,23 @@ type TplStencil struct {
 // This is incredibly useful for allowing other modules to write
 // to files that your module owns. Think of them as extension points
 // for your module. The value returned by this function is always a
-// []any, aka a list.
+// []any, aka a list. This function only returns data during the second
+// pass of stencil to ensure that template render order doesn't impact
+// the data that is returned.
 //
 //	{{- /* This returns a []any */}}
 //	{{ $hook := stencil.GetModuleHook "myModuleHook" }}
 //	{{- range $hook }}
 //	  {{ . }}
 //	{{- end }}
 func (s *TplStencil) GetModuleHook(name string) []any {
+	// On the first pass, never return any data. If we did, the data would
+	// be unreliably set because we don't sort the templates in any way or
+	// guarantee that they will be rendered in specific any order.
+	if s.s.isFirstPass {
+		return []any{}
+	}
+
 	k := s.s.sharedData.key(s.t.Module.Name, name)
 	v := s.s.sharedData.moduleHooks[k]
 	if v == nil {

internal/codegen/tpl_stencil_test.go

@@ -141,6 +141,12 @@ func TestTplStencil_GetModuleHook(t *testing.T) {
 					return
 				}
 			}
+
+			// Ensure that GetModuleHook never returns anything other than
+			// `[]any` during the first pass.
+			if got := s.GetModuleHook(tt.args.name); !reflect.DeepEqual(got, []any{}) {
+				t.Errorf("TplStencil.GetModuleHook() = %v, want %v", got, []any{})
+			}
 			s.s.isFirstPass = false
 
 			// Sort the module hooks, which should be called by stencil before