Skip to content

openapi: give each lowering construct its own file #1062

Closed
11 of 11 issues completed
Closed

Description

@OmarAlJarrah

The goal and context come first, then the ordered plan. Each step is a sub-issue, and the blocked-by relations are set on the issues themselves.

End goal

In compilers/openapi, each construct a spec author would name is lowered in a file named for it: allOf, unions, discriminators, enums, responses, path items, headers and servers. Its tests sit in that file's _test.go or _internal_test.go.

Packages stay the only enforced boundary. No declaration crosses a package, and no behaviour, IR, diagnostic or golden changes.

Context

Several files hold four or five constructs back to back, so finding where one is lowered means searching:

  • internal/schema/compose.go (1,588 lines) holds allOf, oneOf/anyOf, discriminators and enums in sequence.
  • internal/schema/schema.go (2,420 lines) holds dispatch beside the keyword census, models, scalars, $dynamicRef and nullability.
  • internal/operation/operations.go (1,378) and content.go (1,142) hold five constructs each.
  • internal/load/load.go (1,150) and internal/annotation/annotation.go (971) mix concerns their siblings already keep apart.
  • allOf's tests are split across two files: 39 TestAllOf_* in compose_test.go and 16 in schema_test.go.
git ls-files 'compilers/openapi/**/*.go' | grep -v _test.go | xargs wc -l | sort -rn | head -8
grep -c '^func TestAllOf_' compilers/openapi/internal/schema/{compose,schema}_test.go

This is about finding code, not isolating it. A Go file does not isolate anything, which is why docs/micro-compiler-design.md §1 made packages the boundary, and that stays.

Rules every step follows

  • One construct per file. A file holding several is split. A file that is a private fragment of another construct's file merges into it: paramledger.go into params.go, discriminated.go into discriminator.go, enummember.go into enum.go.
  • A declaration lives with the responsibility it serves, not with whoever calls it. A helper that serves one construct or concern sits in that file, however small the file ends up. Entry points, exported API and readers several constructs share stay with their own construct.
  • Tests follow their code. A scenario-named test file that belongs to one construct merges into that construct's test file.
  • Package comments go to doc.go. A package whose comment sits in a file being split gets a doc.go.
  • Each PR is a pure move. Every top-level declaration, with its doc comment and any comment between declarations, is byte-identical before and after, and no golden changes. Comments that name a file are pointed at the file the code moved to, in a commit of their own.
  • Busy files wait. A file an open PR touches is not split until that PR merges.

Plan

1. Files no open PR touches

2. After #1046

3. After #998 and #1000

4. Record the layout

Out of scope

  • compilers/compile, and packages whose files each hold one concern already (bind, overlay, lowering, nodeview, ids, value, diag) are left as they are.
  • Scenario-named test files in load, bind, canary, merge, resolve and lowering are an established practice there.
  • No new checks.
    • A file-length cap would split a construct that is genuinely large at an arbitrary line.
    • A rule pairing every test file with a source file would fight the scenario-named files above.
  • Moving packages. Server lowering moving beneath internal/operation belongs to ir: decide whether four no_ir_home operation constructs get real fields #285.

Priority: P4, rule 1 (housekeeping; every step is a refactor that changes no lowering, validation, IR or emit-contract behaviour, and the tracker takes the tier of its highest open step). wc -l compilers/openapi/internal/schema/schema.go → 2420 (morphic @a1f15b29).

Activity

  1. added
    type:choreRefactor or cleanup, no behavior change
    priority:P4Refactors, tests, CI, hostile-input hardening, cosmetic diagnostics, other formats
    on Oct 21, 2026
  2. OmarAlJarrah commented on Oct 22, 2026

    @OmarAlJarrah
    MemberAuthor

    This widens the scope to three packages the out-of-scope note kept whole on the grounds that each of their files holds one concern. At ff8330ba, after #1000 and #1084, they do not:

    The rules above apply, with one change: a declaration now lives with the responsibility it serves, rather than with the file of whoever calls it, and a file holding one or two functions is fine. The others are unchanged: pure moves, and busy files wait. The three are sub-issues here; they run before #1059 and #1060, and #1061 waits for them. bind, overlay, lowering, nodeview, ids, value and diag stay out of scope. The out-of-scope note in the body is edited to match.

    This tracker is now a step of #1096, which also holds the same rule for pass and the moves out of the root package.

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

    priority:P4Refactors, tests, CI, hostile-input hardening, cosmetic diagnostics, other formatstype:choreRefactor or cleanup, no behavior change

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions