Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Associate OpenAPI elements with the originating line and column in the YAML specification #986

Open
reuvenharrison opened this issue Jul 18, 2024 · 2 comments

Comments

@reuvenharrison
Copy link
Contributor

reuvenharrison commented Jul 18, 2024

Hi all,
As the maintainer of oasdiff which relies on kin-openapi to report changes in OpenAPI specs, I have a requirement from oasdiff users to be able to correlate the breaking-change messages with the originating lines in the YAML specifications.

I created a small proof-of-concept which achieves this as follows:

  1. Extend yaml.v3 to add line and column numbers of each YAML element to the unmarshalled interfaces
  2. Extend kin-openapi UnmarshalJSON functions to read these new elements into the OpenAPI structs (T, Paths, PathItem...)

For example, here's a proposal for the extended T struct:


// Location points to a line and column in the originating YAML file
type Location struct {
	Line int
	Col  int
}

// Origin provides information about the locations of an element and its fields in the originating YAML file
type Origin struct {
	Element *Location
	Fields  map[string]*Location
}

// T is the root of an OpenAPI v3 document
// See https://github.com/OAI/OpenAPI-Specification/blob/main/versions/3.0.3.md#openapi-object
type T struct {
	Extensions map[string]any `json:"-" yaml:"-"`
	Origin     *Origin

	OpenAPI      string               `json:"openapi" yaml:"openapi"` // Required
	Components   *Components          `json:"components,omitempty" yaml:"components,omitempty"`
	Info         *Info                `json:"info" yaml:"info"`   // Required
	Paths        *Paths               `json:"paths" yaml:"paths"` // Required
	Security     SecurityRequirements `json:"security,omitempty" yaml:"security,omitempty"`
	Servers      Servers              `json:"servers,omitempty" yaml:"servers,omitempty"`
	Tags         Tags                 `json:"tags,omitempty" yaml:"tags,omitempty"`
	ExternalDocs *ExternalDocs        `json:"externalDocs,omitempty" yaml:"externalDocs,omitempty"`

	visited visitedComponent
	url     *url.URL
}

Before moving forward, I'd like to collect some feedback:

  1. Do you see value in this feature?
  2. Do you have comments about the proposed solution?
  3. Anything else?

Thanks,
Reuven

@fenollp
Copy link
Collaborator

fenollp commented Jul 27, 2024

Hi! Yes I believe this is a good addition, and quite small at that.

A couple questions:

  • Is Fields map[string]*Location final? I'm thinking Fields map[string]Location would be simpler and as correct
  • Maybe you'd like to extend your Location struct to express the size of whole blocks?
  • What about YAML comments? Do you plan on also proposing to support these in kin-openapi? I believe OpenAPI users should express comments as extensions (as JSON doesn't have comments, just repeated keys, sometimes...) but it would be completely okay to provide access to YAML comments locations and contents from OpenAPI structs.

@reuvenharrison
Copy link
Contributor Author

Hi @fenollp
Thanks for the positive feedback and yes, I agree with your comments above.
Here's the work plan:

  1. Submit a PR in yaml.v3 to add location data in unmarshal
  2. In parallel, I will submit a PR here. If the yaml v3 PR is delayed it will be based on a fork.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants