Errors about inconsistent property types are confusing or invalid

[ainar-g]

Version: 0.34.4.
OS/Arch: Ubuntu 20.04/AMD64.

Spec to reproduce:

'openapi': '3.0.3'
'info':
  'contact':
    'email': 'user@example.com'
    'name': 'My API'
    'url': 'https://example.com/api/'
  'description': >
    My API
  'license':
    'name': 'GNU General Public License v3.0'
    'url': 'https://www.gnu.org/licenses/gpl-3.0.txt'
  'title': 'My API'
  'version': '1.0'

'servers':
- 'description': >
    The V1 HTTP API namespace.
  'url': '/api/v1'

'security':
- 'basicAuth': []

'tags':
- 'description': >
    Users
  'name': 'users'

'paths':
  '/users/refresh':
    'post':
      'description': >
        Refresh all users.
      'operationId': 'refresh_users'
      'responses':
        '200':
          '$ref': '#/components/responses/CreateV1UsersRefreshResp'
      'summary': 'Refresh all users.'
      'tags':
      - 'users'

'components':
  'responses':
    'CreateV1UsersRefreshResp':
      'content':
        'application/json':
          'schema':
            '$ref': '#/components/schemas/CreateV1UsersRefreshResp'
      'description': >
        User refresh response.

  'schemas':
    'CreateV1UsersRefreshResp':
      'description': >
        User refresh response.
      'example':
        'refreshed':
        - 'id': 'abcd1234'
          'refreshed': 1234567890000
      'properties':
        'refreshed':
          'description': >
            All refreshed users.
          'items':
            '$ref': '#/components/schemas/User'
          'type': 'array'
      'required':
      - 'refreshed'
      'type': 'object'

    'User':
      'description': >
        User.
      'example':
        'id': 'abcd1234'
        'refreshed': 1234567890000
      'properties':
        'id':
          'description': >
            User ID.
          'type': 'string'
        'refreshed':
          'description': >
            Unix time of last refresh, in milliseconds.
          'type': 'number'
      'required':
      - 'id'
      - 'refreshed'
      'type': 'object'

  'securitySchemes':
    'basicAuth':
      'description': >
        Basic HTTP authorization.
      'scheme': 'basic'
      'type': 'http'

Want: No errors or warnings.

Got:

[Warning] No .validaterc file found. The validator will run in default mode.
To configure the validator, create a .validaterc file.

warnings

  Message :   Property has inconsistent type: refreshed.
  Path    :   components.schemas.CreateV1UsersRefreshResp.properties.refreshed
  Line    :   60

  Message :   Property has inconsistent type: refreshed.
  Path    :   components.schemas.User.properties.refreshed
  Line    :   81

openapi-validator seems to think that .../User/properties/refreshed and .../CreateV1UsersRefreshResp/properties/refreshed are the same property when in reality they are two different properties of two different schemas with two different types. Either this is a bug or, if the check is intended to keep all properties with the same name the same type, the message should be changed to something like:

  Message :   Properties with the same name have inconsistent types: refreshed.

[dpopp07]

Thanks for the issue - I think this was likely intentionally implemented but it's worth us rethinking if that's really the behavior we want and if so, looking at changing the message. In the meantime, you should be able to disable this rule in a .validaterc file.

[mkistler]

The intent of this warning is flag cases where the same name is used for different things, and that is exactly what is happening here. In one place, refreshed is a list of users, and in another place it is a timestamp. Using different names for different things is a good practice in API design that we are trying to encourage here. In this specific case, I think refreshed_time is a better name for the Unix time of last refresh, in milliseconds.

[ainar-g]

I see. Then, I think, my second point stands, and the diagnostic message should reflect the fact that it is about the names of the properties as opposed to the properties themselves. Because the current wording might imply the latter.

[scottmcarthur-heb]

I completely agree that simply updating the message to indicate that the issue is with the name would be more clear. I spent more time than necessary trying to track down what the actual problem was, when a message like the one proposed by Ainar would have been much more clear.

[dpopp07]

Sorry for the delay on this issue, I agree that the message should be made more clear. I will try to make that change soon

[dpopp07]

This issue is being addressed in #408