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

Use null instead of 'null' in null objects schema #40

Merged
merged 1 commit into from
Apr 19, 2022
Merged

Conversation

miguelaeh
Copy link
Contributor

Signed-off-by: Miguel A. Cabrera Minagorri [email protected]

Description of the change

Null objects were being set as "null" (note string) instead of null into the schema. This PR fixes that and sets it properly.

Benefits

Possible drawbacks

Applicable issues

Additional information

Checklist

  • Version bumped in package.json and package-lock.json according to semver.
  • New features are documented in the README.md
  • New features contain a new test at the tests folder
  • All tests pass running npm test

Copy link
Member

@carrodher carrodher left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link

@antgamdia antgamdia left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the changes. LGTM

null is a valid type allowed by JSON Schema (because it is a JSON Object) and the OAS (3.0.X - note we are not compliant with versions 3.1.X) imposes that the type used in type must conform to the defined type for the Schema Object.

FTR: OAS 3.1.X does support the null type doing something like: type: ["null", "string"], so nullable is deprecated. We should state somewhere that the yielded OAS is 3.0.X-compliant, so that client can adapt their tooling.

Once we add nullable: true we are adding "null" to the list of strings allowed in the property type. So, given that the only allowed value for a type: "null" is, actually, the literal null, the changes in this PR seem to be OK.

However, note that this fine-grained semantic validation is not implemented in most of the editors, as the vast majority only leverage the lexical/syntactic level. For instance, the examples in this PR (pre PR and post PR) will both be "correct" in, for example, Swagger Editor,

Signed-off-by: Miguel A. Cabrera Minagorri <[email protected]>
@miguelaeh miguelaeh merged commit 0928568 into main Apr 19, 2022
@miguelaeh miguelaeh deleted the fixNullString branch April 19, 2022 08:26
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

Successfully merging this pull request may close these issues.

nullable does not allow null as a value
3 participants