feat: source location tracking for all breaking change checks#752
Open
reuvenharrison wants to merge 27 commits intomainfrom
Open
feat: source location tracking for all breaking change checks#752reuvenharrison wants to merge 27 commits intomainfrom
reuvenharrison wants to merge 27 commits intomainfrom
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #752 +/- ##
==========================================
- Coverage 89.55% 89.25% -0.30%
==========================================
Files 239 240 +1
Lines 12154 12627 +473
==========================================
+ Hits 10884 11270 +386
- Misses 840 903 +63
- Partials 430 454 +24
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
46ae1e6 to
8410950
Compare
Replace NewApiChangeWithSources(args..., base, rev) with NewApiChange(args...).WithSources(base, rev). This avoids needing to migrate all ~86 checker functions to a new constructor and eliminates merge conflicts with the OpenAPI 3.1 branch. - Add WithSources() method to ApiChange, ComponentChange, SecurityChange - Add NewSourceFromField() to source.go - Update callers in check_api_added.go and check_api_removed.go - Remove NewApiChangeWithSources constructor Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
8410950 to
e48ff98
Compare
Point to oasdiff/kin-openapi feat/origin-file-tracking branch which adds file paths to origin location data. Enable IncludeOrigin only in tests that check source locations instead of globally. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
NewSourceFromOrigin and NewSourceFromField now use the file path from the origin Location when available, falling back to OperationsSourcesMap only when origin doesn't include a file. This enables precise file tracking for $ref'd schemas from external files. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add .WithSources(baseSource, revisionSource) to every NewApiChange call across all checker files. When origin tracking is enabled, each breaking change now carries precise file/line/column for both base and revision specs. - Add operationSources() helper to compute base/revision sources - Only populate sources when origin data is available (backward compatible) - Update helper functions to return ApiChange for chaining - Add 4 new tests covering all checker patterns (operation, parameter, request property, response property) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Resolve 53 file conflicts: combine WithSources() (source tracking) with WithDetails() (media type/deprecation details) from main. Also add WithSources to new property deprecation checkers from main, and revert openapi3.Ptr to type-specific helpers for fork compatibility. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Rebased feat/origin-file-tracking onto post-Ptr master (45db2ad), so openapi3.Ptr is available. Reverted Uint64Ptr/Float64Ptr workarounds back to openapi3.Ptr. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Add NewSourceFromSequenceItem() for looking up individual items in scalar sequences (e.g. type: [string, null]) by value via Origin.Sequences - Update kin-openapi dependency to pick up Origin.Sequences and Location.Name fields - Add responseSource() helper for sub-operation-level source locations on response status codes - Enable origin tracking by default in calcDiff Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ches Updates all dependencies to use the origin tracking feature branches with sequence origin and Location.Name support. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Replace operation-level operationSources() with finer-grained helpers (SchemaSources, ParameterSources, ResponseSources) across ~70 checker files so reported changes point to the specific schema, parameter, or response rather than the enclosing operation. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Use SchemaFieldSources() instead of SchemaSources()/ParameterSources() for ~45 checkers that detect changes to specific scalar fields (type, format, min, max, pattern, etc.). This points source locations to the exact YAML field line rather than the parent schema/parameter object. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Adds OperationFieldSources and ParameterFieldSources helpers, and upgrades remaining checkers that were still at object-level to use field-level precision: - API-level: deprecated, operationId, tags, security - Schema-level: enum, oneOf/anyOf/allOf, discriminator, required, type - Parameter-level: required - Fixes NewSourceFromField to fall back to key-level instead of file-only Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Standardize nil for the "missing side" in all add/delete checkers (property added → baseSource=nil, property removed → revisionSource=nil) - Fix set checkers (max_set, min_set, etc.) to nil out baseSource - Fix unset checkers (max_length_unset, min_items_unset) to nil out revisionSource - Fix operationId added/removed to nil out the missing side - Fix pattern added/removed to nil out the missing side - Add source tracking to component-level checkers (schema removed, security component added/removed) with new sourceFromOrigin helper - Add Base/Revision fields to SecuritySchemesDiff for origin access - Fix NewSourceFromField: return nil instead of misleading key-level fallback when field doesn't exist; also check origin.Sequences for sequence-valued fields (required, enum, type array) - Fix NewSourceFromSequenceItem: return nil instead of file-only fallback - Simplify SchemaDeletedItemSources/SchemaAddedItemSources to use nil for the missing side - Upgrade request parameter enum value checker to per-item precision using SchemaDeletedItemSources/SchemaAddedItemSources - Add specific item-level sources for response headers, media types, parameters, request body, and properties Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Operation security checkers: remove operation-level sources since "security" isn't tracked at field level in Origin. No source is better than an imprecise one pointing to the operation line. - api-operation-id-removed: clear revisionSource in the "changed" case (old ID replaced by new) since the change is about what was removed. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
yaml3 now includes sequence field keys (required, enum, type array) in origin.Fields, so NewSourceFromField no longer needs to fall back to origin.Sequences. Each function has a clear responsibility: - NewSourceFromField: looks up field key location via origin.Fields - NewSourceFromSequenceItem: looks up specific item via origin.Sequences Also updates kin-openapi and yaml3 dependencies. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Upgrade 9 checkers to use SchemaDeletedItemSources/SchemaAddedItemSources for per-item precision instead of SchemaFieldSources field-level precision. Enum checkers (5 files): - check_request_property_enum_value_updated.go - check_response_property_enum_value_removed.go - check_response_property_enum_value_added.go - check_request_body_enum_deleted.go - check_response_mediatype_enum_value_removed.go Required checkers (4 files): - check_request_property_required_updated.go - check_response_property_became_required.go - check_response_property_became_optional.go - check_request_header_property_became_required.go Each now points to the specific item in the sequence (e.g., the exact enum value or required property name) rather than the field key. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Use GetRevisionSource() for precise file/line/col annotations instead of deprecated GetSourceFile()/GetSourceLine()/GetSourceColumn() methods. Falls back to operation source file when RevisionSource is nil (removal changes appear in Actions log without inline annotation). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Use requestBodyFieldSources() to point to the `required` field within the request body origin instead of the operation-level source. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Changenow carriesBaseSourceandRevisionSourcewith the exact position in the base and revision spec filesopenapi3.IncludeOrigin = true, all 93 checkers report locations--format githubactionsusesRevisionSourcefor inline annotations with file/line/col precisionChanges
New types and interfaces
checker.Sourcestruct withFile,Line,ColumnfieldsCommonChange.BaseSource/RevisionSourcefields on all change typesGetBaseSource()/GetRevisionSource()methods on theChangeinterfaceWithSources()builder method onApiChange,ComponentChange,SecurityChangeChecker updates (93 files)
NewApiChange(...)call chains.WithSources(baseSource, revisionSource)operationSources()helper computes sources from operation pairsSource precision tiers
Tier 1 — Sub-object Origin:
SchemaSources()— source pair from schema OriginsParameterSources()— source pair from parameter OriginsResponseSources()— source pair from response OriginsTier 2 — Field-level Origin:
SchemaFieldSources()— source pair from a specific field within schema Origins (e.g.,type:line)ParameterFieldSources()— source pair from a specific field within parameter OriginsOperationFieldSources()— source pair from a specific field within operation OriginsTier 3 — Sequence item Origin:
NewSourceFromSequenceItem()— source from a specific item in a sequence field (e.g., a particular enum value)SchemaDeletedItemSources()/SchemaAddedItemSources()— source pair for deleted/added sequence itemsNil convention for missing side
baseSource = nil(didn't exist in base)revisionSource = nil(doesn't exist in revision)baseSource = nilrevisionSource = nilNewSourceFromFieldreturns nil when the field doesn't exist in Origin (no fallback to parent)NewSourceFromSequenceItemreturns nil when the item isn't foundGitHub Actions formatter migration
--format githubactionsnow usesGetRevisionSource()forfile=,line=,col=annotation parametersRevisionSourceappear inline on PR diffs at the exact line of the changeRevisionSourceis nilrevisionSource = nil) emit::error title=...::messagewithoutfile=— they appear in the Actions log but not inline on the diff, since the removed element only exists in the base file which cannot be referenced by GitHub annotationsendLine/endColumnparameters (never populated)Other formatter updates
baseSource/revisionSourcein outputInfrastructure
diff.go: passes both base and revision operation sourcesdiff/security_schemes.go: addedBase/Revisionfields for component-level source trackinggo.mod: usesoasdiff/kin-openapifork with origin file trackingfeat/origin-enhancements: sequence field keys now tracked inorigin.Fields(in addition to items inorigin.Sequences)Known limitations
No source locations
api-global-security-*) — spec-root-level changes with no operation context;SecurityRequirementsDiffdoes not store original objects needed for Origins.api-security-*) —securityis a complex array-of-objects not tracked at field level in Origin; pointing to the operation line would be misleading.No per-item sequence precision
check_request_parameter_x_extensible_enum_value_removed.go,check_request_property_x_extensible_enum_value_removed.go) —x-extensible-enumis an extension, not tracked inOrigin.Sequences. Uses schema/parameter-level sources.check_request_parameter_list_of_types_changed.go,check_request_property_list_of_types_changed.go,check_response_property_list_of_types_changed.go) — reports aggregated diffs (joined string of all added/deleted types), not per-item. Uses field-level sources.check_response_required_property_updated.go) — iterates property schemas viaCheckDeletedPropertiesDiff/CheckAddedPropertiesDiff, not required list items. Already usespropertySourcewhich points to the property schema itself.Removal-type changes and GitHub annotations
::error file=,line=annotations can only reference lines in the head (revision) commit. There is no mechanism to annotate a line in the base file before a change.BaseSource— the removed element doesn't exist in the revision file.::errorwithoutfile=so they appear in the Actions log and checks summary, but not inline on the PR diff.side: LEFTcan comment on deleted lines in the diff — this is a separate, richer mechanism suitable for a paid tier.Other
readonlyinstead of the canonicalreadOnly, the source lookup won't match. This matches the OpenAPI specification which requires camelCase field names.Test plan
RevisionSourceinstead of deprecated fieldsgo vetcleanIncludeOriginis false🤖 Generated with Claude Code