Warn on $ref with ignored sibling properties

[tedepstein]

From ZEN-2696: The Uber Swagger examples (both single- and multi-file versions) contain the following erroneous Swagger:

Activities:
    properties:
      offset:
        type: integer
        format: int32
        description: Position in pagination.
      limit:
        type: integer
        format: int32
        description: Number of items to retrieve (100 max).
      count:
        type: integer
        format: int32
        description: Total number of items available.
      history:
        type: array
        $ref: Activity

The history property is supposed to be an array of Activity objects, but in fact it's just a single Activity object. It's missing the items property.

It is not erroneous for a JSON Ref node to contain properties other than $ref, but all such properties are ignored according to the spec. This may be a condition that merits a warning in SwagEdit to help prevent this sort of error.

[tedepstein]

Just fielded a support issue with this issue. I think we should make it a warning, and include it in the next sprint.

[tedepstein]

Second support issue, just addressed. Let's make sure we fix this one.

[Gfgomez]

I would recommend to allow the overload of properties in a referenced object $REF. This will allow to define common generic schema objects and later reuse them but changing some specific attribute, for example the description for a more specific schema.

For example we would like to describe:

In a price schema we defined the following attribute:

            newPrice:
              description: New price to be applied
              $ref: '../../core/schemas/price.yaml#/components/schemas/amount'

And amount is described as:

            amount:
              description: Way to display a decimal value because it is not supported natively.
              Type: string

Kaizen will raise an error in the newPrice schema. To make it compatible with Kaizen, I had to remove all descriptions whenever a REF is used, leaving the following schema:

            newPrice:
              $ref: '../../core/schemas/price.yaml#/components/schemas/amount'

After dereferencing (for example using swagger-cli bundle option), swagger editor will display in the first scenario as:

            newPrice:
              description: New price to be applied
              Type: string

But if defined as expected by Kaizen it will be presented as:

            newPrice:
              description: Way to display a decimal value because it is not supported natively.
              Type: string

Being a generic description instead of the specific one needed for a proper API documentation.

[tedepstein]

@Gfgomez , please keep in mind that JSON Reference, the specification being used by OpenAPI in most contexts where you see a $ref property, intentionally restricts the use of sibling properties:

Any members other than "$ref" in a JSON Reference object SHALL be ignored.

This means that the behavior you described in swagger-cli's bundle option is nonstandard. The standard way to accomplish what you're trying to do would be to create a new schema that inherits from the base schema using allOf and overrides the description property.

All that said, I don't think KaiZen Editor's current validation behavior is really optimal.

• For OpenAPI 2.0, it doesn't display any kind of warning on a reference object with additional sibling properties alongside $ref.
• For OpenAPI 3.0, a $ref with siblings violates the schema, so it's an error, not a warning. This is more severe than it should be. The spec doesn't really disallow sibling properties, it just says they are ignored.

I would like to fix this by making it a warning in both OpenAPI 2.0 and 3.0. Later on, we can consider allowing the severity to be configurable.

[Gfgomez]

@tedepstein I agree with you in both the swagger-cli behavior is nonstandard and the use of siblings in the standard, but it really is an excellent way to achieve the objective. I will try using allof to override the description property in the new Schema but I believe it leaves the resulting Schema more dirty in the final documentation.

To make it a warning is an good middle point approach, though I look forward to include sibling properties in a future version of the spec, to provide a clean use of core schemas and their reuse. Thanks for the solution proposal.