diff --git a/internal/plan/plan.go b/internal/plan/plan.go index 248d47c..facbd9f 100644 --- a/internal/plan/plan.go +++ b/internal/plan/plan.go @@ -111,12 +111,14 @@ func Build(c ir.Container, idx *Index, opts Options) (Plan, []diag.Diag) { diags = append(diags, emDiags...) r := &resolver{ - idx: idx, - inputs: inputs, - overrides: overrides, - embeds: embeds, - stepByKey: map[string]int{}, - active: map[string]bool{}, + idx: idx, + inputs: inputs, + overrides: overrides, + embeds: embeds, + stepByKey: map[string]int{}, + active: map[string]bool{}, + selfPkgPath: c.PkgPath, + selfFuncName: constructorName, } var outputs []Output @@ -175,6 +177,15 @@ type resolver struct { steps []Step stepByKey map[string]int active map[string]bool + + // selfPkgPath and selfFuncName identify this container's own + // generated constructor. They are used to filter the by-type lookup + // so a container whose `inject:"returns"` declares an interface that + // matches an unrelated provider does not see its own previously + // emitted constructor as a candidate (a self-loop that would also + // produce a spurious "multiple providers" error). + selfPkgPath string + selfFuncName string } // embedSource describes one exported field of an inject:"embed" input that @@ -192,6 +203,44 @@ func (r *resolver) resolveField(f ir.Field) (int, []diag.Diag) { return r.resolveByType(f.Type, f.Pos, "field "+f.Name) } +// excludeSelfProvider drops the container's own previously generated +// constructor from the candidate list so a `inject:"returns"` field does +// not pick itself up via type lookup. Callers that name a provider +// explicitly via inject:"with=..." are not affected. +// +// The common case is that the self-provider is not in the candidate +// list at all (most fields request types unrelated to the container's +// own return type), so the function returns the original slice without +// allocating when there is nothing to exclude. +func (r *resolver) excludeSelfProvider(candidates []*ir.Provider) []*ir.Provider { + if r.selfFuncName == "" { + return candidates + } + selfIdx := -1 + for i, c := range candidates { + if c.PkgPath == r.selfPkgPath && c.FuncName == r.selfFuncName { + selfIdx = i + break + } + } + if selfIdx < 0 { + return candidates + } + // Boundary cases — the self-provider sits at one end of the slice, + // so a sub-slice is enough and no allocation is needed. This covers + // the very common case of a single matching candidate. + if selfIdx == 0 { + return candidates[1:] + } + if selfIdx == len(candidates)-1 { + return candidates[:selfIdx] + } + kept := make([]*ir.Provider, 0, len(candidates)-1) + kept = append(kept, candidates[:selfIdx]...) + kept = append(kept, candidates[selfIdx+1:]...) + return kept +} + func (r *resolver) resolveByType(want types.Type, pos token.Position, parent string) (int, []diag.Diag) { tk := TypeKey(want) @@ -210,6 +259,7 @@ func (r *resolver) resolveByType(want types.Type, pos token.Position, parent str } candidates := r.idx.LookupByType(want) + candidates = r.excludeSelfProvider(candidates) if len(candidates) == 0 { return -1, []diag.Diag{ diag.Errorf(pos, "no provider for %s (required by %s)", TypeString(want), parent), diff --git a/internal/plan/plan_test.go b/internal/plan/plan_test.go index 0c1f51f..4a870f4 100644 --- a/internal/plan/plan_test.go +++ b/internal/plan/plan_test.go @@ -344,6 +344,35 @@ type Container struct { } } +func TestBuild_SelfGeneratedProviderIgnored(t *testing.T) { + t.Parallel() + + // A `inject:"returns"` container whose previously generated + // constructor is now visible to the provider scan must not pick that + // constructor up as a candidate when resolving its own field, or it + // would loop forever. The unrelated, non-self provider remains usable. + src := `package test +type Greeter interface{ Greet() string } +type greeterImpl struct{} +func (greeterImpl) Greet() string { return "" } +func NewGreeterImpl() Greeter { return greeterImpl{} } + +// Pretend this came from a previous generation of NewWrapper. +func NewWrapper() Greeter { return nil } + +type wrapper struct { + service Greeter ` + "`inject:\"returns\"`" + ` +} +` + p, ds := build(t, src, "wrapper", plan.Options{}) + if diag.HasErrors(ds) { + t.Fatalf("expected the self provider NewWrapper to be filtered out, got %v", ds) + } + if len(p.Steps) != 1 || p.Steps[0].Provider == nil || p.Steps[0].Provider.FuncName != "NewGreeterImpl" { + t.Errorf("expected NewGreeterImpl to be the only provider step; got %+v", p.Steps) + } +} + func TestBuild_FieldNameTakenForcesSuffix(t *testing.T) { t.Parallel() diff --git a/internal/scan/provider.go b/internal/scan/provider.go index e6d137c..df860b1 100644 --- a/internal/scan/provider.go +++ b/internal/scan/provider.go @@ -45,8 +45,15 @@ func providersInPackage(pkg *packages.Package) ([]ir.Provider, []diag.Diag) { providers []ir.Provider diags []diag.Diag ) + // Generated files are intentionally NOT skipped here. Containers in + // generated files would re-trigger generation (and are filtered by + // containersInPackage), but provider functions emitted by an earlier + // generation are ordinary top-level Go functions whose results other + // packages may legitimately consume via type lookup. Skipping them + // would hide the constructors injector itself produced from downstream + // containers. for _, file := range pkg.Syntax { - if file == nil || isGeneratedFile(file) { + if file == nil { continue } for _, decl := range file.Decls { diff --git a/internal/scan/provider_test.go b/internal/scan/provider_test.go index e082b16..4d9f97b 100644 --- a/internal/scan/provider_test.go +++ b/internal/scan/provider_test.go @@ -61,9 +61,13 @@ func (d *DB) NewSelf() *DB { return d } } } -func TestProviders_GeneratedFileIgnored(t *testing.T) { +func TestProviders_GeneratedFileScanned(t *testing.T) { t.Parallel() + // Provider functions emitted in a previous generation must remain + // discoverable so downstream containers in other packages can resolve + // against the injector-generated constructors. Containers in the same + // file are still filtered out by containersInPackage. src := `// Code generated by injector. DO NOT EDIT. package test @@ -78,8 +82,8 @@ func NewDB() *DB { return nil } if diag.HasErrors(ds) { t.Fatalf("unexpected error diags: %v", ds) } - if len(ps) != 0 { - t.Errorf("expected generated file to be ignored, got %d providers", len(ps)) + if len(ps) != 1 || ps[0].FuncName != "NewDB" { + t.Errorf("expected NewDB to be picked up from a generated file, got %+v", ps) } }