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

Collapse anyOf with only one option? #296

Open
stevekuznetsov opened this issue Oct 22, 2024 · 6 comments
Open

Collapse anyOf with only one option? #296

stevekuznetsov opened this issue Oct 22, 2024 · 6 comments

Comments

@stevekuznetsov
Copy link
Contributor

If I have a schema like:

        "rings": {
          "type": "array",
          "description": "Array of the deployment ring object.",
            "items": {
                "anyOf": [
                    {
                        "$ref": "#/definitions/deploymentRing"
                    }
                ]
            }
      }

In a sense, it's equivalent to:

        "rings": {
          "type": "array",
          "description": "Array of the deployment ring object.",
          "items": {
              "type": "object",
              "$ref": "#/definitions/deploymentRing"
          }
      }

For users that generate Go types using a JSONSchema they do not control, the use of anyOf in the above example will generate []interface{}, which is unfortunate and very much so not ergonomic, either on generation or parsing. Would it be acceptable to generate a Go struct in these cases with a definite type for the slice, even if that is not perfectly what the author of the JSONSchema intended?

I am not sure, but it seems like there may be some other tool that generates these "loose" JSONSchemas with anyOf array items with only one type, perhaps also a bug in that generator. Would be awesome to work around such cases on the consumer end with this tool.

@stevekuznetsov
Copy link
Contributor Author

If this proposal is acceptable, I am happy to implement such a thing, also happy to hide it behind a flag.

@stevekuznetsov
Copy link
Contributor Author

@omissis any thoughts on this? Happy to start work ASAP if you are OK with the idea.

@omissis
Copy link
Owner

omissis commented Nov 7, 2024

Hey, I think that while this approach might be considered attractive for one-child AnyOfs, it exposes the client code that is using the generated code to breaking changes in those cases where the AnyOfs eventually evolve to contain more than one item: for this reason I am not very keen on going down this path.
Unfortunately Go's type system is what it is and Go generics are what they are, so I don't see a good way to provide great ergonomy for now.

@stevekuznetsov
Copy link
Contributor Author

stevekuznetsov commented Nov 8, 2024

@omissis wouldn't the transition from a one-length anyOf to a n-length anyOf just mean that the generated code would change from something like

type Thing struct {
    Member MemberType `json:"member"`
}

type MemberType struct {
    Field string
}

To something like the current generation, which is

type Thing struct {
    Member any `json:"member"`
}

type MemberType struct {
    Field string
}

In what cases would this be breaking for consuming code? Naively I was expecting no impact for some consumer like:

func main() {
    myThing := Thing{
        Member: MemberType{
            Field: "whoa",
       },
    }
}

For example, on the playground


Changes from a one-length anyOf to some other one-length anyOf would be breaking, yes, but they would be breaking regardless if that were the true intent of the schema (and in the current generation, this break would not be caught by the compiler!)

@stevekuznetsov
Copy link
Contributor Author

stevekuznetsov commented Nov 19, 2024

@omissis thoughts? On coding this I realized we just ignore these fields today. The update would be trivial:

$ git diff -- pkg/generator/schema_generator.go
diff --git a/pkg/generator/schema_generator.go b/pkg/generator/schema_generator.go
index 25bea18..24d384c 100644
--- a/pkg/generator/schema_generator.go
+++ b/pkg/generator/schema_generator.go
@@ -704,6 +704,12 @@ func (g *schemaGenerator) generateTypeInline(
                        }
                }
 
+               for _, collection := range [][]*schemas.Type{t.AllOf, t.AnyOf, t.OneOf} {
+                       if len(collection) == 1 {
+                               return g.generateDeclaredType(collection[0], scope)
+                       }
+               }
+
                typeIndex := 0
 
                var typeShouldBePointer bool

Please let me know what backwards compatibility issues you forsee - I hope my comment above helps to show my thinking. Simple summary:

len(anyOf) Before len(anyOf) After Type Before Type After Impact
1 1 typeA typeB breaking change, but the user will appreciate this since the schema did in fact break
1 N>1 typeA interface{} not a breaking change
N>1 N>1 interface{} interface{} not a breaking change
N>1 1 interface{} typeA breaking change, but a user will appreciate this since the schema did in fact break

I believe some upstream tooling that generates JSON Schemas from e.g. .NET applications may be incorrectly using anyOf even where only one type is possible, so consuming these generated JSON Schemas is very tricky without this kind of functionality.

@stevekuznetsov
Copy link
Contributor Author

@omissis any thoughts here?

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

No branches or pull requests

2 participants