Skip to content

Delete go_sum_edit's oracle-only free functions and move the go.sum codec to formats #631

Description

[agent] Filed by the scheduled architecture audit routine (ecosystems and formats). Register: discussion #560 register.

Kind: refactor. Source: new finding (register E52). It is an instance of review 3.7 #9 / E35 (refactor oracles kept in production) and of 4.7 J / E20 (pure codecs outside formats/).

Problem

vendor/go_sum_edit.rs carries two implementations of the same three go.sum edits. Verified on 045d7ec:

Edit Free pub fn (no production caller) GoSumEditor method (used by hosted rewrite_golang)
upsert zip + /go.mod lines upsert_module_lines GoSumEditor::upsert_module_lines
presence check has_module_version GoSumEditor::has_module_version
exact prune remove_exact_module_version_lines GoSumEditor::remove_exact_module_version_lines
  • No production callers. grep -rn over the workspace, including socket-patch-node, finds the free functions only in this file's tests. The only production user is GoSumEditor (redirect/mod.rs#L6714, #L6874, #L6932, #L6950).
  • A test oracle in production code. The free functions survive as the oracle for editor_matches_the_text_transforms_step_by_step (L686): ~90 production lines kept in sync by hand, with the key-matching rule ("{module} {version} " / "{module} {version}/go.mod ") spelled six times.
  • A codec in the wrong module. The file is a pure hosted codec ("Pure go.sum line edits for the hosted Go redirect"), but it lives in vendor/. patch/redirect (mod.rs, upstream/golang.rs, upstream/client.rs), vex::discover::golang and lock_inventory::golang all import it from there.

Symptoms and impact

I found no open bugs. Today any change to the upsert rule, such as #509's /go.mod-only line question, has to be made twice and kept equal through the oracle test. The risk is low.

Proposed change

  1. Move the file to formats/golang/sum.rs (mechanical, its own commit): GoSumEditor, go_sum_lines, is_h1_dirhash, reinsert_lines, remove_lines, remove_module_prefix_lines, go_sum_line_cmp. Re-export it from vendor::go_sum_edit for one release only if that's needed.
  2. Delete the free upsert_module_lines, has_module_version and remove_exact_module_version_lines. Retarget their unit tests at GoSumEditor (feed new(text), then compare into_string()), and replace the step-by-step oracle test with fixed expected outputs.
  3. Name the key rule once: fn is_version_line(line, module, version) -> bool.

Size and scope

About 100 production lines deleted, plus about 750 moved. It touches vendor/go_sum_edit.rs (moved), vendor/mod.rs, formats/mod.rs and five import sites. Out of scope: go_mod_edit.rs (E19) and the hosted Go rewriter's logic.

Acceptance criteria

  • go.sum editing has one implementation (GoSumEditor) under formats/. Nothing in patch/, vex/ or lock_inventory/ imports vendor::go_sum_edit.
  • The upsert, idempotence, stale-line, sort-order, CRLF and bare-\r cases from the current tests pass against the editor.
  • The hosted Go redirect tests (cargo test -p socket-patch-core --lib golang) and the e2e Go suites stay green.

Dependencies

None. It is related to E20 (codecs into formats/), and this is that row's first go child.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    agent:triagedarch-auditFiled by a scheduled architecture audit routine (see the architecture review discussion)pm:goGo modulespriority:p2refactorStructural change: duplicated code or logic, missing abstraction, layering, dead code

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions