fix(scan,plan): make injector-generated providers visible cross-package#8
Conversation
There was a problem hiding this comment.
Code Review
This pull request updates the provider scanning logic to include previously generated files, enabling downstream containers to resolve against injector-generated constructors. To prevent infinite loops, the planning phase now filters out a container's own constructor from the candidate list using the new excludeSelfProvider method. Feedback suggests optimizing excludeSelfProvider to avoid unnecessary slice allocations when the self-provider is not present in the candidates.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| func (r *resolver) excludeSelfProvider(candidates []*ir.Provider) []*ir.Provider { | ||
| if r.selfFuncName == "" { | ||
| return candidates | ||
| } | ||
| kept := candidates[:0:0] | ||
| for _, c := range candidates { | ||
| if c.PkgPath == r.selfPkgPath && c.FuncName == r.selfFuncName { | ||
| continue | ||
| } | ||
| kept = append(kept, c) | ||
| } | ||
| return kept | ||
| } |
There was a problem hiding this comment.
In the common case, the container's own constructor is not present in the candidate list. The current implementation always allocates a new slice (via candidates[:0:0] and append) even when no candidates are excluded.
We can optimize this by first checking if the self-provider is present in the candidates. If it is not, we can return the original candidates slice directly, avoiding any allocation. If it is present, we can pre-allocate the new slice with the exact required capacity (len(candidates) - 1) to avoid multiple slice growth allocations.
func (r *resolver) excludeSelfProvider(candidates []*ir.Provider) []*ir.Provider {
if r.selfFuncName == "" {
return candidates
}
found := false
for _, c := range candidates {
if c.PkgPath == r.selfPkgPath && c.FuncName == r.selfFuncName {
found = true
break
}
}
if !found {
return candidates
}
kept := make([]*ir.Provider, 0, len(candidates)-1)
for _, c := range candidates {
if c.PkgPath == r.selfPkgPath && c.FuncName == r.selfFuncName {
continue
}
kept = append(kept, c)
}
return kept
}There was a problem hiding this comment.
Applied in 0bdec5e, with a small tweak over the suggestion: rather than running two passes through candidates when the self-provider is present, the first pass records its index and the second pass uses append(..., candidates[:idx]...) + append(..., candidates[idx+1:]...). Same fast-path behavior (no allocation when self is absent, exact-capacity allocation otherwise), one pass through the slice when self is found.
selfIdx := -1
for i, c := range candidates {
if c.PkgPath == r.selfPkgPath && c.FuncName == r.selfFuncName {
selfIdx = i
break
}
}
if selfIdx < 0 {
return candidates
}
kept := make([]*ir.Provider, 0, len(candidates)-1)
kept = append(kept, candidates[:selfIdx]...)
kept = append(kept, candidates[selfIdx+1:]...)
return kept|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request modifies the provider scanning and resolution logic to ensure that provider functions emitted in previous generations are discoverable, while preventing a container from resolving against its own previously generated constructor (which would cause a self-loop). Specifically, it stops skipping generated files during package scanning and introduces an excludeSelfProvider helper in the resolver to filter out the container's own constructor from candidate providers. A test case is also added to verify this behavior. Feedback suggests optimizing excludeSelfProvider to avoid slice allocations when the self-provider is at the boundaries of the candidates list.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| if selfIdx < 0 { | ||
| return candidates | ||
| } | ||
| kept := make([]*ir.Provider, 0, len(candidates)-1) | ||
| kept = append(kept, candidates[:selfIdx]...) | ||
| kept = append(kept, candidates[selfIdx+1:]...) | ||
| return kept |
There was a problem hiding this comment.
We can optimize excludeSelfProvider to avoid allocations when the self-provider is at the very beginning or the very end of the candidates list (including the common case where it is the only candidate). By returning a sub-slice of candidates directly, we avoid allocating a new slice.
if selfIdx < 0 {
return candidates
}
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 keptThere was a problem hiding this comment.
Applied in 0f1f216. Boundary checks added before the middle-of-slice case:
if selfIdx == 0 {
return candidates[1:]
}
if selfIdx == len(candidates)-1 {
return candidates[:selfIdx]
}This catches the most common shape — a single matching candidate that is itself the self-provider — without any allocation, plus the head/tail cases. The interior case still allocates len-1 exactly.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request ensures that provider functions emitted by an earlier generation in generated files remain discoverable for downstream containers, while preventing a container from picking up its own previously generated constructor during type lookup (which would cause a self-loop). This is achieved by scanning generated files for providers and introducing an excludeSelfProvider filter in the resolver. Unit tests have been updated and added to verify this behavior. There are no review comments to address.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
Summary
Two coupled changes so that constructors injector itself emits become usable as providers in downstream containers — a pattern that comes up the moment a project lays out per-feature usecases with
inject:"returns"and a top-level handler container that consumes those usecase interfaces.1.
fix(scan): include providers declared in injector-generated filesprovidersInPackageused to short-circuit on any file that carried the canonical "Code generated … DO NOT EDIT." marker. That was the right call forcontainersInPackage(so injector does not re-generate from its own output), but it was wrong for the provider scan: the emittedNewSignUp(infras *di.Infras) (usecase.SignUp, error)is just a top-level Go function. Skipping it hid every injector-produced constructor from cross-package resolution, so a handler container withSignUp usecase.SignUp inject:""could never find a provider.The check is removed from
providersInPackageonly; container scanning continues to skip generated files.2.
fix(plan): exclude the container's own constructor from by-type lookupRe-scanning generated files exposes one regression on the
inject:"returns"pattern: anappcontainer whose constructor producesgreeter.Greeterwould, after this PR, see its own previously generatedNewGreeterlisted as a candidate alongside the hand-writtengreeter.NewGreeter. The naive lookup then errors with "multiple providers", and even if disambiguated, picking the self-constructor would loop forever.The plan resolver now remembers the current container's
(PkgPath, ConstructorName)and filters the by-type lookup so the container can never resolve against its own constructor. Explicit references viainject:"with=…"are untouched — naming the self-constructor on purpose still works.Test plan
internal/scan:TestProviders_GeneratedFileScanned(replaces the oldTestProviders_GeneratedFileIgnored) verifies aNewDBin a generated file is discoverable.internal/plan: newTestBuild_SelfGeneratedProviderIgnoredmocks awrappercontainer whose generatedNewWrapperreturns the same interface as an unrelatedNewGreeterImpl; resolution still picks the unrelated provider without "multiple providers".make lintreports 0 issuesmake test-e2eregenerates every example and produces no diff on the committed files — the new logic is a strict superset of the old behavior on well-formed setups.