tracking: one home for each rule and one responsibility per file #1096

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. Steps that #976 already tracks are named here and stay its sub-issues.

End goal

A contributor finds a rule in one place, in a file named for what it does, and two copies of a rule can no longer disagree. Done means all of:

• every rule below that is written more than once has one definition, and the defects its copies caused are closed;
• every production file in pass and compilers/openapi holds one responsibility, and a file holding one or two functions is fine;
• the compiler's root package holds its public face and nothing else;
• no package re-exports another package's call under a second name.

Context

Measured at ff8330ba.

• The copies are rules, not text. A duplicate-code scan of the production code (golangci-lint's dupl linter alone, threshold 50, tests excluded) finds seven short repeats, each inside one file. What costs is a rule decided in several places:
  • how a mapping's pairs and keys are read, which fifteen sites each decide (openapi: raw mapping readers disagree with the parser on merge keys and alias keys #1089);
  • how a YAML scalar is classified (openapi: one classifier for YAML scalars, shared by value lowering and raw preservation #784);
  • the decimal grammar (ir: one decimal-literal grammar for BigVal, bound comparison and YAML integers #786);
  • which fields declare an ID (ir: the rule for which struct fields declare an ID is written in ir and again in irverify, and no test runs both #984);
  • the inferred-marker list (openapi: share one implementation of the inferred-marker list #1071).
• Files hold several responsibilities. One way to find them is to group a file's declarations by the calls, receivers and type uses that link them, treating a helper that four or more of them call as shared. A file that falls into several groups is a candidate, and reading it settles the matter. Some candidates are fine as they are: ir/types.go groups by sum type, and operation/callbacks.go holds callbacks beside webhooks. Read that way, the files each step below names hold the responsibilities that step lists. openapi: give each lowering construct its own file #1062 already covers schema.go, compose.go and content.go.
• The root package holds more than its face. meta.go lowers document-level metadata, and most of detect.go is how detection reads bytes (openapi: move document lowering and detection's byte reading out of the root package #1091).
• One package re-exports another's calls. schema/accumulate.go forwards three annotation calls under names of its own. Other packages make the same three calls directly at ten sites (openapi: schema re-exports three annotation calls that operation can make itself #1092).
• The largest single simplification left is openapi: name every node from one function of the document and the coordinate, and remove the provisional-name machinery #981's: naming every node from one function and removing the provisional-name machinery.

Ordering

1. Behaviour changes that close defects come before moves.
2. Within a package, semantic work already in flight goes first. A move is scripted (the go/ast mover refactor(compilers/openapi): split operations.go and load.go #1076 used) and re-run on the new code rather than rebased across it.
3. Moves land one pull request per package. A move within a package is proved by byte-identical declarations and no golden change. A move across packages is proved by unchanged goldens, harness sweep and diagnostics.
4. An open pull request holds back the moves of its files only while it is moving. fix(compilers/openapi): flatten a nested inline allOf #998 holds openapi: split content.go by construct #1059 and openapi: split schema lowering into one file per construct #1060. That is stated in prose, since a pull request cannot be a dependency target.

Plan

1. One home for each rule (changes behaviour)

• openapi: raw mapping readers disagree with the parser on merge keys and alias keys #1089: one reading of a mapping's pairs, designed with openapi: raw YAML tree walks each carry their own stack, depth cap and alias rule #788. It fixes two unfiled defects and openapi: raw-node readers miss keys merged in through a << #395, openapi: a preserved field supplied through a merge key or alias is dropped with no trace #384 and openapi: a verbatim entry keeps the first copy of a key repeated through an alias #1072, and openapi: a section entry whose key is no scalar is dropped without a diagnostic #947's fix lands once.
• openapi: one classifier for YAML scalars, shared by value lowering and raw preservation #784, ir: one decimal-literal grammar for BigVal, bound comparison and YAML integers #786 and openapi: share one implementation of the inferred-marker list #1071; ir: the rule for which struct fields declare an ID is written in ir and again in irverify, and no test runs both #984 is tracking: a simpler architecture and contributor path for ir, pass, engine and the CLI #976's.
• openapi: derive a node's name from its coordinate in one function, and diff it against the names the lowering gives #980, then openapi: name every node from one function of the document and the coordinate, and remove the provisional-name machinery #981 with openapi: the $dynamicAnchor memo is a parameter of 72 functions and is read by two #982 (tracking: a simpler architecture and contributor path for ir, pass, engine and the CLI #976's): naming from one function.
• irverify: run every walking check from one traversal of the document #962 with ir: build WalkValues paths only when a visitor reads them #813 (tracking: a simpler architecture and contributor path for ir, pass, engine and the CLI #976's): irverify in one traversal.
• openapi: the compile's work and size bounds are written as a dozen separate counters #785 and openapi: raw YAML tree walks each carry their own stack, depth cap and alias rule #788; openapi: six hand-written panic barriers share one shape and no output bound #787 is tracking: a simpler architecture and contributor path for ir, pass, engine and the CLI #976's.

2. One responsibility per file (moves)

• pass: split validate.go into one file per rule family #1090: pass/validate.go by rule family.
• openapi: give each lowering construct its own file #1062, the layout tracker for compilers/openapi: openapi: split merge.go and admits.go by responsibility #1095 (merge), openapi: split resolve's entry.go and resolve.go by responsibility #1094 (resolve) and openapi: split auth.go into security requirements, schemes and OAuth flows #1093 (auth) now, then openapi: split content.go by construct #1059 and openapi: split schema lowering into one file per construct #1060, then docs: say which file a lowering rule's code goes in #1061.
• openapi: move document lowering and detection's byte reading out of the root package #1091: the root package's face.
• openapi: schema re-exports three annotation calls that operation can make itself #1092: the re-exports in schema/accumulate.go, before openapi: split content.go by construct #1059 and openapi: split schema lowering into one file per construct #1060.

3. Small

• openapi+irfacts: functions signal none or unbounded with an in-band -1 #1088: functions that return an in-band -1.
• openapi: Scope.DeclaredAt, Scope.ModelAt and loops.into are called only from tests #983 (tracking: a simpler architecture and contributor path for ir, pass, engine and the CLI #976's) gains fragment.Of and oas.Kinds, two more exports only tests call.

Out of scope

• Look-alikes kept apart on purpose:
  • ir's subtype and exposure definitions beside the irfacts indexes over them (ir-design §4.9);
  • pass.Validate beside irverify, which report through separate channels;
  • the URI check in oas/format.go beside the one in merge/admits.go, which answer different questions;
  • the per-package copies of test helpers under compilers/openapi, which openapitest's allowlist forces;
  • the conformance goldens beside each conformance case's focused assertion (conformanceCases).
• Bundling lowering.Ctx, *compile.Types, the anchor memo and depth into one struct, which openapi: the $dynamicAnchor memo is a parameter of 72 functions and is read by two #982 rules out.
• A cohesion check in CI. The grouping above flags files that are fine, and openapi: give each lowering construct its own file #1062 rules out new checks.

Done check

• Every pair read openapi: raw mapping readers disagree with the parser on merge keys and alias keys #1089 lists asks nodeview or carries a comment saying why it reads raw, and git grep -nE '[ij] \+= 2' -- 'compilers/openapi/*.go' ':!*_test.go' finds no new one without such a comment.
• ls compilers/openapi/*.go (tests aside) lists only doc.go, openapi.go, options.go and detect.go.
• git grep -nE 'func (PreserveNode|ExtensionsOf|ExtensionsIn)\(' -- compilers/openapi/internal/schema finds nothing.
• Every step above is closed, and openapi: give each lowering construct its own file #1062's out-of-scope note no longer lists merge, resolve or auth.

Priority: P3, the tier of its highest open steps (#980 and #981, which #976 also tracks; #813).