Skip to content

checker: privatize CheckModifiedPropertiesDiff and friends once walker migration completes #953

@reuvenharrison

Description

@reuvenharrison

Follow-up to #936 (walker migration). Surfaced while reviewing the still-exported primitives.

Observation

The three Check*PropertiesDiff helpers in checker/checks-utils.go are exported but have zero external callers:

Symbol Defined External callers (checker.X references)
CheckModifiedPropertiesDiff checker/checks-utils.go:68 0
CheckAddedPropertiesDiff checker/checks-utils.go:173 0
CheckDeletedPropertiesDiff checker/checks-utils.go:276 0

Verified with grep -rln "checker\.Check\(Modified\|Added\|Deleted\)PropertiesDiff" across the repo — no hits outside checker/ itself.

Inside checker/, CheckModifiedPropertiesDiff has 30 call sites today, including the walker's own walkProperties helper (media_type_walker.go). As batches of #936 migrate checkers from direct CheckModifiedPropertiesDiff calls to info.walkProperties, the direct-call count shrinks. After the migration completes, the only remaining caller is walkProperties itself, in the same package.

CheckAddedPropertiesDiff and CheckDeletedPropertiesDiff have fewer direct callers (a handful of checkers, e.g. check_response_required_property_updated.go), and the walker does not yet wrap them.

Proposal

A small endgame on top of the existing migration:

  1. Finish migrating the remaining ~23 checkers off direct CheckModifiedPropertiesDiff calls — already in flight as part of checker: extract a media-type walker to dedupe the per-checker traversal prelude #936 (batches PR-5 through PR-9 merged so far; ~25 files left).

  2. Add walkAddedProperties and walkDeletedProperties siblings to media_type_walker.go. Same shape as walkProperties but wrapping the Added / Deleted primitives. Migrate the few checkers that currently need the other two (e.g. check_response_required_property_updated.go).

  3. Rename and unexport the three primitives:

    • CheckModifiedPropertiesDiffcheckModifiedPropertiesDiff
    • CheckAddedPropertiesDiffcheckAddedPropertiesDiff
    • CheckDeletedPropertiesDiffcheckDeletedPropertiesDiff

    Single mechanical commit (one find/replace + go test).

End state

The checker package's exported surface around property walking shrinks to just walkProperties / walkAddedProperties / walkDeletedProperties (which remain unexported, accessed via the mediaTypeInfo receiver), plus the higher-level public entry points (CheckBackwardCompatibility… and the per-rule Check… functions). The primitive Check*PropertiesDiff helpers move out of the public API where they were only ever placeholder exports for now-defunct external callers.

Acceptance

  • All un-migrated checkers use info.walkProperties (or the new walkAdded* / walkDeleted* siblings). Direct Check*PropertiesDiff call sites = 3 (one per primitive, called from the walker only).
  • walkAddedProperties and walkDeletedProperties exist on mediaTypeInfo, mirroring walkProperties.
  • Three primitives renamed to lowercase.
  • Public surface check: grep -rn '^func Check\(Modified\|Added\|Deleted\)PropertiesDiff' checker/*.go returns nothing.
  • Full test suite green; no behavioural change.

Sequencing note

This is a strictly mechanical follow-up that lands cleanly only once the bulk migration is done. Filing now so the cleanup doesn't get forgotten when the last batch of #936 merges.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions