diff --git a/internal/wire/analyze.go b/internal/wire/analyze.go index 7d93c06..8d2c3dc 100644 --- a/internal/wire/analyze.go +++ b/internal/wire/analyze.go @@ -299,22 +299,20 @@ func verifyArgsUsed(set *ProviderSet, used []*providerSetSrc) []error { func buildProviderMap(fset *token.FileSet, hasher typeutil.Hasher, set *ProviderSet) (*typeutil.Map, *typeutil.Map, []error) { providerMap := new(typeutil.Map) providerMap.SetHasher(hasher) - srcMap := new(typeutil.Map) + srcMap := new(typeutil.Map) // to *providerSetSrc srcMap.SetHasher(hasher) - setMap := new(typeutil.Map) // to *ProviderSet, for error messages - setMap.SetHasher(hasher) // Process imports first, verifying that there are no conflicts between sets. ec := new(errorCollector) for _, imp := range set.Imports { + src := &providerSetSrc{Import: imp} imp.providerMap.Iterate(func(k types.Type, v interface{}) { - if providerMap.At(k) != nil { - ec.add(bindingConflictError(fset, imp.Pos, k, setMap.At(k).(*ProviderSet))) + if prevSrc := srcMap.At(k); prevSrc != nil { + ec.add(bindingConflictError(fset, k, set, src, prevSrc.(*providerSetSrc))) return } providerMap.Set(k, v) - srcMap.Set(k, &providerSetSrc{Import: imp}) - setMap.Set(k, imp) + srcMap.Set(k, src) }) } if len(ec.errors) > 0 { @@ -325,23 +323,22 @@ func buildProviderMap(fset *token.FileSet, hasher typeutil.Hasher, set *Provider for _, p := range set.Providers { src := &providerSetSrc{Provider: p} for _, typ := range p.Out { - if providerMap.At(typ) != nil { - ec.add(bindingConflictError(fset, p.Pos, typ, setMap.At(typ).(*ProviderSet))) + if prevSrc := srcMap.At(typ); prevSrc != nil { + ec.add(bindingConflictError(fset, typ, set, src, prevSrc.(*providerSetSrc))) continue } providerMap.Set(typ, &ProvidedType{t: typ, p: p}) srcMap.Set(typ, src) - setMap.Set(typ, set) } } for _, v := range set.Values { - if providerMap.At(v.Out) != nil { - ec.add(bindingConflictError(fset, v.Pos, v.Out, setMap.At(v.Out).(*ProviderSet))) + src := &providerSetSrc{Value: v} + if prevSrc := srcMap.At(v.Out); prevSrc != nil { + ec.add(bindingConflictError(fset, v.Out, set, src, prevSrc.(*providerSetSrc))) continue } providerMap.Set(v.Out, &ProvidedType{t: v.Out, v: v}) - srcMap.Set(v.Out, &providerSetSrc{Value: v}) - setMap.Set(v.Out, set) + srcMap.Set(v.Out, src) } if len(ec.errors) > 0 { return nil, nil, ec.errors @@ -350,8 +347,9 @@ func buildProviderMap(fset *token.FileSet, hasher typeutil.Hasher, set *Provider // Process bindings in set. Must happen after the other providers to // ensure the concrete type is being provided. for _, b := range set.Bindings { - if providerMap.At(b.Iface) != nil { - ec.add(bindingConflictError(fset, b.Pos, b.Iface, setMap.At(b.Iface).(*ProviderSet))) + src := &providerSetSrc{Binding: b} + if prevSrc := srcMap.At(b.Iface); prevSrc != nil { + ec.add(bindingConflictError(fset, b.Iface, set, src, prevSrc.(*providerSetSrc))) continue } concrete := providerMap.At(b.Provided) @@ -362,8 +360,7 @@ func buildProviderMap(fset *token.FileSet, hasher typeutil.Hasher, set *Provider continue } providerMap.Set(b.Iface, concrete) - srcMap.Set(b.Iface, &providerSetSrc{Binding: b}) - setMap.Set(b.Iface, set) + srcMap.Set(b.Iface, src) } if len(ec.errors) > 0 { return nil, nil, ec.errors @@ -433,15 +430,15 @@ func verifyAcyclic(providerMap *typeutil.Map, hasher typeutil.Hasher) []error { // bindingConflictError creates a new error describing multiple bindings // for the same output type. -func bindingConflictError(fset *token.FileSet, pos token.Pos, typ types.Type, prevSet *ProviderSet) error { - typString := types.TypeString(typ, nil) - var err error - if prevSet.VarName == "" { - err = fmt.Errorf("multiple bindings for %s (previous binding at %v)", - typString, fset.Position(prevSet.Pos)) +func bindingConflictError(fset *token.FileSet, typ types.Type, set *ProviderSet, cur, prev *providerSetSrc) error { + sb := new(strings.Builder) + if set.VarName == "" { + fmt.Fprintf(sb, "wire.Build") } else { - err = fmt.Errorf("multiple bindings for %s (previous binding in %q.%s)", - typString, prevSet.PkgPath, prevSet.VarName) + fmt.Fprintf(sb, set.VarName) } - return notePosition(fset.Position(pos), err) + fmt.Fprintf(sb, " has multiple bindings for %s (", types.TypeString(typ, nil)) + fmt.Fprintf(sb, "current binding: %s", strings.Join(cur.trace(fset, typ), " <- ")) + fmt.Fprintf(sb, "; previous binding: %s", strings.Join(prev.trace(fset, typ), " <- ")) + return notePosition(fset.Position(set.Pos), errors.New(sb.String())) } diff --git a/internal/wire/parse.go b/internal/wire/parse.go index 2450100..6b9c32f 100644 --- a/internal/wire/parse.go +++ b/internal/wire/parse.go @@ -38,6 +38,37 @@ type providerSetSrc struct { Import *ProviderSet } +// trace returns a slice of strings describing the (possibly recursive) source +// of p, including line numbers. +func (p *providerSetSrc) trace(fset *token.FileSet, typ types.Type) []string { + quoted := func(s string) string { + if s == "" { + return "" + } + return fmt.Sprintf("%q ", s) + } + var retval []string + if p.Provider != nil { + kind := "provider" + if p.Provider.IsStruct { + kind = "struct provider" + } + retval = append(retval, fmt.Sprintf("%s %s(%s)", kind, quoted(p.Provider.Name), fset.Position(p.Provider.Pos))) + } else if p.Binding != nil { + retval = append(retval, fmt.Sprintf("wire.Bind (%s)", fset.Position(p.Binding.Pos))) + } else if p.Value != nil { + retval = append(retval, fmt.Sprintf("wire.Value (%s)", fset.Position(p.Value.Pos))) + } else if p.Import != nil { + if parent := p.Import.srcMap.At(typ); parent != nil { + retval = append(retval, parent.(*providerSetSrc).trace(fset, typ)...) + } + retval = append(retval, fmt.Sprintf("provider set %s(%s)", quoted(p.Import.VarName), fset.Position(p.Import.Pos))) + } else { + panic("providerSetSrc with no fields set") + } + return retval +} + // A ProviderSet describes a set of providers. The zero value is an empty // ProviderSet. type ProviderSet struct { diff --git a/internal/wire/testdata/MultipleBindings/foo/foo.go b/internal/wire/testdata/MultipleBindings/foo/foo.go new file mode 100644 index 0000000..960ce49 --- /dev/null +++ b/internal/wire/testdata/MultipleBindings/foo/foo.go @@ -0,0 +1,45 @@ +// Copyright 2018 The Go Cloud Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// https://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package main + +import ( + "io" + "strings" + + "github.com/google/go-cloud/wire" +) + +type context struct{} + +func main() {} + +type Foo string +type Bar io.Reader + +var Set = wire.NewSet(provideFoo) +var SuperSet = wire.NewSet(Set) +var SetWithDuplicateBindings = wire.NewSet(Set, SuperSet) + +func provideFoo() Foo { + return Foo("foo") +} + +func provideFooAgain() Foo { + return Foo("foo foo") +} + +func provideBar() Bar { + return io.Reader(strings.NewReader("hello")) +} diff --git a/internal/wire/testdata/MultipleBindings/foo/wire.go b/internal/wire/testdata/MultipleBindings/foo/wire.go new file mode 100644 index 0000000..ad1434a --- /dev/null +++ b/internal/wire/testdata/MultipleBindings/foo/wire.go @@ -0,0 +1,53 @@ +// Copyright 2018 The Go Cloud Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// https://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//+build wireinject + +package main + +import ( + "strings" + + "github.com/google/go-cloud/wire" +) + +func inject() Foo { + // fail: provideFoo and provideFooAgain both provide Foo. + panic(wire.Build(provideFoo, provideFooAgain)) +} + +func injectFromSet() Foo { + // fail: provideFoo is also provided by Set. + panic(wire.Build(provideFoo, Set)) +} + +func injectFromNestedSet() Foo { + // fail: provideFoo is also provided by SuperSet, via Set. + panic(wire.Build(provideFoo, SuperSet)) +} + +func injectFromSetWithDuplicateBindings() Foo { + // fail: DuplicateBindingsSet has two providers for Foo. + panic(wire.Build(SetWithDuplicateBindings)) +} + +func injectDuplicateValues() Foo { + // fail: provideFoo and wire.Value both provide Foo. + panic(wire.Build(provideFoo, wire.Value(Foo("foo")))) +} + +func injectDuplicateInterface() Bar { + // fail: provideBar and wire.Bind both provide Bar. + panic(wire.Build(provideBar, wire.Bind(new(Bar), strings.NewReader("hello")))) +} diff --git a/internal/wire/testdata/MultipleBindings/pkg b/internal/wire/testdata/MultipleBindings/pkg new file mode 100644 index 0000000..f7a5c8c --- /dev/null +++ b/internal/wire/testdata/MultipleBindings/pkg @@ -0,0 +1 @@ +example.com/foo diff --git a/internal/wire/testdata/MultipleBindings/want/wire_errs.txt b/internal/wire/testdata/MultipleBindings/want/wire_errs.txt new file mode 100644 index 0000000..acb47f5 --- /dev/null +++ b/internal/wire/testdata/MultipleBindings/want/wire_errs.txt @@ -0,0 +1,6 @@ +/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)