diff --git a/binding.go b/binding.go index af0829e..98be328 100644 --- a/binding.go +++ b/binding.go @@ -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) { diff --git a/binding_test.go b/binding_test.go index 46b6cd9..07a4fa9 100644 --- a/binding_test.go +++ b/binding_test.go @@ -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 { diff --git a/docs/roadmap/audit/catalog.md b/docs/roadmap/audit/catalog.md index de5c4ae..d7d6918 100644 --- a/docs/roadmap/audit/catalog.md +++ b/docs/roadmap/audit/catalog.md @@ -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 diff --git a/docs/roadmap/implementation.md b/docs/roadmap/implementation.md index 210d2fe..0d4a661 100644 --- a/docs/roadmap/implementation.md +++ b/docs/roadmap/implementation.md @@ -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. diff --git a/reporter.go b/reporter.go index 8625baf..ebadab6 100644 --- a/reporter.go +++ b/reporter.go @@ -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} diff --git a/reporter_test.go b/reporter_test.go index a2555e9..8e73aee 100644 --- a/reporter_test.go +++ b/reporter_test.go @@ -747,3 +747,9 @@ func TestReporterFinalizationReleasesBorrowedDestinations(t *testing.T) { }) } } + +func (state *reportingState) presentation() PresentationResult { + state.mu.Lock() + defer state.mu.Unlock() + return state.presentationLocked() +}