From 38a06dad8ecf7ffe2f0e0aa08f7a8e003a4d729d Mon Sep 17 00:00:00 2001 From: Pete Davison Date: Thu, 22 Feb 2024 20:52:05 +0000 Subject: [PATCH] feat: error when multiple wildcard matches are found --- docs/docs/usage.md | 4 ++-- errors/errors_task.go | 6 +++--- internal/summary/summary.go | 2 +- task.go | 24 ++++++++++++++++++++---- task_test.go | 29 ++++++++++++++++++++--------- taskfile/ast/tasks.go | 32 ++++++++++++++++++-------------- testdata/wildcards/Taskfile.yml | 4 ++++ 7 files changed, 68 insertions(+), 33 deletions(-) diff --git a/docs/docs/usage.md b/docs/docs/usage.md index b4a377aa..3d386386 100644 --- a/docs/docs/usage.md +++ b/docs/docs/usage.md @@ -1249,8 +1249,8 @@ $ task run-foo-bar foo bar ``` -If multiple matching tasks are found, the first one listed in the Taskfile will -be used. If you are using included Taskfiles, +If multiple matching tasks are found, an error occurs. If you are using included +Taskfiles, tasks in parent files will be considered first. ## Doing task cleanup with `defer` diff --git a/errors/errors_task.go b/errors/errors_task.go index 4263359e..bcbf1bb3 100644 --- a/errors/errors_task.go +++ b/errors/errors_task.go @@ -65,15 +65,15 @@ func (err *TaskInternalError) Code() int { return CodeTaskInternal } -// TaskNameConflictError is returned when multiple tasks with the same name or +// TaskNameConflictError is returned when multiple tasks with a matching name or // alias are found. type TaskNameConflictError struct { - AliasName string + Call string TaskNames []string } func (err *TaskNameConflictError) Error() string { - return fmt.Sprintf(`task: Multiple tasks (%s) with alias %q found`, strings.Join(err.TaskNames, ", "), err.AliasName) + return fmt.Sprintf(`task: Found multiple tasks (%s) that match %q`, strings.Join(err.TaskNames, ", "), err.Call) } func (err *TaskNameConflictError) Code() int { diff --git a/internal/summary/summary.go b/internal/summary/summary.go index edf4d677..a1bcdabe 100644 --- a/internal/summary/summary.go +++ b/internal/summary/summary.go @@ -10,7 +10,7 @@ import ( func PrintTasks(l *logger.Logger, t *ast.Taskfile, c []*ast.Call) { for i, call := range c { PrintSpaceBetweenSummaries(l, i) - PrintTask(l, t.Tasks.Get(call)) + PrintTask(l, t.Tasks.Get(call.Task)) } } diff --git a/task.go b/task.go index 6bc98a8a..885f3ce0 100644 --- a/task.go +++ b/task.go @@ -415,12 +415,28 @@ func (e *Executor) startExecution(ctx context.Context, t *ast.Task, execute func // If multiple tasks contain the same alias or no matches are found an error is returned. func (e *Executor) GetTask(call *ast.Call) (*ast.Task, error) { // Search for a matching task - matchingTask := e.Taskfile.Tasks.Get(call) - if matchingTask != nil { - return matchingTask, nil + matchingTasks := e.Taskfile.Tasks.FindMatchingTasks(call) + switch len(matchingTasks) { + case 0: // Carry on + case 1: + if call.Vars == nil { + call.Vars = &ast.Vars{} + } + call.Vars.Set("MATCH", ast.Var{Value: matchingTasks[0].Wildcards}) + return matchingTasks[0].Task, nil + default: + taskNames := make([]string, len(matchingTasks)) + for i, matchingTask := range matchingTasks { + taskNames[i] = matchingTask.Task.Task + } + return nil, &errors.TaskNameConflictError{ + Call: call.Task, + TaskNames: taskNames, + } } // If didn't find one, search for a task with a matching alias + var matchingTask *ast.Task var aliasedTasks []string for _, task := range e.Taskfile.Tasks.Values() { if slices.Contains(task.Aliases, call.Task) { @@ -431,7 +447,7 @@ func (e *Executor) GetTask(call *ast.Call) (*ast.Task, error) { // If we found multiple tasks if len(aliasedTasks) > 1 { return nil, &errors.TaskNameConflictError{ - AliasName: call.Task, + Call: call.Task, TaskNames: aliasedTasks, } } diff --git a/task_test.go b/task_test.go index 4c1d09a3..8cfa832b 100644 --- a/task_test.go +++ b/task_test.go @@ -2253,33 +2253,44 @@ func TestFor(t *testing.T) { func TestWildcard(t *testing.T) { tests := []struct { name string + call string expectedOutput string wantErr bool }{ { - name: "wildcard-foo", + name: "basic wildcard", + call: "wildcard-foo", expectedOutput: "Hello foo\n", }, { - name: "foo-wildcard-bar", + name: "double wildcard", + call: "foo-wildcard-bar", expectedOutput: "Hello foo bar\n", }, { - name: "start-foo", + name: "store wildcard", + call: "start-foo", expectedOutput: "Starting foo\n", }, { - name: "matches-exactly-*", - expectedOutput: "I don't consume matches: \n", + name: "matches exactly", + call: "matches-exactly-*", + expectedOutput: "I don't consume matches: []\n", }, { - name: "no-match", + name: "no matches", + call: "no-match", + wantErr: true, + }, + { + name: "multiple matches", + call: "wildcard-foo-bar", wantErr: true, }, } for _, test := range tests { - t.Run(test.name, func(t *testing.T) { + t.Run(test.call, func(t *testing.T) { var buff bytes.Buffer e := task.Executor{ Dir: "testdata/wildcards", @@ -2290,10 +2301,10 @@ func TestWildcard(t *testing.T) { } require.NoError(t, e.Setup()) if test.wantErr { - require.Error(t, e.Run(context.Background(), &ast.Call{Task: test.name})) + require.Error(t, e.Run(context.Background(), &ast.Call{Task: test.call})) return } - require.NoError(t, e.Run(context.Background(), &ast.Call{Task: test.name})) + require.NoError(t, e.Run(context.Background(), &ast.Call{Task: test.call})) assert.Equal(t, test.expectedOutput, buff.String()) }) } diff --git a/taskfile/ast/tasks.go b/taskfile/ast/tasks.go index a0d288db..b6c85bb4 100644 --- a/taskfile/ast/tasks.go +++ b/taskfile/ast/tasks.go @@ -14,29 +14,34 @@ type Tasks struct { omap.OrderedMap[string, *Task] } -func (t *Tasks) Get(call *Call) *Task { +type MatchingTask struct { + Task *Task + Wildcards []string +} + +func (t *Tasks) FindMatchingTasks(call *Call) []*MatchingTask { if call == nil { return nil } var task *Task + var matchingTasks []*MatchingTask // If there is a direct match, return it if task = t.OrderedMap.Get(call.Task); task != nil { - return task - } - if call.Vars == nil { - call.Vars = &Vars{} + matchingTasks = append(matchingTasks, &MatchingTask{Task: task, Wildcards: nil}) + return matchingTasks } // Attempt a wildcard match - // TODO: We need to add a yield func to the Range method so that we can stop looping when we find a match // For now, we can just nil check the task before each loop _ = t.Range(func(key string, value *Task) error { - if match, wildcards := value.WildcardMatch(call.Task); match && task == nil { - task = value - call.Vars.Set("MATCH", Var{Value: wildcards}) + if match, wildcards := value.WildcardMatch(call.Task); match { + matchingTasks = append(matchingTasks, &MatchingTask{ + Task: value, + Wildcards: wildcards, + }) } return nil }) - return task + return matchingTasks } func (t1 *Tasks) Merge(t2 Tasks, include *Include) { @@ -86,11 +91,10 @@ func (t1 *Tasks) Merge(t2 Tasks, include *Include) { // run the included Taskfile's default task without specifying its full // name. If the parent namespace has aliases, we add another alias for each // of them. - if t2.Get(&Call{Task: "default"}) != nil && t1.Get(&Call{Task: include.Namespace}) == nil { + if t2.Get("default") != nil && t1.Get(include.Namespace) == nil { defaultTaskName := fmt.Sprintf("%s:default", include.Namespace) - defaultTaskCall := &Call{Task: defaultTaskName} - t1.Get(defaultTaskCall).Aliases = append(t1.Get(defaultTaskCall).Aliases, include.Namespace) - t1.Get(defaultTaskCall).Aliases = append(t1.Get(defaultTaskCall).Aliases, include.Aliases...) + t1.Get(defaultTaskName).Aliases = append(t1.Get(defaultTaskName).Aliases, include.Namespace) + t1.Get(defaultTaskName).Aliases = append(t1.Get(defaultTaskName).Aliases, include.Aliases...) } } diff --git a/testdata/wildcards/Taskfile.yml b/testdata/wildcards/Taskfile.yml index 31d8ec9f..5b0d9765 100644 --- a/testdata/wildcards/Taskfile.yml +++ b/testdata/wildcards/Taskfile.yml @@ -5,6 +5,10 @@ tasks: cmds: - echo "Hello {{index .MATCH 0}}" + wildcard-*-*: + cmds: + - echo "Hello {{index .MATCH 0}}" + '*-wildcard-*': cmds: - echo "Hello {{index .MATCH 0}} {{index .MATCH 1}}"