internal/cmd/nixopts/nixopts.go (3)

22-30: Remove commented-out code.

Dead code should be removed rather than left commented out. It's tracked in version control if needed later.

---

785-818: CollectFlags silently drops fields when two fields share the same type.

The doc comment warns about this, but a silent overwrite in a map[typeName] is easy to miss during future maintenance. Consider returning an error or panicking (consistent with the other precondition checks in this function) when a duplicate type name is detected, so misuse is caught immediately rather than producing subtly wrong argument lists.

Proposed defensive check

 	for i := 0; i < rv.NumField(); i++ {
 		f := rv.Field(i)
 
 		if flag, ok := f.Addr().Interface().(NixOption); ok {
 			name := f.Type().Name()
+			if _, exists := fieldMap[name]; exists {
+				panic("CollectFlags: duplicate NixOption type " + name)
+			}
 			fieldMap[name] = flag
 		}
 	}

---

524-530: Args() for slice/map-based options returns an empty slice instead of nil when unset.

Bool/int/string options return nil when unset, but Include, Option, UpdateInput, and OverrideInput return []string{}. This is functionally harmless (appending an empty slice is a no-op), but the inconsistency could cause surprises in tests using reflect.DeepEqual. Consider guarding with an early nil/length check:

Example for Include

 func (i Include) Args() []string {
+	if len(i) == 0 {
+		return nil
+	}
 	args := []string{}
 	for _, include := range i {
 		args = append(args, "--include", include)
 	}
 	return args
 }

Same pattern applies to Option.Args(), UpdateInput.Args(), and OverrideInput.Args().

cmd/apply/apply.go (1)

187-187: Completion closure captures Include before flags are parsed.

opts.NixOptions.Include is nil at command construction time. Since CompleteSpecialisationFlagFromConfig receives the slice by value, the completion function will always see a nil includes list regardless of what the user passes via --include. The same issue likely existed with the old Includes field, but worth noting if accurate completions depend on user-supplied includes.

If completions need runtime include values, consider passing a closure or pointer that defers the read until completion time.

internal/configuration/flake.go (1)

184-190: Remove dead code block that attempts to move --impure flag from realiseArgs to evalArgs.

Since Impure.Supports(CmdStoreRealise) returns false (per internal/cmd/nixopts/nixopts.go), the --impure flag is never added to realiseArgs in the first place. This makes slices.Index(realiseArgs, "--impure") always return -1, rendering the entire conditional block unreachable. The flag is already correctly included in evalArgs via ArgsForCommand(CmdEval) since Impure.Supports(CmdEval) returns true.

♻️ Proposed fix

-	// --impure must be part of the eval arguments, rather than the
-	// realisation arguments here, since that is where environment
-	// variables are accessed.
-	if i := slices.Index(realiseArgs, "--impure"); i >= 0 {
-		evalArgs = append(evalArgs, "--impure")
-		realiseArgs = slices.Delete(realiseArgs, i, i+1)
-	}