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

Skip inferred properties #381

Merged
merged 2 commits into from
Nov 16, 2023
Merged

Conversation

czechboy0
Copy link
Contributor

@czechboy0 czechboy0 commented Nov 14, 2023

Motivation

Fixes #379.

Context:

If you define an object schema as follows:

Foo:
  type: object
  properties:
    a:
      type: string
  required:
    - b

OpenAPIKit will parse it as having two properties: a of type string and b being a fragment, with a optional and b required.

Unfortunately, often property names appear only in the required list as a result of a typo, leading to non-compiling code and a difficult debugging story.

It comes down to the fact that Swift OpenAPI Generator never officially supported defining properties this way, and continues not to.

If adopters want to do this intentionally, they should do this:

Foo:
  type: object
  properties:
    a:
      type: string
    b: {}
  required:
    - b

Modifications

This PR tightens the rules around inferred properties:

  1. Inferred properties are skipped, so the result of the first example will only have the property a generated, and the second example both a and b.
  2. The generator emits a warning diagnostic when encountering an inferred property, which should help adopters who made a typo catch it much earlier in the process.
  3. Bumps OpenAPIKit to 3.1.0, as the inferred property was added just yesterday.

Result

Less likely typos and mismatch between properties and required, and a diagnostic to help debug this case.

Test Plan

Added a snippet test.

@czechboy0
Copy link
Contributor Author

@swift-server-bot test this please

@czechboy0 czechboy0 merged commit 577bafd into apple:main Nov 16, 2023
@czechboy0 czechboy0 deleted the hd-fix-inferred-properties branch November 16, 2023 13:56
@czechboy0 czechboy0 added the 🔨 semver/patch No public API change. label Nov 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🔨 semver/patch No public API change.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Filter out fragment properties only appearing in the required list
2 participants