Skip to content

Commit

Permalink
Improve error message on wrong type of mg.Deps argument (#240) (#241)
Browse files Browse the repository at this point in the history
This change adds a location of mg.Deps invocation, shrinking the search space
from "all your build scripts" to "this particular mg.Deps invocation"
  • Loading branch information
Mikhail Gusarov authored and natefinch committed Jul 18, 2019
1 parent d30a2cf commit 05f8c9d
Show file tree
Hide file tree
Showing 3 changed files with 51 additions and 1 deletion.
19 changes: 19 additions & 0 deletions mage/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1261,6 +1261,25 @@ func TestAliasToImport(t *testing.T) {

}

var wrongDepRx = regexp.MustCompile("^Error: Invalid type for dependent function.*@ main.FooBar .*magefile.go")

func TestWrongDependency(t *testing.T) {
stderr := &bytes.Buffer{}
inv := Invocation{
Dir: "./testdata/wrong_dep",
Stderr: stderr,
Stdout: ioutil.Discard,
}
code := Invoke(inv)
if code != 1 {
t.Fatalf("expected 1, but got %v", code)
}
actual := stderr.String()
if !wrongDepRx.MatchString(actual) {
t.Fatalf("expected matching %q, but got %q", wrongDepRx, actual)
}
}

/// This code liberally borrowed from https://github.com/rsc/goversion/blob/master/version/exe.go

type exeType int
Expand Down
16 changes: 16 additions & 0 deletions mage/testdata/wrong_dep/magefile.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
//+build mage

package main

import (
"github.com/magefile/mage/mg"
)

var Default = FooBar

func WrongSignature(i int) {
}

func FooBar() {
mg.Deps(WrongSignature)
}
17 changes: 16 additions & 1 deletion mg/deps.go
Original file line number Diff line number Diff line change
Expand Up @@ -214,6 +214,21 @@ func (o *onceFun) run() error {
return o.err
}

// Returns a location of mg.Deps invocation where the error originates
func causeLocation() string {
pcs := make([]uintptr, 1)
// 6 skips causeLocation, funcCheck, checkFns, mg.CtxDeps, mg.Deps in stacktrace
if runtime.Callers(6, pcs) != 1 {
return "<unknown>"
}
frames := runtime.CallersFrames(pcs)
frame, _ := frames.Next()
if frame.Function == "" && frame.File == "" && frame.Line == 0 {
return "<unknown>"
}
return fmt.Sprintf("%s %s:%d", frame.Function, frame.File, frame.Line)
}

// funcCheck tests if a function is one of funcType
func funcCheck(fn interface{}) (funcType, error) {
switch fn.(type) {
Expand All @@ -227,7 +242,7 @@ func funcCheck(fn interface{}) (funcType, error) {
return contextErrorType, nil
}

err := fmt.Errorf("Invalid type for dependent function: %T. Dependencies must be func(), func() error, func(context.Context), func(context.Context) error, or the same method on an mg.Namespace.", fn)
err := fmt.Errorf("Invalid type for dependent function: %T. Dependencies must be func(), func() error, func(context.Context), func(context.Context) error, or the same method on an mg.Namespace @ %s", fn, causeLocation())

// ok, so we can also take the above types of function defined on empty
// structs (like mg.Namespace). When you pass a method of a type, it gets
Expand Down

0 comments on commit 05f8c9d

Please sign in to comment.