Skip to content

Implement nested schemas #42

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

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open

Conversation

RainerBayr
Copy link

#41 support "$ref" property:

  • the type of the property gets linked to the property in the definitions section
  • all property in the definitions section get written in there own comment sections

support for the "allOf" property:

  • "$ref" :
    • if only one "$ref" is available the type gets linked
    • with multiple "$ref" all properties of the referenced definitions get copied into the current property and the type is set to "object"
  • all properties of sub objects get copied into the current property

support "$ref" property:
- the type of the property gets linked to the property in the definitions section
- all property in the definitions section get written in there own comment sections

support for the "allOf" property:
- "$ref" :
   + if only one "$ref" is available the type gets linked
   + with multiple "$ref" all properties of the referenced definitions get copied into the current property and the type is set to "object"
- all properties of sub objects get copied into the current property
@brettz9
Copy link
Contributor

brettz9 commented Aug 27, 2020

This is exciting to see implemented, but could we implement a mechanism as in #41 by which a type can be derived from a type (e.g., an option that the first item in allOf will be treated as the base type (for the @typedef {<baseType>})?

Especially for where one is following classical type inheritance, it would be convenient to be able to explicitly link the base type (rather than just treating it as an object mixin, though that would make sense for some projects/scenarios).

It'd also be nice to have a test case to ensure it is working as intended.

@brettz9
Copy link
Contributor

brettz9 commented Sep 1, 2020

I've continued the discussion back in #41 . Especially since that approach is detailed in the JSON Schema spec, I think that would be the way to go.

@RainerBayr
Copy link
Author

RainerBayr commented Sep 1, 2020

@brettz9 that would mean an "is-a" annotation would set the type of the object, an "has-a" annotation would create a property with this type and no annotation (or "default") would copy the properties into the current object.

This would be easy to implement for a allOf section.

For a anyOf section the "is-a" would become a link in the type, the "has-a" would become a optional property, but the "default" have to create a subtype an link it like a "is-a" e.g.:

Laborer: {
      title: 'Laborer',
      anyOf: [
        {$ref: '#/$defs/Person', "classRelation": "is-a",},
        {$ref: '#/$defs/Person1', "classRelation": "has-a",},
        {$ref: '#/$defs/Person2',},
      ]
}

would become:

/**
 * @typedef {object|Person|generatedType} Laborer
 * @property {Person1} [Person1]
 */

/**
 * @typedef {object} generatedType
 * "copy of Person2"
 */

or should the "has-a" annotation also generate and link a type like the "default" dose, because there isn't really a way to state, that the Person1 property isn't optional if "object" is chosen as the type?

@brettz9
Copy link
Contributor

brettz9 commented Sep 1, 2020

I think the example they list where "has-a" is attached to one of the "properties" may have been intended to suggest merely that the "foo" property was related to the main schema in a "has-a" relationship as opposed to say just being an "attribute" of the main schema (e.g., like if a Person class had a children and age property, the former would be "has-a", and the latter would be something else).

I don't think though that "has-a" has any utility for jsdoc (at least standard jsdoc) since using it to add a @property would I think be contradictory to JSON Schema itself since a schema in a given place is supposed to describe the object in that place--not to describe the properties (except where it is explicitly describing the properties). (So in your example, the anyOf should not be repurposed to sometimes imply it is actually adding to properties.)

But the inheritance vs. property (aka mixin) distinction does map to jsdoc differently (i.e., typedef {type} referencing a separate typdef vs. a set of @property being added to the existing typedef) so I think it makes sense to have "is-a" (vs. not "is-a").

Rainer added 2 commits September 3, 2020 11:00
@RainerBayr
Copy link
Author

Implementation for the allOf and anyOf attribute:

allOf:

  • properties of normal refs get copied into the object
  • refs with the classRelation: 'is-a' keyword will be linked as type (multiple types are separated by &)
  • properties of nested objects get copied into the object

anyOf:

  • normal refs generate a new object, whitch get linked in the type (multiple types are separated by |)
  • refs with the classRelation: 'is-a' keyword will be linked as type
  • properties of nested objects generate a new object, whitch get linked in the type

@brettz9
Copy link
Contributor

brettz9 commented Sep 27, 2020

Just an FYI that I've added support for this format in my project, jsdoc-jsonschema, and I've been using json-schema-to-jsdoc for sanity checking in my tests.

With 100% coverage in both projects, it appears that if this PR can be rebased and is deemed ready to be merged, there should I believe now be full round-trippabiility between the formats we expect/create.

@RainerBayr
Copy link
Author

Just a small update: I have fixed the merging conflicts with the main branch, so it should be save to implement the changes.

@brettz9
Copy link
Contributor

brettz9 commented Oct 10, 2020

Hi,

A couple items, if I may...

  1. When running npm test, lines 66,167-168,177,335 show as uncovered.
  2. For the addition of ${obj.title}_subtype${count} for a missing title or ${obj.title}_${e.title} for a present one, if this is necessary, can we make this optional?

Even if you make no. 2 as an option, could we allow hierarchies to be shown in a more standard fashion with typedefs cross-referencing each other like:

/**
 * @typedef {PlainObject} Coordinate
 * @property {number} x
 * @property {number} y
 */

/**
 * @typedef {Coordinate} Circle
 * @property {number} r
 */

/**
 * @typedef {Circle} ColoredShape
 * @property {string} color
 */

And for unions/intersections, e.g.,

/**
 * @typedef {ShapeInfo & (Circle|Rectangle)} PositionedShape3D
 * @property {number} z
 */

FWIW, on my test-json-schema branch of jsdoc-jsonschema I have sanity checks using your branch to show the results my jsdoc-jsonschema would expect when converting back its JSON schema test results into jsdoc (the anyOf/allOf tests are failing due to this style of inheritance not being supported in your branch).

Thanks!

@brettz9
Copy link
Contributor

brettz9 commented Jan 13, 2021

@RainerBayr : Do you think you could take a look at my comment at https://github.com/n3ps/json-schema-to-jsdoc/pull/42/files#issuecomment-706453382 for possible consideration in this PR?

@brettz9
Copy link
Contributor

brettz9 commented Jun 9, 2021

Looks like I had the wrong link. I think I had been referring to #41 (comment)

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.

2 participants