Skip to content

pass: split validate.go into one file per rule family #1090

Closed

Description

@OmarAlJarrah

Problem

pass/validate.go (1,012 lines) holds every check Validate runs. That is 18 calls from Validate into 13 unrelated rule families, so finding where one rule lives means searching one file. Its tests are already split by family: validate_aliases_test.go, validate_default_test.go, validate_encoding_test.go, validate_idrefs_test.go, validate_indices_test.go, validate_messages_test.go, validate_propids_test.go, validate_refs_test.go and validate_typednil_test.go, beside the cross-cutting corpus, edge-case, scaling and bench files.

wc -l pass/validate.go
awk '/^func Validate\(/,/^}/' pass/validate.go | grep -c 'diags = append(diags, check'

Direction

One file per family. Each declaration moves with the rule it serves, and a file holding one or two functions is fine.

File Holds
validate.go Validate, diag, and liveTypeIDs, which four families call from five sites
refs.go gains checkDanglingRefs and checkPropIDRefs beside the collectors it already holds
types.go checkNilTypes
groups.go checkGroupWalkTruncated
indices.go checkServerIndices, appendServerIndexDiags, checkResponseIndices, appendSuccessStatusDiags
payloads.go payloadSite, checkEncodingKeys, checkPayloadRequired, forEachPayload, forEachErrorPayload, appendEncodingKeyDiags, appendUnknownPartDiags
discriminators.go checkDiscriminators, checkUnionDiscriminator, checkModelDiscriminator, checkDiscriminatorProperty, checkMapping, mappingTarget
wirenames.go checkDuplicateWireNames, checkAmbiguousAliases, ambiguousAliases, effectiveWireName
enummembers.go checkDuplicateMemberIDs
params.go checkParams, declaredParamIDs, danglingWalkSkips, checkParamBindings, paramLocation, checkHTTPParamBinding, unboundParamWarnings, paramRef, operationParamRefs, checkParamReferences
idempotency.go checkIdempotency
messages.go checkMessageBindings, checkBoundMessages, appendUncarriedMessageDiags, checkOneWay
responses.go checkDefaultResponses, statusNames, answersCatchAllOnly
graphql.go checkArgsOutsideGraphQL, graphqlReachableTypes, appendTypeIDs

The test files keep their names, which already name the families.

Done when

  • Every top-level declaration of the package, with its doc comment and any comment between declarations, is byte-identical before and after the move (compared with a throwaway go/ast script), and no golden changes.
  • Comments that name a file name the file the code now lives in, changed in a commit of their own: pass/refs.go points at validate.go for the reachability analysis (then graphql.go) and the integer-index checks (then indices.go).
  • go test ./pass/... passes without -update.

Priority: P4, rule 1 (housekeeping: moves code between files and changes no validation or IR behaviour). wc -l pass/validate.go → 1012 (morphic @ff8330ba).

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