refactor: improve tmpls that eval to a bool (#3726)

- added a tmpl.Bool that checks if the result of a template is "true"
- added an ErrSkipper interface so Skip() methods can return errors as well

---------

Signed-off-by: Carlos A Becker <caarlos0@users.noreply.github.com>

internal/middleware/skip/skip.go

@@ -16,12 +16,26 @@ type Skipper interface {
 	fmt.Stringer
 }
 
+// Skipper defines a method to skip an entire Piper.
+type ErrSkipper interface {
+	// Skip returns true if the Piper should be skipped.
+	Skip(ctx *context.Context) (bool, error)
+	fmt.Stringer
+}
+
 // Maybe returns an action that skips immediately if the given p is a Skipper
 // and its Skip method returns true.
 func Maybe(skipper interface{}, next middleware.Action) middleware.Action {
 	if skipper, ok := skipper.(Skipper); ok {
+		return Maybe(wrapper{skipper}, next)
+	}
+	if skipper, ok := skipper.(ErrSkipper); ok {
 		return func(ctx *context.Context) error {
-			if skipper.Skip(ctx) {
+			skip, err := skipper.Skip(ctx)
+			if err != nil {
+				return fmt.Errorf("skip %s: %w", skipper.String(), err)
+			}
+			if skip {
 				log.Debugf("skipped %s", skipper.String())
 				return nil
 			}
@@ -30,3 +44,19 @@ func Maybe(skipper interface{}, next middleware.Action) middleware.Action {
 	}
 	return next
 }
+
+var _ ErrSkipper = wrapper{}
+
+type wrapper struct {
+	skipper Skipper
+}
+
+// String implements SkipperErr
+func (w wrapper) String() string {
+	return w.skipper.String()
+}
+
+// Skip implements SkipperErr
+func (w wrapper) Skip(ctx *context.Context) (bool, error) {
+	return w.skipper.Skip(ctx), nil
+}

internal/middleware/skip/skip_test.go

@@ -27,6 +27,29 @@ func TestSkip(t *testing.T) {
 	})
 }
 
+func TestSkipErr(t *testing.T) {
+	fakeErr := fmt.Errorf("fake error")
+	action := func(_ *context.Context) error {
+		return fakeErr
+	}
+
+	t.Run("no err", func(t *testing.T) {
+		require.NoError(t, Maybe(errSkipper{true, nil}, action)(nil))
+	})
+
+	t.Run("with err", func(t *testing.T) {
+		require.EqualError(t, Maybe(
+			errSkipper{false, fmt.Errorf("skip err")},
+			action,
+		)(nil), "skip blah: skip err")
+	})
+}
+
+var (
+	_ Skipper    = skipper{}
+	_ ErrSkipper = errSkipper{}
+)
+
 type skipper struct {
 	skip bool
 }
@@ -36,3 +59,14 @@ func (s skipper) String() string { return "blah" }
 func (s skipper) Skip(_ *context.Context) bool {
 	return s.skip
 }
+
+type errSkipper struct {
+	skip bool
+	err  error
+}
+
+func (s errSkipper) String() string { return "blah" }
+
+func (s errSkipper) Skip(_ *context.Context) (bool, error) {
+	return s.skip, s.err
+}

internal/pipe/announce/announce.go

@@ -4,7 +4,6 @@ package announce
 import (
 	"fmt"
 
-	"github.com/caarlos0/log"
 	"github.com/goreleaser/goreleaser/internal/middleware/errhandler"
 	"github.com/goreleaser/goreleaser/internal/middleware/logging"
 	"github.com/goreleaser/goreleaser/internal/middleware/skip"
@@ -50,20 +49,11 @@ type Pipe struct{}
 
 func (Pipe) String() string { return "announcing" }
 
-func (Pipe) Skip(ctx *context.Context) bool {
+func (Pipe) Skip(ctx *context.Context) (bool, error) {
 	if ctx.SkipAnnounce {
-		return true
+		return true, nil
 	}
-	if ctx.Config.Announce.Skip == "" {
-		return false
-	}
-	skip, err := tmpl.New(ctx).Apply(ctx.Config.Announce.Skip)
-	if err != nil {
-		log.Error("invalid announce.skip template, will skip the announcing step")
-		return true
-	}
-	log.Debugf("announce.skip evaluated from %q to %q", ctx.Config.Announce.Skip, skip)
-	return skip == "true"
+	return tmpl.New(ctx).Bool(ctx.Config.Announce.Skip)
 }
 
 // Run the pipe.

internal/pipe/announce/announce_test.go

@@ -42,7 +42,9 @@ func TestSkip(t *testing.T) {
 	t.Run("skip", func(t *testing.T) {
 		ctx := context.New(config.Project{})
 		ctx.SkipAnnounce = true
-		require.True(t, Pipe{}.Skip(ctx))
+		b, err := Pipe{}.Skip(ctx)
+		require.NoError(t, err)
+		require.True(t, b)
 	})
 
 	t.Run("skip on patches", func(t *testing.T) {
@@ -52,21 +54,26 @@ func TestSkip(t *testing.T) {
 			},
 		})
 		ctx.Semver.Patch = 1
-		require.True(t, Pipe{}.Skip(ctx))
+		b, err := Pipe{}.Skip(ctx)
+		require.NoError(t, err)
+		require.True(t, b)
 	})
 
-	t.Run("skip on invalid template", func(t *testing.T) {
+	t.Run("invalid template", func(t *testing.T) {
 		ctx := context.New(config.Project{
 			Announce: config.Announce{
 				Skip: "{{if eq .Patch 123}",
 			},
 		})
 		ctx.Semver.Patch = 1
-		require.True(t, Pipe{}.Skip(ctx))
+		_, err := Pipe{}.Skip(ctx)
+		require.Error(t, err)
 	})
 
 	t.Run("dont skip", func(t *testing.T) {
-		require.False(t, Pipe{}.Skip(context.New(config.Project{})))
+		b, err := Pipe{}.Skip(context.New(config.Project{}))
+		require.NoError(t, err)
+		require.False(t, b)
 	})
 
 	t.Run("dont skip based on template", func(t *testing.T) {
@@ -76,6 +83,8 @@ func TestSkip(t *testing.T) {
 			},
 		})
 		ctx.Semver.Patch = 0
-		require.False(t, Pipe{}.Skip(ctx))
+		b, err := Pipe{}.Skip(ctx)
+		require.NoError(t, err)
+		require.False(t, b)
 	})
 }

internal/pipe/env/env.go

@@ -114,19 +114,13 @@ func (Pipe) Run(ctx *context.Context) error {
 	return nil
 }
 
-func isSkipRelease(ctx *context.Context) bool {
-	d, err := tmpl.New(ctx).Apply(ctx.Config.Release.Disable)
-	if err != nil {
-		log.WithError(err).Error("could not execute release.disable template, will assume false")
-		return false
-	}
-	return strings.ToLower(d) == "true"
-}
-
 func checkErrors(ctx *context.Context, noTokens, noTokenErrs bool, gitlabTokenErr, githubTokenErr, giteaTokenErr error) error {
-	if ctx.SkipTokenCheck || ctx.SkipPublish || isSkipRelease(ctx) {
+	if ctx.SkipTokenCheck || ctx.SkipPublish {
 		return nil
 	}
+	if b, err := tmpl.New(ctx).Bool(ctx.Config.Release.Disable); err != nil || b {
+		return err
+	}
 
 	if noTokens && noTokenErrs {
 		return ErrMissingToken

internal/pipe/env/env_test.go

@@ -5,6 +5,7 @@ import (
 	"os"
 	"testing"
 
+	"github.com/goreleaser/goreleaser/internal/testlib"
 	"github.com/goreleaser/goreleaser/pkg/config"
 	"github.com/goreleaser/goreleaser/pkg/context"
 	"github.com/stretchr/testify/require"
@@ -247,7 +248,7 @@ func TestInvalidEnvReleaseDisabled(t *testing.T) {
 				Disable: "{{ .Env.FOO }}",
 			},
 		})
-		require.EqualError(t, Pipe{}.Run(ctx), ErrMissingToken.Error())
+		testlib.RequireTemplateError(t, Pipe{}.Run(ctx))
 	})
 }
 

internal/pipe/release/release.go

@@ -5,7 +5,6 @@ import (
 	"fmt"
 	"io/fs"
 	"os"
-	"strings"
 	"time"
 
 	"github.com/caarlos0/log"
@@ -28,13 +27,9 @@ var ErrMultipleReleases = errors.New("multiple releases are defined. Only one is
 type Pipe struct{}
 
 func (Pipe) String() string { return "scm releases" }
-func (Pipe) Skip(ctx *context.Context) bool {
-	d, err := tmpl.New(ctx).Apply(ctx.Config.Release.Disable)
-	if err != nil {
-		log.WithError(err).Error("could not execute release.disable template, will assume false")
-		return false
-	}
-	return strings.ToLower(d) == "true"
+
+func (Pipe) Skip(ctx *context.Context) (bool, error) {
+	return tmpl.New(ctx).Bool(ctx.Config.Release.Disable)
 }
 
 // Default sets the pipe defaults.
@@ -125,11 +120,11 @@ func doPublish(ctx *context.Context, client client.Client) error {
 		return err
 	}
 
-	d, err := tmpl.New(ctx).Apply(ctx.Config.Release.SkipUpload)
+	skipUpload, err := tmpl.New(ctx).Bool(ctx.Config.Release.SkipUpload)
 	if err != nil {
 		return err
 	}
-	if strings.ToLower(d) == "true" {
+	if skipUpload {
 		return pipe.Skip("release.skip_upload is set")
 	}
 

internal/pipe/release/release_test.go

@@ -609,7 +609,9 @@ func TestSkip(t *testing.T) {
 				Disable: "true",
 			},
 		})
-		require.True(t, Pipe{}.Skip(ctx))
+		b, err := Pipe{}.Skip(ctx)
+		require.NoError(t, err)
+		require.True(t, b)
 	})
 
 	t.Run("skip tmpl", func(t *testing.T) {
@@ -619,16 +621,19 @@ func TestSkip(t *testing.T) {
 				Disable: "{{ .Env.FOO }}",
 			},
 		})
-		require.True(t, Pipe{}.Skip(ctx))
+		b, err := Pipe{}.Skip(ctx)
+		require.NoError(t, err)
+		require.True(t, b)
 	})
 
-	t.Run("skip tmpl err", func(t *testing.T) {
+	t.Run("tmpl err", func(t *testing.T) {
 		ctx := context.New(config.Project{
 			Release: config.Release{
 				Disable: "{{ .Env.FOO }}",
 			},
 		})
-		require.False(t, Pipe{}.Skip(ctx))
+		_, err := Pipe{}.Skip(ctx)
+		require.Error(t, err)
 	})
 
 	t.Run("skip upload", func(t *testing.T) {
@@ -657,6 +662,8 @@ func TestSkip(t *testing.T) {
 	})
 
 	t.Run("dont skip", func(t *testing.T) {
-		require.False(t, Pipe{}.Skip(context.New(config.Project{})))
+		b, err := Pipe{}.Skip(context.New(config.Project{}))
+		require.NoError(t, err)
+		require.False(t, b)
 	})
 }

internal/pipe/scoop/scoop.go

@@ -142,11 +142,12 @@ func doPublish(ctx *context.Context, cl client.Client) error {
 	if ctx.Config.Release.Draft {
 		return pipe.Skip("release is marked as draft")
 	}
-	d, err := tmpl.New(ctx).Apply(ctx.Config.Release.Disable)
+
+	relDisabled, err := tmpl.New(ctx).Bool(ctx.Config.Release.Disable)
 	if err != nil {
 		return err
 	}
-	if strings.ToLower(d) == "true" {
+	if relDisabled {
 		return pipe.Skip("release is disabled")
 	}
 

internal/tmpl/tmpl.go

@@ -190,6 +190,12 @@ func buildOptsToFields(opts build.Options) Fields {
 	}
 }
 
+// Bool Apply the given string, and converts it to a bool.
+func (t *Template) Bool(s string) (bool, error) {
+	r, err := t.Apply(s)
+	return strings.TrimSpace(strings.ToLower(r)) == "true", err
+}
+
 // Apply applies the given string against the Fields stored in the template.
 func (t *Template) Apply(s string) (string, error) {
 	var out bytes.Buffer

internal/tmpl/tmpl_test.go

@@ -360,3 +360,39 @@ func TestWithExtraFields(t *testing.T) {
 	}).Apply("{{ .MyCustomField }}")
 	require.Equal(t, "foo", out)
 }
+
+func TestBool(t *testing.T) {
+	t.Run("true", func(t *testing.T) {
+		for _, v := range []string{
+			" TruE   ",
+			"true",
+			"TRUE",
+		} {
+			t.Run(v, func(t *testing.T) {
+				ctx := context.New(config.Project{
+					Env: []string{"FOO=" + v},
+				})
+				b, err := New(ctx).Bool("{{.Env.FOO}}")
+				require.NoError(t, err)
+				require.True(t, b)
+			})
+		}
+	})
+	t.Run("false", func(t *testing.T) {
+		for _, v := range []string{
+			"    ",
+			"",
+			"false",
+			"yada yada",
+		} {
+			t.Run(v, func(t *testing.T) {
+				ctx := context.New(config.Project{
+					Env: []string{"FOO=" + v},
+				})
+				b, err := New(ctx).Bool("{{.Env.FOO}}")
+				require.NoError(t, err)
+				require.False(t, b)
+			})
+		}
+	})
+}