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.

Sign up for GitHub

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

Jump to bottom

Filter out fragment properties only appearing in the required list #379

Closed
czechboy0 opened this issue Nov 14, 2023 · 0 comments · Fixed by #381
Closed

Filter out fragment properties only appearing in the required list #379

czechboy0 opened this issue Nov 14, 2023 · 0 comments · Fixed by #381
Assignees
@czechboy0
Labels
area/generator Affects: plugin, CLI, config file. kind/usability Usability of generated code, ergonomics. size/S Small task. (A couple of hours of work.)
Milestone
1.0

Comments

@czechboy0
Copy link
Contributor

Adopt OpenAPIKit 3.1.0, which allows us to differentiate now.

https://github.com/mattpolzin/OpenAPIKit/releases/tag/3.1.0

We should emit a warning diagnostic when an inferred property is encountered and filtered out.
@czechboy0 czechboy0 added area/generator Affects: plugin, CLI, config file. kind/usability Usability of generated code, ergonomics. size/S Small task. (A couple of hours of work.) labels Nov 14, 2023
@czechboy0 czechboy0 added this to the 1.0 milestone Nov 14, 2023
@czechboy0 czechboy0 changed the title Filter out fragment properties only appearing in the required list Filter out inferred properties only appearing in the required list Nov 14, 2023
@czechboy0 czechboy0 changed the title Filter out inferred properties only appearing in the required list Filter out fragment properties only appearing in the required list Nov 14, 2023
@czechboy0 czechboy0 mentioned this issue Nov 14, 2023
Merged
@czechboy0 czechboy0 self-assigned this Nov 15, 2023
czechboy0 added a commit that referenced this issue Nov 16, 2023
@czechboy0
Skip inferred properties (#381)
577bafd 
### Motivation

Fixes #379.

Context:

If you define an object schema as follows:

```yaml
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: 

```yaml
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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@czechboy0 czechboy0

Labels
area/generator Affects: plugin, CLI, config file. kind/usability Usability of generated code, ergonomics. size/S Small task. (A couple of hours of work.)
Projects
None yet
Milestone
1.0
Development

Successfully merging a pull request may close this issue.

Skip inferred properties czechboy0/swift-openapi-generator
1 participant
@czechboy0