fix: Use better function / variable names, use cmd.Use where possible…

…, improve comments, implement leftover TODO
aspect-build · Feb 1, 2022 · b2ca9f4 · b2ca9f4
1 parent f204944
commit b2ca9f4
Show file tree

Hide file tree

Showing 7 changed files with 43 additions and 35 deletions.
diff --git a/pkg/aspect/aquery/aquery.go b/pkg/aspect/aquery/aquery.go
@@ -28,7 +28,7 @@ type AQuery struct {
 }
 
 func New(streams ioutils.Streams, bzl bazel.Bazel, isInteractive bool) *AQuery {
-	presets := shared.GetPrecannedQueries("aquery")
+	presets := shared.PrecannedQueries("aquery")
 
 	return &AQuery{
 		Streams:       streams,
@@ -42,13 +42,12 @@ func New(streams ioutils.Streams, bzl bazel.Bazel, isInteractive bool) *AQuery {
 }
 
 func (q *AQuery) Run(cmd *cobra.Command, args []string) error {
-	verb := "aquery"
 	presets, presetNames, err := shared.ProcessQueries(q.Presets)
 	if err != nil {
 		return shared.GetPrettyError(cmd, err)
 	}
 
-	verb, query, runReplacements, err := shared.SelectQuery(verb, presets, q.Presets, presetNames, q.Streams, args, q.Select)
+	presetVerb, query, runReplacements, err := shared.SelectQuery(cmd.Use, presets, q.Presets, presetNames, q.Streams, args, q.Select)
 
 	if err != nil {
 		return shared.GetPrettyError(cmd, err)
@@ -62,5 +61,5 @@ func (q *AQuery) Run(cmd *cobra.Command, args []string) error {
 		}
 	}
 
-	return shared.RunQuery(q.Bzl, verb, query)
+	return shared.RunQuery(q.Bzl, presetVerb, query)
 }
diff --git a/pkg/aspect/aquery/aquery_test.go b/pkg/aspect/aquery/aquery_test.go
@@ -46,7 +46,8 @@ func TestQuery(t *testing.T) {
 			},
 		}
 
-		g.Expect(q.Run(nil, []string{"why", "//cmd/aspect/query:query", "@com_github_bazelbuild_bazelisk//core:go_default_library"})).Should(Succeed())
+		cmd := &cobra.Command{Use: "aquery"}
+		g.Expect(q.Run(cmd, []string{"why", "//cmd/aspect/query:query", "@com_github_bazelbuild_bazelisk//core:go_default_library"})).Should(Succeed())
 	})
 
 	t.Run("query can be selected by default and will prompt for inputs", func(t *testing.T) {
@@ -93,7 +94,8 @@ func TestQuery(t *testing.T) {
 				Verb:        "aquery",
 			},
 		}
-		err := q.Run(nil, []string{"why"})
+		cmd := &cobra.Command{Use: "aquery"}
+		err := q.Run(cmd, []string{"why"})
 		g.Expect(err).To(BeNil())
 	})
 
@@ -109,8 +111,6 @@ func TestQuery(t *testing.T) {
 
 		spawner := bazel_mock.NewMockBazel(ctrl)
 
-		cmd := &cobra.Command{Use: "fake"}
-
 		promptRunner := query_mock.NewMockPromptRunner(ctrl)
 		gomock.InOrder(
 			promptRunner.
@@ -141,6 +141,7 @@ func TestQuery(t *testing.T) {
 				Verb:        "aquery",
 			},
 		}
+		cmd := &cobra.Command{Use: "aquery"}
 		err := q.Run(cmd, []string{"why"})
 		g.Expect(err).To(MatchError(expectedError))
 	})
@@ -212,7 +213,8 @@ func TestQuery(t *testing.T) {
 				Verb:        "aquery",
 			},
 		}
-		err := q.Run(nil, []string{})
+		cmd := &cobra.Command{Use: "aquery"}
+		err := q.Run(cmd, []string{})
 		g.Expect(err).To(BeNil())
 	})
 }
diff --git a/pkg/aspect/cquery/cquery.go b/pkg/aspect/cquery/cquery.go
@@ -28,7 +28,7 @@ type CQuery struct {
 }
 
 func New(streams ioutils.Streams, bzl bazel.Bazel, isInteractive bool) *CQuery {
-	presets := shared.GetPrecannedQueries("cquery")
+	presets := shared.PrecannedQueries("cquery")
 
 	return &CQuery{
 		Streams:       streams,
@@ -42,13 +42,12 @@ func New(streams ioutils.Streams, bzl bazel.Bazel, isInteractive bool) *CQuery {
 }
 
 func (q *CQuery) Run(cmd *cobra.Command, args []string) error {
-	verb := "cquery"
 	presets, presetNames, err := shared.ProcessQueries(q.Presets)
 	if err != nil {
 		return shared.GetPrettyError(cmd, err)
 	}
 
-	verb, query, runReplacements, err := shared.SelectQuery(verb, presets, q.Presets, presetNames, q.Streams, args, q.Select)
+	presetVerb, query, runReplacements, err := shared.SelectQuery(cmd.Use, presets, q.Presets, presetNames, q.Streams, args, q.Select)
 
 	if err != nil {
 		return shared.GetPrettyError(cmd, err)
@@ -62,5 +61,5 @@ func (q *CQuery) Run(cmd *cobra.Command, args []string) error {
 		}
 	}
 
-	return shared.RunQuery(q.Bzl, verb, query)
+	return shared.RunQuery(q.Bzl, presetVerb, query)
 }
diff --git a/pkg/aspect/cquery/cquery_test.go b/pkg/aspect/cquery/cquery_test.go
@@ -46,7 +46,8 @@ func TestQuery(t *testing.T) {
 			},
 		}
 
-		g.Expect(q.Run(nil, []string{"why", "//cmd/aspect/query:query", "@com_github_bazelbuild_bazelisk//core:go_default_library"})).Should(Succeed())
+		cmd := &cobra.Command{Use: "cquery"}
+		g.Expect(q.Run(cmd, []string{"why", "//cmd/aspect/query:query", "@com_github_bazelbuild_bazelisk//core:go_default_library"})).Should(Succeed())
 	})
 
 	t.Run("query can be selected by default and will prompt for inputs", func(t *testing.T) {
@@ -93,7 +94,8 @@ func TestQuery(t *testing.T) {
 				Verb:        "cquery",
 			},
 		}
-		err := q.Run(nil, []string{"why"})
+		cmd := &cobra.Command{Use: "cquery"}
+		err := q.Run(cmd, []string{"why"})
 		g.Expect(err).To(BeNil())
 	})
 
@@ -109,8 +111,6 @@ func TestQuery(t *testing.T) {
 
 		spawner := bazel_mock.NewMockBazel(ctrl)
 
-		cmd := &cobra.Command{Use: "fake"}
-
 		promptRunner := query_mock.NewMockPromptRunner(ctrl)
 		gomock.InOrder(
 			promptRunner.
@@ -141,6 +141,7 @@ func TestQuery(t *testing.T) {
 				Verb:        "cquery",
 			},
 		}
+		cmd := &cobra.Command{Use: "cquery"}
 		err := q.Run(cmd, []string{"why"})
 		g.Expect(err).To(MatchError(expectedError))
 	})
@@ -212,7 +213,8 @@ func TestQuery(t *testing.T) {
 				Verb:        "cquery",
 			},
 		}
-		err := q.Run(nil, []string{})
+		cmd := &cobra.Command{Use: "cquery"}
+		err := q.Run(cmd, []string{})
 		g.Expect(err).To(BeNil())
 	})
 }
diff --git a/pkg/aspect/query/query.go b/pkg/aspect/query/query.go
@@ -39,8 +39,10 @@ type Query struct {
 }
 
 func New(streams ioutils.Streams, bzl bazel.Bazel, isInteractive bool) *Query {
-	// will potentially be updated during Run() if the user requests that query also show aquery and cquery predefined queries
-	presets := shared.GetPrecannedQueries("query")
+	// the list of available preset queries will potentially be updated during the "Run" function.
+	// if the user requests that query also show aquery and cquery predefined queries then these
+	// will be added to the list of presets
+	presets := shared.PrecannedQueries("query")
 
 	return &Query{
 		Streams:       streams,
@@ -73,22 +75,22 @@ func (q *Query) Run(cmd *cobra.Command, args []string) error {
 		return err
 	}
 
-	verb := "query"
+	verb := cmd.Use
 
 	if q.Prefs.GetBool(useCQuery) {
 		verb = "cquery"
 	}
 
 	if q.Prefs.GetBool(allowAllQueries) {
-		q.Presets = shared.GetPrecannedQueries("")
+		q.Presets = shared.PrecannedQueries("")
 	}
 
 	presets, presetNames, err := shared.ProcessQueries(q.Presets)
 	if err != nil {
 		return shared.GetPrettyError(cmd, err)
 	}
 
-	verb, query, runReplacements, err := shared.SelectQuery(verb, presets, q.Presets, presetNames, q.Streams, args, q.Select)
+	presetVerb, query, runReplacements, err := shared.SelectQuery(verb, presets, q.Presets, presetNames, q.Streams, args, q.Select)
 
 	if err != nil {
 		return shared.GetPrettyError(cmd, err)
@@ -102,17 +104,17 @@ func (q *Query) Run(cmd *cobra.Command, args []string) error {
 		}
 	}
 
-	return shared.RunQuery(q.Bzl, verb, query)
+	return shared.RunQuery(q.Bzl, presetVerb, query)
 }
 
 func (q *Query) checkConfig(baseUseKey string, baseInquiredKey string, question string) error {
 	if !q.Prefs.GetBool(baseInquiredKey) {
 		q.Prefs.Set(baseInquiredKey, true)
 
-		// if user types in y or Y then err will be nil. Any other input will not result in nil
-		_, someErr := q.Confirmation(question).Run()
+		// Y = no error; N = error
+		_, err := q.Confirmation(question).Run()
 
-		q.Prefs.Set(baseUseKey, someErr == nil)
+		q.Prefs.Set(baseUseKey, err == nil)
 
 		if err := q.Prefs.WriteConfig(); err != nil {
 			return fmt.Errorf("failed to update config file: %w", err)

diff --git a/pkg/aspect/query/query_test.go b/pkg/aspect/query/query_test.go
@@ -55,7 +55,8 @@ func TestQuery(t *testing.T) {
 		viper.SetConfigFile(cfg.Name())
 		q.Prefs = viper
 
-		g.Expect(q.Run(nil, []string{"why", "//cmd/aspect/query:query", "@com_github_bazelbuild_bazelisk//core:go_default_library"})).Should(Succeed())
+		cmd := &cobra.Command{Use: "query"}
+		g.Expect(q.Run(cmd, []string{"why", "//cmd/aspect/query:query", "@com_github_bazelbuild_bazelisk//core:go_default_library"})).Should(Succeed())
 	})
 
 	t.Run("query can be selected by default and will prompt for inputs", func(t *testing.T) {
@@ -126,7 +127,8 @@ func TestQuery(t *testing.T) {
 
 		viper.SetConfigFile(cfg.Name())
 		q.Prefs = viper
-		err = q.Run(nil, []string{"why"})
+		cmd := &cobra.Command{Use: "query"}
+		err = q.Run(cmd, []string{"why"})
 		g.Expect(err).To(BeNil())
 	})
 
@@ -142,8 +144,6 @@ func TestQuery(t *testing.T) {
 
 		spawner := bazel_mock.NewMockBazel(ctrl)
 
-		cmd := &cobra.Command{Use: "fake"}
-
 		promptRunner := query_mock.NewMockPromptRunner(ctrl)
 		gomock.InOrder(
 			promptRunner.
@@ -199,6 +199,7 @@ func TestQuery(t *testing.T) {
 		viper.SetConfigFile(cfg.Name())
 		q.Prefs = viper
 
+		cmd := &cobra.Command{Use: "query"}
 		err = q.Run(cmd, []string{"why"})
 		g.Expect(err).To(MatchError(expectedError))
 	})
@@ -257,7 +258,6 @@ func TestQuery(t *testing.T) {
 			Bzl:           spawner,
 			IsInteractive: true,
 			Prompt: func(label string) shared.PromptRunner {
-				fmt.Println("label:", label)
 				g.Expect(strings.Contains(label, "targettwo") || strings.Contains(label, "dependencytwo")).To(Equal(true))
 				return promptRunner
 			},
@@ -295,7 +295,8 @@ func TestQuery(t *testing.T) {
 
 		viper.SetConfigFile(cfg.Name())
 		q.Prefs = viper
-		err = q.Run(nil, []string{})
+		cmd := &cobra.Command{Use: "query"}
+		err = q.Run(cmd, []string{})
 		g.Expect(err).To(BeNil())
 	})
 }
diff --git a/pkg/aspect/query/shared/query.go b/pkg/aspect/query/shared/query.go
@@ -23,6 +23,8 @@ import (
 	"aspect.build/cli/pkg/ioutils"
 )
 
+var placeholderRegex = regexp.MustCompile(`(\?[a-zA-Z]*)`)
+
 type PresetQuery struct {
 	Name        string
 	Description string
@@ -62,7 +64,7 @@ func Select(presetNames []string) SelectRunner {
 	}
 }
 
-func GetPrecannedQueries(verb string) []*PresetQuery {
+func PrecannedQueries(verb string) []*PresetQuery {
 	// TODO: Queries should be loadable from the plugin config
 	// https://github.com/aspect-build/aspect-cli/issues/98
 	presets := []*PresetQuery{
@@ -149,10 +151,11 @@ func RunQuery(bzl bazel.Bazel, verb string, query string) error {
 }
 
 func ReplacePlaceholders(query string, args []string, p func(label string) PromptRunner) (string, error) {
-	placeholders := regexp.MustCompile(`(\?[a-zA-Z]*)`).FindAllString(query, -1)
+	placeholders := placeholderRegex.FindAllString(query, -1)
 
 	if len(placeholders) == len(args)-1 {
 		for i, placeholder := range placeholders {
+			fmt.Printf("%s set to %s\n", strings.Replace(placeholder, "?", "", 1), args[i+1])
 			// todo.... Print out targetA was set to //foo and targetB was set to //bar
 			query = strings.ReplaceAll(query, placeholder, args[i+1])
 		}