refactor: command hooks as interceptors (#104)

cmd/aspect/build/BUILD.bazel

@@ -6,9 +6,7 @@ go_library(
     importpath = "aspect.build/cli/cmd/aspect/build",
     visibility = ["//cmd/aspect/root:__pkg__"],
     deps = [
-        "//cmd/aspect/root/flags",
         "//pkg/aspect/build",
-        "//pkg/aspecterrors",
         "//pkg/bazel",
         "//pkg/interceptors",
         "//pkg/ioutils",

cmd/aspect/build/build.go

@@ -8,14 +8,10 @@ package build
 
 import (
 	"context"
-	"errors"
-	"fmt"
 
 	"github.com/spf13/cobra"
 
-	rootFlags "aspect.build/cli/cmd/aspect/root/flags"
 	"aspect.build/cli/pkg/aspect/build"
-	"aspect.build/cli/pkg/aspecterrors"
 	"aspect.build/cli/pkg/bazel"
 	"aspect.build/cli/pkg/interceptors"
 	"aspect.build/cli/pkg/ioutils"
@@ -48,27 +44,9 @@ func NewBuildCmd(
 			[]interceptors.Interceptor{
 				interceptors.WorkspaceRootInterceptor(),
 				pluginSystem.BESBackendInterceptor(),
+				pluginSystem.BuildHooksInterceptor(streams),
 			},
 			func(ctx context.Context, cmd *cobra.Command, args []string) (exitErr error) {
-				isInteractiveMode, err := cmd.Root().PersistentFlags().GetBool(rootFlags.InteractiveFlagName)
-				if err != nil {
-					return err
-				}
-
-				// TODO(f0rmiga): test this post-build hook.
-				defer func() {
-					errs := pluginSystem.ExecutePostBuild(isInteractiveMode).Errors()
-					if len(errs) > 0 {
-						for _, err := range errs {
-							fmt.Fprintf(streams.Stderr, "Error: failed to run build command: %v\n", err)
-						}
-						var err *aspecterrors.ExitError
-						if errors.As(exitErr, &err) {
-							err.ExitCode = 1
-						}
-					}
-				}()
-
 				workspaceRoot := ctx.Value(interceptors.WorkspaceRootKey).(string)
 				bzl.SetWorkspaceRoot(workspaceRoot)
 				b := build.New(streams, bzl)

cmd/aspect/root/BUILD.bazel

@@ -13,11 +13,11 @@ go_library(
         "//cmd/aspect/clean",
         "//cmd/aspect/docs",
         "//cmd/aspect/info",
-        "//cmd/aspect/root/flags",
         "//cmd/aspect/run",
         "//cmd/aspect/test",
         "//cmd/aspect/version",
         "//docs/help/topics",
+        "//pkg/aspect/root/flags",
         "//pkg/ioutils",
         "//pkg/plugin/system",
         "@com_github_fatih_color//:color",

cmd/aspect/root/root.go

@@ -19,11 +19,11 @@ import (
 	"aspect.build/cli/cmd/aspect/clean"
 	"aspect.build/cli/cmd/aspect/docs"
 	"aspect.build/cli/cmd/aspect/info"
-	"aspect.build/cli/cmd/aspect/root/flags"
 	"aspect.build/cli/cmd/aspect/run"
 	"aspect.build/cli/cmd/aspect/test"
 	"aspect.build/cli/cmd/aspect/version"
 	"aspect.build/cli/docs/help/topics"
+	"aspect.build/cli/pkg/aspect/root/flags"
 	"aspect.build/cli/pkg/ioutils"
 	"aspect.build/cli/pkg/plugin/system"
 )

cmd/aspect/test/BUILD.bazel

@@ -6,9 +6,7 @@ go_library(
     importpath = "aspect.build/cli/cmd/aspect/test",
     visibility = ["//visibility:public"],
     deps = [
-        "//cmd/aspect/root/flags",
         "//pkg/aspect/test",
-        "//pkg/aspecterrors",
         "//pkg/bazel",
         "//pkg/interceptors",
         "//pkg/ioutils",

cmd/aspect/test/test.go

@@ -8,14 +8,10 @@ package test
 
 import (
 	"context"
-	"errors"
-	"fmt"
 
 	"github.com/spf13/cobra"
 
-	rootFlags "aspect.build/cli/cmd/aspect/root/flags"
 	"aspect.build/cli/pkg/aspect/test"
-	"aspect.build/cli/pkg/aspecterrors"
 	"aspect.build/cli/pkg/bazel"
 	"aspect.build/cli/pkg/interceptors"
 	"aspect.build/cli/pkg/ioutils"
@@ -56,26 +52,9 @@ specify targets.
 			[]interceptors.Interceptor{
 				interceptors.WorkspaceRootInterceptor(),
 				pluginSystem.BESBackendInterceptor(),
+				pluginSystem.TestHooksInterceptor(streams),
 			},
 			func(ctx context.Context, cmd *cobra.Command, args []string) (exitErr error) {
-				isInteractiveMode, err := cmd.Root().PersistentFlags().GetBool(rootFlags.InteractiveFlagName)
-				if err != nil {
-					return err
-				}
-
-				defer func() {
-					errs := pluginSystem.ExecutePostTest(isInteractiveMode).Errors()
-					if len(errs) > 0 {
-						for _, err := range errs {
-							fmt.Fprintf(streams.Stderr, "Error: failed to run test command: %v\n", err)
-						}
-						var err *aspecterrors.ExitError
-						if errors.As(exitErr, &err) {
-							err.ExitCode = 1
-						}
-					}
-				}()
-
 				workspaceRoot := ctx.Value(interceptors.WorkspaceRootKey).(string)
 				bzl.SetWorkspaceRoot(workspaceRoot)
 				t := test.New(streams, bzl)

cmd/aspect/root/flags/BUILD.bazel → pkg/aspect/root/flags/BUILD.bazel

@@ -3,6 +3,6 @@ load("@io_bazel_rules_go//go:def.bzl", "go_library")
 go_library(
     name = "flags",
     srcs = ["config.go"],
-    importpath = "aspect.build/cli/cmd/aspect/root/flags",
-    visibility = ["//visibility:public"],
+    importpath = "aspect.build/cli/pkg/aspect/root/flags",
+    visibility = ["//:__subpackages__"],
 )

pkg/plugin/system/BUILD.bazel

@@ -9,6 +9,7 @@ go_library(
     importpath = "aspect.build/cli/pkg/plugin/system",
     visibility = ["//visibility:public"],
     deps = [
+        "//pkg/aspect/root/flags",
         "//pkg/aspecterrors",
         "//pkg/interceptors",
         "//pkg/ioutils",

pkg/plugin/system/system.go

@@ -8,14 +8,17 @@ package system
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"os/exec"
+	"reflect"
 	"time"
 
 	hclog "github.com/hashicorp/go-hclog"
 	goplugin "github.com/hashicorp/go-plugin"
 	"github.com/spf13/cobra"
 
+	rootFlags "aspect.build/cli/pkg/aspect/root/flags"
 	"aspect.build/cli/pkg/aspecterrors"
 	"aspect.build/cli/pkg/interceptors"
 	"aspect.build/cli/pkg/ioutils"
@@ -30,8 +33,8 @@ type PluginSystem interface {
 	Configure(streams ioutils.Streams) error
 	TearDown()
 	BESBackendInterceptor() interceptors.Interceptor
-	ExecutePostBuild(isInteractiveMode bool) *aspecterrors.ErrorList
-	ExecutePostTest(isInteractiveMode bool) *aspecterrors.ErrorList
+	BuildHooksInterceptor(streams ioutils.Streams) interceptors.Interceptor
+	TestHooksInterceptor(streams ioutils.Streams) interceptors.Interceptor
 }
 
 type pluginSystem struct {
@@ -145,26 +148,47 @@ func (ps *pluginSystem) BESBackendInterceptor() interceptors.Interceptor {
 	}
 }
 
-// ExecutePostBuild executes all post-build hooks from all plugins.
-func (ps *pluginSystem) ExecutePostBuild(isInteractiveMode bool) *aspecterrors.ErrorList {
-	errors := &aspecterrors.ErrorList{}
-	for node := ps.plugins.head; node != nil; node = node.next {
-		if err := node.plugin.PostBuildHook(isInteractiveMode, ps.promptRunner); err != nil {
-			errors.Insert(err)
-		}
-	}
-	return errors
+// BuildHooksInterceptor returns an interceptor that runs the pre and post-build
+// hooks from all plugins.
+func (ps *pluginSystem) BuildHooksInterceptor(streams ioutils.Streams) interceptors.Interceptor {
+	return ps.commandHooksInterceptor("PostBuildHook", streams)
+}
+
+// TestHooksInterceptor returns an interceptor that runs the pre and post-test
+// hooks from all plugins.
+func (ps *pluginSystem) TestHooksInterceptor(streams ioutils.Streams) interceptors.Interceptor {
+	return ps.commandHooksInterceptor("PostTestHook", streams)
 }
 
-// ExecutePostTest executes all post-build hooks from all plugins.
-func (ps *pluginSystem) ExecutePostTest(isInteractiveMode bool) *aspecterrors.ErrorList {
-	errors := &aspecterrors.ErrorList{}
-	for node := ps.plugins.head; node != nil; node = node.next {
-		if err := node.plugin.PostTestHook(isInteractiveMode, ps.promptRunner); err != nil {
-			errors.Insert(err)
+func (ps *pluginSystem) commandHooksInterceptor(methodName string, streams ioutils.Streams) interceptors.Interceptor {
+	return func(ctx context.Context, cmd *cobra.Command, args []string, next interceptors.RunEContextFn) (exitErr error) {
+		isInteractiveMode, err := cmd.Root().PersistentFlags().GetBool(rootFlags.InteractiveFlagName)
+		if err != nil {
+			return fmt.Errorf("failed to run 'aspect %s' command: %w", cmd.Use, err)
 		}
+
+		// TODO(f0rmiga): test this hook.
+		defer func() {
+			hasErrors := false
+			for node := ps.plugins.head; node != nil; node = node.next {
+				params := []reflect.Value{
+					reflect.ValueOf(isInteractiveMode),
+					reflect.ValueOf(ps.promptRunner),
+				}
+				if err := reflect.ValueOf(node.plugin).MethodByName(methodName).Call(params)[0].Interface(); err != nil {
+					fmt.Fprintf(streams.Stderr, "Error: failed to run 'aspect %s' command: %v\n", cmd.Use, err)
+					hasErrors = true
+				}
+			}
+			if hasErrors {
+				var err *aspecterrors.ExitError
+				if errors.As(exitErr, &err) {
+					err.ExitCode = 1
+				}
+			}
+		}()
+		return next(ctx, cmd, args)
 	}
-	return errors
 }
 
 // ClientFactory hides the call to goplugin.NewClient.