diff --git a/internal/wire/analyze.go b/internal/wire/analyze.go index 0b823aa..e0e69f8 100644 --- a/internal/wire/analyze.go +++ b/internal/wire/analyze.go @@ -20,6 +20,7 @@ import ( "go/ast" "go/token" "go/types" + "sort" "strings" "golang.org/x/tools/go/types/typeutil" @@ -381,7 +382,10 @@ func verifyAcyclic(providerMap *typeutil.Map, hasher typeutil.Hasher) []error { visited := new(typeutil.Map) // to bool visited.SetHasher(hasher) ec := new(errorCollector) - for _, root := range providerMap.Keys() { + // Sort output types so that errors about cycles are consistent. + outputs := providerMap.Keys() + sort.Slice(outputs, func(i, j int) bool { return types.TypeString(outputs[i], nil) < types.TypeString(outputs[j], nil) }) + for _, root := range outputs { // Depth-first search using a stack of trails through the provider map. stk := [][]types.Type{{root}} for len(stk) > 0 { diff --git a/internal/wire/testdata/Cycle/want/wire_errs.txt b/internal/wire/testdata/Cycle/want/wire_errs.txt index 88b2d48..1eb8821 100644 --- a/internal/wire/testdata/Cycle/want/wire_errs.txt +++ b/internal/wire/testdata/Cycle/want/wire_errs.txt @@ -1 +1,5 @@ -cycle +/wire_gopath/src/example.com/foo/wire.go:x:y: cycle for example.com/foo.Bar: +example.com/foo.Bar (example.com/foo.provideBar) -> +example.com/foo.Foo (example.com/foo.provideFoo) -> +example.com/foo.Baz (example.com/foo.provideBaz) -> +example.com/foo.Bar diff --git a/internal/wire/testdata/EmptyVar/want/wire_errs.txt b/internal/wire/testdata/EmptyVar/want/wire_errs.txt index c586bfe..c197b4e 100644 --- a/internal/wire/testdata/EmptyVar/want/wire_errs.txt +++ b/internal/wire/testdata/EmptyVar/want/wire_errs.txt @@ -1 +1 @@ -not a provider +/wire_gopath/src/example.com/foo/wire.go:x:y: var example.com/foo.myFakeSet struct{} is not a provider or a provider set \ No newline at end of file diff --git a/internal/wire/testdata/InjectInputConflict/want/wire_errs.txt b/internal/wire/testdata/InjectInputConflict/want/wire_errs.txt index e2d450e..1bcae60 100644 --- a/internal/wire/testdata/InjectInputConflict/want/wire_errs.txt +++ b/internal/wire/testdata/InjectInputConflict/want/wire_errs.txt @@ -1,2 +1 @@ -foo.Foo -conflicts with provider +/wire_gopath/src/example.com/foo/wire.go:x:y: inject injectBar: input of example.com/foo.Foo conflicts with provider provideFoo at /wire_gopath/src/example.com/foo/foo.go:x:y \ No newline at end of file diff --git a/internal/wire/testdata/InjectorMissingCleanup/want/wire_errs.txt b/internal/wire/testdata/InjectorMissingCleanup/want/wire_errs.txt index 336ff3c..8919b7d 100644 --- a/internal/wire/testdata/InjectorMissingCleanup/want/wire_errs.txt +++ b/internal/wire/testdata/InjectorMissingCleanup/want/wire_errs.txt @@ -1 +1 @@ -provider for example.com/foo.Foo returns cleanup but injection does not return cleanup function +/wire_gopath/src/example.com/foo/wire.go:x:y: inject injectFoo: provider for example.com/foo.Foo returns cleanup but injection does not return cleanup function \ No newline at end of file diff --git a/internal/wire/testdata/InjectorMissingError/want/wire_errs.txt b/internal/wire/testdata/InjectorMissingError/want/wire_errs.txt index 6874f1e..6b3e578 100644 --- a/internal/wire/testdata/InjectorMissingError/want/wire_errs.txt +++ b/internal/wire/testdata/InjectorMissingError/want/wire_errs.txt @@ -1 +1 @@ -provider for example.com/foo.Foo returns error but injection not allowed to fail +/wire_gopath/src/example.com/foo/wire.go:x:y: inject injectFoo: provider for example.com/foo.Foo returns error but injection not allowed to fail \ No newline at end of file diff --git a/internal/wire/testdata/InterfaceBindingDoesntImplement/want/wire_errs.txt b/internal/wire/testdata/InterfaceBindingDoesntImplement/want/wire_errs.txt index c92114a..2d86d8b 100644 --- a/internal/wire/testdata/InterfaceBindingDoesntImplement/want/wire_errs.txt +++ b/internal/wire/testdata/InterfaceBindingDoesntImplement/want/wire_errs.txt @@ -1 +1 @@ -string does not implement example.com/foo.Fooer +/wire_gopath/src/example.com/foo/wire.go:x:y: string does not implement example.com/foo.Fooer \ No newline at end of file diff --git a/internal/wire/testdata/InterfaceBindingInvalidArg0/want/wire_errs.txt b/internal/wire/testdata/InterfaceBindingInvalidArg0/want/wire_errs.txt index 6c4aa11..76e5954 100644 --- a/internal/wire/testdata/InterfaceBindingInvalidArg0/want/wire_errs.txt +++ b/internal/wire/testdata/InterfaceBindingInvalidArg0/want/wire_errs.txt @@ -1 +1 @@ -first argument to Bind must be a pointer to an interface type; found string +/wire_gopath/src/example.com/foo/wire.go:x:y: first argument to Bind must be a pointer to an interface type; found string \ No newline at end of file diff --git a/internal/wire/testdata/InterfaceBindingNotEnoughArgs/want/wire_errs.txt b/internal/wire/testdata/InterfaceBindingNotEnoughArgs/want/wire_errs.txt index ffd395a..c75e2cb 100644 --- a/internal/wire/testdata/InterfaceBindingNotEnoughArgs/want/wire_errs.txt +++ b/internal/wire/testdata/InterfaceBindingNotEnoughArgs/want/wire_errs.txt @@ -1 +1 @@ -too few arguments in call to wire.Bind +/wire_gopath/src/example.com/foo/wire.go:x:y: too few arguments in call to wire.Bind \ No newline at end of file diff --git a/internal/wire/testdata/InterfaceValueDoesntImplement/want/wire_errs.txt b/internal/wire/testdata/InterfaceValueDoesntImplement/want/wire_errs.txt index d88c9ce..525859e 100644 --- a/internal/wire/testdata/InterfaceValueDoesntImplement/want/wire_errs.txt +++ b/internal/wire/testdata/InterfaceValueDoesntImplement/want/wire_errs.txt @@ -1 +1 @@ -string does not implement io.Reader +/wire_gopath/src/example.com/foo/wire.go:x:y: string does not implement io.Reader \ No newline at end of file diff --git a/internal/wire/testdata/InterfaceValueInvalidArg0/want/wire_errs.txt b/internal/wire/testdata/InterfaceValueInvalidArg0/want/wire_errs.txt index 8c6768c..b956fb4 100644 --- a/internal/wire/testdata/InterfaceValueInvalidArg0/want/wire_errs.txt +++ b/internal/wire/testdata/InterfaceValueInvalidArg0/want/wire_errs.txt @@ -1 +1 @@ -first argument to InterfaceValue must be a pointer to an interface type; found string +/wire_gopath/src/example.com/foo/wire.go:x:y: first argument to InterfaceValue must be a pointer to an interface type; found string \ No newline at end of file diff --git a/internal/wire/testdata/InterfaceValueNotEnoughArgs/want/wire_errs.txt b/internal/wire/testdata/InterfaceValueNotEnoughArgs/want/wire_errs.txt index e26d19d..5c75536 100644 --- a/internal/wire/testdata/InterfaceValueNotEnoughArgs/want/wire_errs.txt +++ b/internal/wire/testdata/InterfaceValueNotEnoughArgs/want/wire_errs.txt @@ -1 +1 @@ -too few arguments in call to wire.InterfaceValue +/wire_gopath/src/example.com/foo/wire.go:x:y: too few arguments in call to wire.InterfaceValue \ No newline at end of file diff --git a/internal/wire/testdata/InvalidInjector/want/wire_errs.txt b/internal/wire/testdata/InvalidInjector/want/wire_errs.txt index fbd6607..aba63c0 100644 --- a/internal/wire/testdata/InvalidInjector/want/wire_errs.txt +++ b/internal/wire/testdata/InvalidInjector/want/wire_errs.txt @@ -1,4 +1,3 @@ -a call to wire.Build indicates that this function is an injector, but injectors -must consist of only the wire.Build call and an optional return -a call to wire.Build indicates that this function is an injector, but injectors -must consist of only the wire.Build call and an optional return +a call to wire.Build indicates that this function is an injector, but injectors must consist of only the wire.Build call and an optional return + +a call to wire.Build indicates that this function is an injector, but injectors must consist of only the wire.Build call and an optional return \ No newline at end of file diff --git a/internal/wire/testdata/MultipleBindings/want/wire_errs.txt b/internal/wire/testdata/MultipleBindings/want/wire_errs.txt index acb47f5..56c177e 100644 --- a/internal/wire/testdata/MultipleBindings/want/wire_errs.txt +++ b/internal/wire/testdata/MultipleBindings/want/wire_errs.txt @@ -1,6 +1,11 @@ -/wire_gopath/src/example.com/foo/wire.go:27:8: wire.Build has multiple bindings for example.com/foo.Foo (current binding: provider "provideFooAgain" (/wire_gopath/src/example.com/foo/foo.go:39:6); previous binding: provider "provideFoo" (/wire_gopath/src/example.com/foo/foo.go:35:6) -/wire_gopath/src/example.com/foo/wire.go:32:8: wire.Build has multiple bindings for example.com/foo.Foo (current binding: provider "provideFoo" (/wire_gopath/src/example.com/foo/foo.go:35:6); previous binding: provider "provideFoo" (/wire_gopath/src/example.com/foo/foo.go:35:6) <- provider set "Set" (/wire_gopath/src/example.com/foo/foo.go:31:11) -/wire_gopath/src/example.com/foo/wire.go:37:8: wire.Build has multiple bindings for example.com/foo.Foo (current binding: provider "provideFoo" (/wire_gopath/src/example.com/foo/foo.go:35:6); previous binding: provider "provideFoo" (/wire_gopath/src/example.com/foo/foo.go:35:6) <- provider set "Set" (/wire_gopath/src/example.com/foo/foo.go:31:11) <- provider set "SuperSet" (/wire_gopath/src/example.com/foo/foo.go:32:16) -/wire_gopath/src/example.com/foo/foo.go:33:32: SetWithDuplicateBindings has multiple bindings for example.com/foo.Foo (current binding: provider "provideFoo" (/wire_gopath/src/example.com/foo/foo.go:35:6) <- provider set "Set" (/wire_gopath/src/example.com/foo/foo.go:31:11) <- provider set "SuperSet" (/wire_gopath/src/example.com/foo/foo.go:32:16); previous binding: provider "provideFoo" (/wire_gopath/src/example.com/foo/foo.go:35:6) <- provider set "Set" (/wire_gopath/src/example.com/foo/foo.go:31:11) -/wire_gopath/src/example.com/foo/wire.go:47:8: wire.Build has multiple bindings for example.com/foo.Foo (current binding: wire.Value (/wire_gopath/src/example.com/foo/wire.go:47:42); previous binding: provider "provideFoo" (/wire_gopath/src/example.com/foo/foo.go:35:6) -/wire_gopath/src/example.com/foo/wire.go:52:8: wire.Build has multiple bindings for example.com/foo.Bar (current binding: wire.Bind (/wire_gopath/src/example.com/foo/wire.go:52:31); previous binding: provider "provideBar" (/wire_gopath/src/example.com/foo/foo.go:43:6) +/wire_gopath/src/example.com/foo/wire.go:x:y: wire.Build has multiple bindings for example.com/foo.Foo (current binding: provider "provideFooAgain" (/wire_gopath/src/example.com/foo/foo.go:x:y); previous binding: provider "provideFoo" (/wire_gopath/src/example.com/foo/foo.go:x:y) + +/wire_gopath/src/example.com/foo/wire.go:x:y: wire.Build has multiple bindings for example.com/foo.Foo (current binding: provider "provideFoo" (/wire_gopath/src/example.com/foo/foo.go:x:y); previous binding: provider "provideFoo" (/wire_gopath/src/example.com/foo/foo.go:x:y) <- provider set "Set" (/wire_gopath/src/example.com/foo/foo.go:x:y) + +/wire_gopath/src/example.com/foo/wire.go:x:y: wire.Build has multiple bindings for example.com/foo.Foo (current binding: provider "provideFoo" (/wire_gopath/src/example.com/foo/foo.go:x:y); previous binding: provider "provideFoo" (/wire_gopath/src/example.com/foo/foo.go:x:y) <- provider set "Set" (/wire_gopath/src/example.com/foo/foo.go:x:y) <- provider set "SuperSet" (/wire_gopath/src/example.com/foo/foo.go:x:y) + +/wire_gopath/src/example.com/foo/foo.go:x:y: SetWithDuplicateBindings has multiple bindings for example.com/foo.Foo (current binding: provider "provideFoo" (/wire_gopath/src/example.com/foo/foo.go:x:y) <- provider set "Set" (/wire_gopath/src/example.com/foo/foo.go:x:y) <- provider set "SuperSet" (/wire_gopath/src/example.com/foo/foo.go:x:y); previous binding: provider "provideFoo" (/wire_gopath/src/example.com/foo/foo.go:x:y) <- provider set "Set" (/wire_gopath/src/example.com/foo/foo.go:x:y) + +/wire_gopath/src/example.com/foo/wire.go:x:y: wire.Build has multiple bindings for example.com/foo.Foo (current binding: wire.Value (/wire_gopath/src/example.com/foo/wire.go:x:y); previous binding: provider "provideFoo" (/wire_gopath/src/example.com/foo/foo.go:x:y) + +/wire_gopath/src/example.com/foo/wire.go:x:y: wire.Build has multiple bindings for example.com/foo.Bar (current binding: wire.Bind (/wire_gopath/src/example.com/foo/wire.go:x:y); previous binding: provider "provideBar" (/wire_gopath/src/example.com/foo/foo.go:x:y) \ No newline at end of file diff --git a/internal/wire/testdata/MultipleMissingInputs/want/wire_errs.txt b/internal/wire/testdata/MultipleMissingInputs/want/wire_errs.txt index 9a42fd1..386de29 100644 --- a/internal/wire/testdata/MultipleMissingInputs/want/wire_errs.txt +++ b/internal/wire/testdata/MultipleMissingInputs/want/wire_errs.txt @@ -1,4 +1,7 @@ -inject injectMissingOutputType: no provider found for example.com/foo.Foo, output of injector -inject injectMultipleMissingTypes: no provider found for example.com/foo.Foo, needed by example.com/foo.Baz in provider "provideBaz" (/wire_gopath/src/example.com/foo/foo.go:29:6) -inject injectMultipleMissingTypes: no provider found for example.com/foo.Bar, needed by example.com/foo.Baz in provider "provideBaz" (/wire_gopath/src/example.com/foo/foo.go:29:6) -inject injectMissingRecursiveType: no provider found for example.com/foo.Foo, needed by example.com/foo.Zip in provider "provideZip" (/wire_gopath/src/example.com/foo/foo.go:37:6), needed by example.com/foo.Zap in provider "provideZap" (/wire_gopath/src/example.com/foo/foo.go:41:6), needed by example.com/foo.Zop in provider "provideZop" (/wire_gopath/src/example.com/foo/foo.go:45:6) +/wire_gopath/src/example.com/foo/wire.go:x:y: inject injectMissingOutputType: no provider found for example.com/foo.Foo, output of injector + +/wire_gopath/src/example.com/foo/wire.go:x:y: inject injectMultipleMissingTypes: no provider found for example.com/foo.Foo, needed by example.com/foo.Baz in provider "provideBaz" (/wire_gopath/src/example.com/foo/foo.go:x:y) + +/wire_gopath/src/example.com/foo/wire.go:x:y: inject injectMultipleMissingTypes: no provider found for example.com/foo.Bar, needed by example.com/foo.Baz in provider "provideBaz" (/wire_gopath/src/example.com/foo/foo.go:x:y) + +/wire_gopath/src/example.com/foo/wire.go:x:y: inject injectMissingRecursiveType: no provider found for example.com/foo.Foo, needed by example.com/foo.Zip in provider "provideZip" (/wire_gopath/src/example.com/foo/foo.go:x:y), needed by example.com/foo.Zap in provider "provideZap" (/wire_gopath/src/example.com/foo/foo.go:x:y), needed by example.com/foo.Zop in provider "provideZop" (/wire_gopath/src/example.com/foo/foo.go:x:y) \ No newline at end of file diff --git a/internal/wire/testdata/NoImplicitInterface/want/wire_errs.txt b/internal/wire/testdata/NoImplicitInterface/want/wire_errs.txt index 1e7c147..2a5a901 100644 --- a/internal/wire/testdata/NoImplicitInterface/want/wire_errs.txt +++ b/internal/wire/testdata/NoImplicitInterface/want/wire_errs.txt @@ -1,2 +1 @@ -no provider found -Fooer +/wire_gopath/src/example.com/foo/wire.go:x:y: inject injectFooer: no provider found for example.com/foo.Fooer, output of injector \ No newline at end of file diff --git a/internal/wire/testdata/UnexportedValue/want/wire_errs.txt b/internal/wire/testdata/UnexportedValue/want/wire_errs.txt index 8760fd1..1ad9802 100644 --- a/internal/wire/testdata/UnexportedValue/want/wire_errs.txt +++ b/internal/wire/testdata/UnexportedValue/want/wire_errs.txt @@ -1 +1 @@ -unexported identifier privateMsg +/wire_gopath/src/example.com/foo/wire.go:x:y: inject injectedMessage: value string can't be used: uses unexported identifier privateMsg \ No newline at end of file diff --git a/internal/wire/testdata/UnusedProviders/want/wire_errs.txt b/internal/wire/testdata/UnusedProviders/want/wire_errs.txt index d096736..ab32986 100644 --- a/internal/wire/testdata/UnusedProviders/want/wire_errs.txt +++ b/internal/wire/testdata/UnusedProviders/want/wire_errs.txt @@ -1,4 +1,7 @@ -unused provider set "unusedSet" -unused provider "provideUnused" -unused value of type string -unused interface binding to type example.com/foo.Fooer +/wire_gopath/src/example.com/foo/wire.go:x:y: inject injectBar: unused provider set "unusedSet" + +/wire_gopath/src/example.com/foo/wire.go:x:y: inject injectBar: unused provider "provideUnused" + +/wire_gopath/src/example.com/foo/wire.go:x:y: inject injectBar: unused value of type string + +/wire_gopath/src/example.com/foo/wire.go:x:y: inject injectBar: unused interface binding to type example.com/foo.Fooer \ No newline at end of file diff --git a/internal/wire/testdata/ValueFromFunctionScope/want/wire_errs.txt b/internal/wire/testdata/ValueFromFunctionScope/want/wire_errs.txt index a3b3705..a7cbd58 100644 --- a/internal/wire/testdata/ValueFromFunctionScope/want/wire_errs.txt +++ b/internal/wire/testdata/ValueFromFunctionScope/want/wire_errs.txt @@ -1 +1 @@ -inject injectBar: value int can't be used: f is not declared in package scope +/wire_gopath/src/example.com/foo/wire.go:x:y: inject injectBar: value int can't be used: f is not declared in package scope \ No newline at end of file diff --git a/internal/wire/testdata/ValueIsInterfaceValue/want/wire_errs.txt b/internal/wire/testdata/ValueIsInterfaceValue/want/wire_errs.txt index 022f9b8..939a972 100644 --- a/internal/wire/testdata/ValueIsInterfaceValue/want/wire_errs.txt +++ b/internal/wire/testdata/ValueIsInterfaceValue/want/wire_errs.txt @@ -1 +1 @@ -argument to Value may not be an interface value (found io.Reader); use InterfaceValue instead +/wire_gopath/src/example.com/foo/wire.go:x:y: argument to Value may not be an interface value (found io.Reader); use InterfaceValue instead \ No newline at end of file diff --git a/internal/wire/wire_test.go b/internal/wire/wire_test.go index f6571fa..2c1327c 100644 --- a/internal/wire/wire_test.go +++ b/internal/wire/wire_test.go @@ -25,6 +25,7 @@ import ( "os" "os/exec" "path/filepath" + "regexp" "runtime" "sort" "strings" @@ -81,15 +82,22 @@ func TestWire(t *testing.T) { defer t.Logf("wire_gen.go:\n%s", gen) } if len(errs) > 0 { - for _, e := range errs { - t.Log(e) + gotErrStrings := make([]string, len(errs)) + for i, e := range errs { + gotErrStrings[i] = scrubError(e.Error()) + t.Log(gotErrStrings[i]) } if !test.wantWireError { - t.Fatal("Did not expect errors.") + t.Fatal("Did not expect errors. To -record an error, create want/wire_errs.txt.") } - for _, s := range test.wantWireErrorStrings { - if !errorListContains(errs, s) { - t.Errorf("Errors did not contain %q", s) + if *setup.Record { + wireErrsFile := filepath.Join(testRoot, test.name, "want", "wire_errs.txt") + if err := ioutil.WriteFile(wireErrsFile, []byte(strings.Join(gotErrStrings, "\n\n")), 0666); err != nil { + t.Fatalf("failed to write wire_errs.txt file: %v", err) + } + } else { + if diff := cmp.Diff(gotErrStrings, test.wantWireErrorStrings); diff != "" { + t.Errorf("Errors didn't match expected errors from wire_errors.txt:\n%s", diff) } } return @@ -359,6 +367,14 @@ type testCase struct { wantWireErrorStrings []string } +var scrubLineNumberAndPositionRegex = regexp.MustCompile("\\.go:[\\d]+:[\\d]+") +var scrubLineNumberRegex = regexp.MustCompile("\\.go:[\\d]+") + +func scrubError(s string) string { + s = scrubLineNumberAndPositionRegex.ReplaceAllString(s, ".go:x:y") + return scrubLineNumberRegex.ReplaceAllString(s, ".go:x") +} + // loadTestCase reads a test case from a directory. // // The directory structure is: @@ -375,8 +391,11 @@ type testCase struct { // want/ // // wire_errs.txt -// expected errors from the Wire Generate function, -// missing if no errors expected +// Expected errors from the Wire Generate function, +// missing if no errors expected. +// Distinct errors are separated by a blank line, +// and line numbers and line positions are scrubbed +// (e.g., "foo.go:52:8" --> "foo.go:x:y"). // // wire_gen.go // verified output of wire from a test run with @@ -398,7 +417,7 @@ func loadTestCase(root string, wireGoSrc []byte) (*testCase, error) { wantWireError := err == nil var wantWireErrorStrings []string if wantWireError { - wantWireErrorStrings = strings.Split(strings.TrimSpace(string(wireErrb)), "\n") + wantWireErrorStrings = strings.Split(scrubError(string(wireErrb)), "\n\n") } else { if !*setup.Record { wantWireOutput, err = ioutil.ReadFile(filepath.Join(root, "want", "wire_gen.go")) @@ -691,12 +710,3 @@ func runGo(bctx *build.Context, dir string, args ...string) error { } return nil } - -func errorListContains(errs []error, substr string) bool { - for _, e := range errs { - if strings.Contains(e.Error(), substr) { - return true - } - } - return false -}