Skip to content

Commit

Permalink
Fix nil checks and improve error message
Browse files Browse the repository at this point in the history
The new mg.F function is the root of the nil-pointer error that can happen
when someone accidentally calls mg.Deps(TargetA()). I have fixed the check
for whether a function was passed so that it won't panic.

I then added a similar check, is the target a function, to
Deps/SerialDeps/CxtSerialDeps functions so that we can give a better
error message when it's passed a non-function. The error message
otherwise will reference mg.F which I don't think will help point people
to the real problem, which is how they called mg.Deps/SerialDeps.

Signed-off-by: Carolyn Van Slyck <me@carolynvanslyck.com>
  • Loading branch information
carolynvs committed Jan 10, 2022
1 parent d9c0439 commit e310432
Show file tree
Hide file tree
Showing 3 changed files with 9 additions and 5 deletions.
10 changes: 7 additions & 3 deletions mg/deps.go
Expand Up @@ -130,13 +130,17 @@ func runDeps(ctx context.Context, fns []Fn) {
func checkFns(fns []interface{}) []Fn {
funcs := make([]Fn, len(fns))
for i, f := range fns {
if f == nil {
panic("A dependency of the current target was defined improperly, with parenthesis. Dependencies should be defined as mg.Deps(MyDep), not mg.Deps(MyDep())")
}
if fn, ok := f.(Fn); ok {
funcs[i] = fn
continue
}

// Check if the target provided is a not function so we can give a clear warning
t := reflect.TypeOf(f)
if t == nil || t.Kind() != reflect.Func {
panic(fmt.Errorf("non-function used as a target dependency: %T. The mg.Deps, mg.SerialDeps and mg.CtxDeps functions accept function names, such as mg.Deps(TargetA, TargetB)", f))
}

funcs[i] = F(f)
}
return funcs
Expand Down
2 changes: 1 addition & 1 deletion mg/deps_internal_test.go
Expand Up @@ -45,7 +45,7 @@ func TestDepWasNotInvoked(t *testing.T) {
t.Fatal("expected panic, but didn't get one")
}
gotErr := fmt.Sprint(err)
wantErr := "A dependency of the current target was defined improperly, with parenthesis"
wantErr := "non-function used as a target dependency: <nil>. The mg.Deps, mg.SerialDeps and mg.CtxDeps functions accept function names, such as mg.Deps(TargetA, TargetB)"
if !strings.Contains(gotErr, wantErr) {
t.Fatalf(`expected to get "%s" but got "%s"`, wantErr, gotErr)
}
Expand Down
2 changes: 1 addition & 1 deletion mg/fn.go
Expand Up @@ -100,7 +100,7 @@ func (f fn) Run(ctx context.Context) error {

func checkF(target interface{}, args []interface{}) (hasContext, isNamespace bool, _ error) {
t := reflect.TypeOf(target)
if t.Kind() != reflect.Func {
if t == nil || t.Kind() != reflect.Func {
return false, false, fmt.Errorf("non-function passed to mg.F: %T", target)
}

Expand Down

0 comments on commit e310432

Please sign in to comment.