feat(plan): wire-style variable naming (field name, type fallback, cascade suffix)

[mickamy]

Summary

Adopt a wire-style variable naming policy for the constructor body:

1. Field-bound step (its value goes into a container field) — variable name = lowerFirst(field name). The reader sees tx := tx.New(...) for a field named Tx, not new or transactor.
2. Intermediate step (consumed only as an argument to a later step) — variable name = lowerFirst(result type name). db.Open(ctx, cfg) *sql.DB becomes db := db.Open(...). The function name is no longer used unless the result type is anonymous.
3. Suffix on collision — when the desired base is already taken by another step or an import alias, append 2, 3, …. The 2-based scheme matches the existing import-alias disambiguation in addByPath (foo / foo2).

The first commit (fix(plan): name vars by result type when the provider is a bare New() factory) was the narrow fix for tx.New() producing the literal new; the second (refactor(plan): adopt wire-style variable naming …) broadens the rule and adds the field-name rename pass. Keeping them split makes the policy change a separate review unit from the bare-New bug fix.

Behavior table (atode shape)

type Infra struct {
    DB  *sql.DB        `inject:"with=db.Open"`
    Tx  tx.Transactor  `inject:""`
    KVS kv.Client      `inject:"with=kv.New"`
}

step	reason	before	after
config.NewDB()	intermediate, type config.DB	db2	db2 (intermediate "db" + db alias collision)
db.Open(ctx, cfg)	bound to field DB	open	db3 (field "db" cascade after intermediate took db2)
tx.New(db)	bound to field Tx	new	tx2 (field "tx" + tx alias collision)
config.NewKV()	intermediate, type config.KV	kv2	kv2
kv.New(cfg)	bound to field KVS	new2	kvs

The db3 cascade is the documented option-A behavior: intermediate steps populate the name table in declaration order, and a later field-bound step that wants a previously-claimed base suffixes upward.

Why result type over function name for intermediates

db.Open(...) *sql.DB is a value of type *sql.DB, not "an open". The function name describes the act of constructing; the variable name should describe the value the rest of the body uses. NewReaderDatabaseConfig() DatabaseConfig becomes databaseConfig := ... — the "Reader" modifier is dropped because it is already visible at the call site.

Test plan

• internal/plan new tests: TestBuild_FieldBoundStepUsesFieldName, TestBuild_IntermediateStepUsesResultType, TestBuild_FieldNameTakenForcesSuffix
• All previously-existing plan tests still pass
• make lint reports 0 issues
• make test-e2e regenerates every example and passes go vet + go build; the diff shows the new naming applied uniformly (databaseConfig, userService, etc.)

[gemini-code-assist]

Code Review

This pull request updates the variable name generation logic for providers in internal/plan/plan.go to handle bare New factories by naming the variable after the result type instead of using the literal name "new". It also adds corresponding unit tests. A review comment suggests mapping the derived name "arg" back to "" (falling back to "v") to avoid misleading variable names when deriveInputName is used on basic types, slices, maps, or anonymous interfaces.

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.

[gemini-code-assist]

Using deriveInputName directly for a provider's result variable name can lead to the variable being named arg (for example, if the result type is a basic type, slice, map, or anonymous interface where a named type cannot be resolved). Since this variable represents a resolved dependency/provider output rather than a constructor input argument, naming it arg is misleading. We should map "arg" back to "" so that it falls back to "v".

Suggested change

	if base == "" {
	base = deriveInputName(p.Result)
	}
	if base == "" {
	base = deriveInputName(p.Result)
	if base == "arg" {
	base = ""
	}
	}

[mickamy]

This comment is reviewing the 1st commit (1025d99), where deriveInputName was a fallback for bare New(). The 2nd commit (d347b32) restructures varNameForProvider to take the deriveInputName result as the primary and fall back to the function name (not "v") when it returns the "arg" sentinel — which is a more informative label than v for anonymous/unnamed result types.

Current shape on main-of-this-branch:

base := deriveInputName(p.Result)
if base == "arg" {
    // Anonymous or unnamed result type — fall back to the function
    // name for a less generic label than "arg".
    base = p.FuncName
    if strings.HasPrefix(base, "New") && len(base) > 3 {
        base = base[3:]
    }
    base = lowerFirst(base)
}
if base == "" {
    base = "v"
}

So a literal arg cannot reach the emitted code: it is either replaced by the function-derived name, or, if that is also empty, by the existing v floor. I'd rather keep the function-name fallback than route everything to v, since v tells the reader nothing about the value.

If you saw a case where arg still leaks through this path, do flag it — happy to look.

[mickamy]

/gemini review

[gemini-code-assist]

Code Review

This pull request improves the variable naming in the generated dependency injection code. Instead of naming variables after constructor functions, they are now named after the types they produce or the destination fields they bind to, with fallback logic and collision resolution. The review feedback points out a performance optimization in the renameOutputSteps function, suggesting to initialize the used map once to reduce the time complexity from $O(M \times N)$ to $O(N)$.

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.

[gemini-code-assist]

The current implementation of renameOutputSteps has an $O(M \times N)$ complexity (where $M$ is the number of outputs and $N$ is the number of steps) because it re-allocates and populates a used map of size $N$ in every iteration of the outer loop.

We can optimize this to $O(N)$ time complexity and a single map allocation by initializing the used map once at the beginning, and updating it dynamically as we rename steps.

func renameOutputSteps(steps []Step, outputs []Output) {
	used := make(map[string]bool, len(steps))
	for _, st := range steps {
		used[st.VarName] = true
	}

	for _, o := range outputs {
		if o.StepIndex < 0 || o.StepIndex >= len(steps) {
			continue
		}
		s := &steps[o.StepIndex]
		if s.Kind == StepKindInput {
			continue
		}
		base := lowerFirst(o.FieldName)
		if base == "" || base == s.VarName {
			continue
		}

		delete(used, s.VarName)

		pick := base
		if used[pick] {
			for i := 2; ; i++ {
				try := fmt.Sprintf("%s%d", base, i)
				if !used[try] {
					pick = try
					break
				}
			}
		}
		s.VarName = pick
		used[pick] = true
	}
}

[mickamy]

Good call — applied in ea4ef55. The used map is built once over all steps, and each output rename now does delete(used, oldName) / used[newName] = true instead of rebuilding the whole set. Behavior is unchanged (no diff in any regenerated example) and the loop is now O(N + M) instead of O(M·N).

[mickamy]

/gemini review

[gemini-code-assist]

Code Review

This pull request improves variable naming in the dependency injection planner by naming variables after their produced result types or destination field names rather than constructor function names. It introduces a renameOutputSteps function to rename steps bound to container outputs and updates varNameForProvider to derive names from result types, with corresponding updates to example generated files and new unit tests. The review feedback suggests a two-pass approach in renameOutputSteps to prevent unnecessary suffixing during renaming collisions, and recommends using types.Unalias to correctly resolve Go 1.22+ type aliases when deriving variable names.

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.

[gemini-code-assist]

In the current implementation of renameOutputSteps, the old name of a step is deleted from the used map and its new name is added during a single pass over the outputs. This can lead to unnecessary suffixing (e.g., db22 instead of db2) if a step's new name was previously occupied by another step that is also scheduled to be renamed later in the loop.

Using a two-pass approach resolves this: first, identify all steps that will be renamed and free up their old names from the used map; second, assign the new names in the original order of their last occurrence. This avoids naming collisions with names that are about to be vacated.

func renameOutputSteps(steps []Step, outputs []Output) {
	used := make(map[string]bool, len(steps))
	for _, st := range steps {
		used[st.VarName] = true
	}

	type renameInfo struct {
		base      string
		lastIndex int
	}
	renameInfos := make(map[int]renameInfo)
	for i, o := range outputs {
		if o.StepIndex < 0 || o.StepIndex >= len(steps) {
			continue
		}
		s := &steps[o.StepIndex]
		if s.Kind == StepKindInput {
			continue
		}
		base := lowerFirst(o.FieldName)
		if base == "" || base == s.VarName {
			continue
		}
		renameInfos[o.StepIndex] = renameInfo{
			base:      base,
			lastIndex: i,
		}
	}

	for stepIdx := range renameInfos {
		delete(used, steps[stepIdx].VarName)
	}

	for i, o := range outputs {
		info, ok := renameInfos[o.StepIndex]
		if !ok || info.lastIndex != i {
			continue
		}
		s := &steps[o.StepIndex]
		pick := info.base
		if used[pick] {
			for i := 2; ; i++ {
				try := fmt.Sprintf("%s%d", info.base, i)
				if !used[try] {
					pick = try
					break
				}
			}
		}
		s.VarName = pick
		used[pick] = true
	}
}

[mickamy]

Applied in 4d4e52a (slightly simpler shape than the suggestion — no lastIndex tracking, since each step appears in outputs at most once given how outputs is built):

var renames []pendingRename
for _, o := range outputs {
    // …skip irrelevant outputs…
    delete(used, s.VarName)
    renames = append(renames, pendingRename{stepIdx: o.StepIndex, base: base})
}
for _, r := range renames {
    pick := r.base
    if used[pick] { /* suffix loop */ }
    steps[r.stepIdx].VarName = pick
    used[pick] = true
}

Pass 1 vacates every old name first, pass 2 picks the new ones — so a swap (foo/db → db/foo) lands cleanly instead of producing foo2. Covered by TestBuild_FieldNameSwapNoUnnecessarySuffix.

[gemini-code-assist]

If a provider returns a type alias, deriveInputName will fail to recognize it as a named type (since it is represented as a *types.Alias in Go 1.22+), causing it to fall back to the function name. Resolving the alias using types.Unalias before deriving the name ensures consistent naming for type aliases.

Suggested change

	base := deriveInputName(p.Result)
	base := deriveInputName(types.Unalias(p.Result))

[mickamy]

Applied in 4d4e52a. Folded types.Unalias into deriveInputName itself so both call sites (buildInputs for arg-field naming and varNameForProvider for step naming) get the alias resolution for free:

func deriveInputName(t types.Type) string {
    if t == nil { return "arg" }
    t = types.Unalias(t)
    // …Pointer / Named handling as before…
}

Covered by TestBuild_TypeAliasResultDerivesAliasName, which sets up type Alias = Real; func New() Alias and verifies the intermediate variable lands on real rather than falling through to the function-name fallback.

[mickamy]

/gemini review

[gemini-code-assist]

Code Review

This pull request refactors variable naming in the dependency injection planner by naming variables after their produced types rather than constructor function names, resolving Go 1.22+ type aliases, and renaming steps based on their destination field names. The reviewer identified an issue in the renaming logic where multiple container fields bound to the same shared dependency can cause duplicate entries in the renames slice, which leaks unused names into the used map and results in unnecessary suffixing for other steps. A code suggestion was provided to deduplicate these renames.

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.

[gemini-code-assist]

When multiple container fields are bound to the same shared dependency (which resolves to a single step), outputs will contain multiple entries pointing to the same StepIndex.

In the current implementation, this causes the step to be added to renames multiple times with different base names (e.g., foo and bar). During the renaming loop, the step's VarName is overwritten multiple times (the last one wins), but all intermediate candidate names are marked as used in the used map.

This leaks unused names (like foo) into the used map, which can cause other unrelated steps that naturally want those names to be unnecessarily suffixed (e.g., forced to use foo2).

We can fix this by deduplicating the renames per StepIndex while preserving the deterministic order of renaming.

	renameOrder := make([]int, 0, len(outputs))
	renames := make(map[int]string)
	for _, o := range outputs {
		if o.StepIndex < 0 || o.StepIndex >= len(steps) {
			continue
		}
		s := &steps[o.StepIndex]
		if s.Kind == StepKindInput {
			continue
		}
		base := lowerFirst(o.FieldName)
		if base == "" || base == s.VarName {
			continue
		}
		if _, exists := renames[o.StepIndex]; !exists {
			delete(used, s.VarName)
			renames[o.StepIndex] = base
			renameOrder = append(renameOrder, o.StepIndex)
		}
	}

	for _, stepIdx := range renameOrder {
		base := renames[stepIdx]
		pick := base
		if used[pick] {
			for i := 2; ; i++ {
				try := fmt.Sprintf("%s%d", base, i)
				if !used[try] {
					pick = try
					break
				}
			}
		}
		steps[stepIdx].VarName = pick
		used[pick] = true
	}

[mickamy]

Applied in 87e5f01, with a small extension over the suggested form. Same core fix — only the first valid candidate for a given StepIndex is queued, so a step shared across two output fields no longer leaks its dropped candidate name into used.

The extra wrinkle: the gate also catches the "first base already matches the existing VarName" case, so a shared step whose current name aligns with the first field is left alone instead of being renamed to a different field's name on the second visit. Example:

type Container struct {
    DB     *DB `inject:""`   // step current "db" — already aligned
    Backup *DB `inject:""`   // would otherwise queue "backup" and rename
}

Without the gate, the variable ends up as backup, and the struct literal reads DB: backup, Backup: backup. With the gate, the variable stays db and aligns with at least the first field.

Two regression tests added: TestBuild_SharedStepDoesNotLeakCandidate (the leak you described — extra Twin *DB field, makes sure an unrelated Backup step still gets its natural name) and TestBuild_SharedStepPreservesMatchingFieldName (the matching-base case above).

[mickamy]

/gemini review

[gemini-code-assist]

Code Review

This pull request improves variable naming in the generated dependency injection code. It introduces a renaming pass (renameOutputSteps) that renames steps producing container outputs to match their destination field names, resolving name swaps cleanly without unnecessary suffixing. Additionally, it updates provider variable naming to derive from the produced type rather than the constructor function name, and adds support for Go 1.22+ type aliases using types.Unalias. Corresponding updates have been made to the generated example files, and comprehensive unit tests have been added to verify these behaviors. There are no review comments to evaluate, and I have no additional feedback to provide.

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.