Azure / openapi-diff

Command line tool to detect breaking changes between two openapi specifications
MIT License
256 stars 36 forks source link

False positives for rule DefaultValueChanged #285

Closed cataggar closed 9 months ago

cataggar commented 9 months ago

I am working on https://github.com/Azure/azure-rest-api-specs-pr/pull/15631 and have false positives for rule DefaultValueChanged.

It is claiming a different default value for definitions.PrivateCloudUpdateProperties.properties.internet for:

Before:

    "PrivateCloudUpdateProperties": {
      "type": "object",
      "description": "The properties of a private cloud resource that may be updated",
      "properties": {
        "managementCluster": {
          "description": "The default cluster used for management",
          "$ref": "#/definitions/ManagementCluster"
        },
        "internet": {
          "description": "Connectivity to internet is enabled or disabled",
          "type": "string",
          "enum": [
            "Enabled",
            "Disabled"
          ],
          "default": "Disabled",
          "x-ms-enum": {
            "name": "InternetEnum",
            "modelAsString": true
          }
        },

After:

    "PrivateCloudUpdateProperties": {
      "type": "object",
      "description": "The properties of a private cloud resource that may be updated",
      "properties": {
        "managementCluster": {
          "$ref": "#/definitions/ManagementCluster",
          "description": "The default cluster used for management"
        },
        "internet": {
          "$ref": "#/definitions/InternetEnum",
          "description": "Connectivity to internet is enabled or disabled",
          "default": "Disabled"
        },

In both cases, it is "default": "Disabled".

mikekistler commented 9 months ago

This is not really a false positive. As a sibling property of $ref, default should be ignored and thus is "not specified" in the latter case. Also note that default is not one of the exceptions to this rule in autorest.

cataggar commented 9 months ago

That makes sense. I opened up https://github.com/Azure/typespec-azure/issues/3933.