Simplify typed input preparation

This commit is contained in:
2026-11-07 15:59:28 +00:00
parent b66a00da26
commit c4b5bcdbc2
6 changed files with 61 additions and 47 deletions

View File

@@ -239,17 +239,6 @@ func (p preparedInput[I]) build() I {
return input
}
// prepareInput parses all supplied bindings into typed closures before it
// creates an input value. When enforceRequired is false, missing required flags
// and operands are allowed so metadata help can validate supplied syntax.
func prepareInput[I any](spec CommandSpec[I], flagValues [][]string, operands []string, enforceRequired bool) (preparedInput[I], error) {
return prepareInputWithSpellings(spec, flagValues, nil, operands, enforceRequired)
}
func prepareInputWithSpellings[I any](spec CommandSpec[I], flagValues, flagSpellings [][]string, operands []string, enforceRequired bool) (preparedInput[I], error) {
return prepareInputWithOccurrences(spec, flagValues, flagSpellings, nil, operands, enforceRequired)
}
func prepareInputWithOccurrences[I any](spec CommandSpec[I], flagValues, flagSpellings [][]string, occurrences []commandmeta.FlagOccurrence, operands []string, enforceRequired bool) (preparedInput[I], error) {
if len(flagValues) != len(spec.Flags) {
return preparedInput[I]{}, fmt.Errorf("received %d flag value sets for %d bindings", len(flagValues), len(spec.Flags))
@@ -272,18 +261,7 @@ func prepareInputWithOccurrences[I any](spec CommandSpec[I], flagValues, flagSpe
binding := flag.binding
value, err := binding.prepare(flagValues[i])
if err != nil {
var occurrenceErr *flagOccurrenceError
if errors.As(err, &occurrenceErr) {
spelling := "--" + binding.meta.Name
if occurrenceErr.index >= 0 && occurrenceErr.index < len(flagSpellings[i]) && flagSpellings[i][occurrenceErr.index] != "" {
spelling = flagSpellings[i][occurrenceErr.index]
}
return preparedInput[I]{}, &flagConversionError{
binding: binding.meta.Name, spelling: spelling,
occurrence: occurrenceErr.index + 1, cause: occurrenceErr.cause,
}
}
return preparedInput[I]{}, fmt.Errorf("flag --%s: %w", binding.meta.Name, err)
return preparedInput[I]{}, attributeFlagConversionError(binding.meta.Name, flagSpellings[i], err)
}
preparedFlags[i] = value
for index, occurrence := range value.ordered {
@@ -315,7 +293,7 @@ func prepareInputWithOccurrences[I any](spec CommandSpec[I], flagValues, flagSpe
if missingRequired != "" {
return preparedInput[I]{}, fmt.Errorf("required flag --%s was not supplied", missingRequired)
}
argumentValues, err := allocateOperands(operands, spec.Arguments, enforceRequired)
arguments, err := prepareOperands(operands, spec.Arguments, enforceRequired)
if err != nil {
return preparedInput[I]{}, err
}
@@ -327,17 +305,38 @@ func prepareInputWithOccurrences[I any](spec CommandSpec[I], flagValues, flagSpe
preparedValues = append(preparedValues, preparedValue[I]{apply: occurrence.apply})
}
}
for i, argument := range spec.Arguments {
binding := argument.binding
value, err := binding.prepare(argumentValues[i])
if err != nil {
return preparedInput[I]{}, fmt.Errorf("argument %q: %w", binding.meta.Name, err)
}
preparedValues = append(preparedValues, value)
}
preparedValues = append(preparedValues, arguments...)
return preparedInput[I]{newInput: spec.NewInput, values: preparedValues}, nil
}
func attributeFlagConversionError(name string, spellings []string, err error) error {
var occurrenceErr *flagOccurrenceError
if errors.As(err, &occurrenceErr) {
spelling := "--" + name
if occurrenceErr.index >= 0 && occurrenceErr.index < len(spellings) && spellings[occurrenceErr.index] != "" {
spelling = spellings[occurrenceErr.index]
}
return &flagConversionError{binding: name, spelling: spelling, occurrence: occurrenceErr.index + 1, cause: occurrenceErr.cause}
}
return fmt.Errorf("flag --%s: %w", name, err)
}
func prepareOperands[I any](operands []string, definitions []Argument[I], enforceRequired bool) ([]preparedValue[I], error) {
values, err := allocateOperands(operands, definitions, enforceRequired)
if err != nil {
return nil, err
}
prepared := make([]preparedValue[I], 0, len(definitions))
for i, argument := range definitions {
value, err := argument.binding.prepare(values[i])
if err != nil {
return nil, fmt.Errorf("argument %q: %w", argument.binding.meta.Name, err)
}
prepared = append(prepared, value)
}
return prepared, nil
}
func newBindingIdentity() *bindingIdentity { return &bindingIdentity{marker: 1} }
func allocateOperands[I any](operands []string, definitions []Argument[I], enforceRequired bool) ([][]string, error) {

View File

@@ -36,7 +36,7 @@ func TestScalarBindingsKeepDefaultsSeparateFromPresence(t *testing.T) {
t.Fatal(err)
}
defaultMessage = "changed after declaration"
prepared, err := prepareInput(spec, [][]string{nil, nil, nil}, nil, true)
prepared, err := prepareInputWithOccurrences(spec, [][]string{nil, nil, nil}, nil, nil, nil, true)
if err != nil {
t.Fatal(err)
}
@@ -60,7 +60,7 @@ func TestScalarBindingsKeepDefaultsSeparateFromPresence(t *testing.T) {
t.Fatalf("NewInput runtime value = %q, want caller value", input.runtime)
}
prepared, err = prepareInput(spec, [][]string{{"", "last"}, {"0"}, {"true", "false"}}, nil, true)
prepared, err = prepareInputWithOccurrences(spec, [][]string{{"", "last"}, {"0"}, {"true", "false"}}, nil, nil, nil, true)
if err != nil {
t.Fatal(err)
}
@@ -75,7 +75,7 @@ func TestScalarBindingsKeepDefaultsSeparateFromPresence(t *testing.T) {
t.Fatalf("enabled = %#v, want explicit false", input.enabled)
}
prepared, err = prepareInput(spec, [][]string{{""}, nil, nil}, nil, true)
prepared, err = prepareInputWithOccurrences(spec, [][]string{{""}, nil, nil}, nil, nil, nil, true)
if err != nil {
t.Fatal(err)
}
@@ -83,7 +83,7 @@ func TestScalarBindingsKeepDefaultsSeparateFromPresence(t *testing.T) {
if input.message.Value != "" || !input.message.Set {
t.Fatalf("message = %#v, want explicitly supplied empty string", input.message)
}
prepared, err = prepareInput(spec, [][]string{nil, nil, nil}, nil, true)
prepared, err = prepareInputWithOccurrences(spec, [][]string{nil, nil, nil}, nil, nil, nil, true)
if err != nil {
t.Fatal(err)
}
@@ -207,7 +207,7 @@ func TestRepeatArgumentAllocationReservesLaterMinimums(t *testing.T) {
if err := validateCommandDeclaration(NewCommand(spec)); err != nil {
t.Fatal(err)
}
prepared, err := prepareInput(spec, nil, []string{"a", "b", "id-1", "x", "y"}, true)
prepared, err := prepareInputWithOccurrences(spec, nil, nil, nil, []string{"a", "b", "id-1", "x", "y"}, true)
if err != nil {
t.Fatal(err)
}
@@ -237,10 +237,10 @@ func TestIncompletePreparationSkipsMissingRequirementsAndInputFactory(t *testing
if err := validateCommandDeclaration(NewCommand(spec)); err != nil {
t.Fatal(err)
}
if _, err := prepareInput(spec, [][]string{nil}, nil, true); err == nil {
if _, err := prepareInputWithOccurrences(spec, [][]string{nil}, nil, nil, nil, true); err == nil {
t.Fatal("strict preparation accepted missing requirements")
}
if _, err := prepareInput(spec, [][]string{nil}, nil, false); err != nil {
if _, err := prepareInputWithOccurrences(spec, [][]string{nil}, nil, nil, nil, false); err != nil {
t.Fatalf("help preparation rejected omitted execution inputs: %v", err)
}
if factoryCalls != 0 {

View File

@@ -1,6 +1,6 @@
# Consolidated audit catalog
Status: **6 distinct open findings: 6 P3**, consolidated from 30
Status: **4 distinct open findings: 4 P3**, consolidated from 30
numbered entries in seven audit documents. Reviewed audit state:
`db732e3a1de55c844f77870e1c6db683c2685d5d`.
@@ -412,6 +412,10 @@ their purpose; no generic operation engine or rule registry is selected.
## F19 — P3: Typed input preparation mixes conversion, provenance, and replay assembly
**Complete.** Conversion-error attribution and operand preparation are extracted
without replacing flag ordering or replay. Focused conversion, occurrence,
capture and replay tests pass.
`prepareInputWithOccurrences` handles shape validation, conversion-error
spelling, occurrence identities, application order, required flags, operands,
and prepared callbacks. Captured binding replay depends on those shared
@@ -515,6 +519,10 @@ snapshot errors and operator-input validation.
## F24 — P3: Obsolete private production entry points remain for tests alone
**Complete.** Reporter selection/finalization, routed parsing and occurrence-aware
preparation replace the obsolete wrappers. Graph and text checks confirm their
removal; useful lifecycle and replay tests pass.
Legacy reporter setters/close, `App.parse`, and typed-preparation wrappers have
only test callers, while production uses current selection, finalization,
routed-session, and occurrence-aware paths. Some tests therefore exercise

View File

@@ -1,6 +1,6 @@
# Audit remediation implementation plan
Status: in progress; Stages 1–19 complete.
Status: in progress; Stages 1–20 complete.
Baseline: `d492c7747175c01a53261206c2b4356e53453621`. Before this sprint,
intervening changes recorded decisions and retired the completed older
@@ -629,6 +629,13 @@ Complete when each capture responsibility has a clear place and behavior holds.
## Stage 20 — Simplify typed preparation and retire remaining test wrappers
**Complete.** Conversion-error attribution and operand preparation are separate
private responsibilities (F19); obsolete reporter, parser and binding wrappers
are removed and test-only observation lives in tests (F24). Graph discovery and
text references confirmed the current occurrence-aware callers and absence of
the retired entry points. Focused binding, replay, occurrence, conversion, routed
parsing and reporter tests passed offline with Go 1.27.0 and `-mod=readonly`.
Findings: F19 and completion of F24. Sources: `binding.go`, `flag_values.go`,
`parser.go` and related tests; review reporter cleanup from stage 3 as well.

View File

@@ -639,12 +639,6 @@ func (state *reportingState) presentationLocked() PresentationResult {
}
}
func (state *reportingState) presentation() PresentationResult {
state.mu.Lock()
defer state.mu.Unlock()
return state.presentationLocked()
}
func (state *reportingState) deliveryResultLocked(destination reportingDestination) DeliveryResult {
current := state.destination(destination)
result := DeliveryResult{RenderError: current.renderError, WriteError: current.writeError}

View File

@@ -747,3 +747,9 @@ func TestReporterFinalizationReleasesBorrowedDestinations(t *testing.T) {
})
}
}
func (state *reportingState) presentation() PresentationResult {
state.mu.Lock()
defer state.mu.Unlock()
return state.presentationLocked()
}