[Feature Request] Don't create allOf when adding description to ref field

[banool]

First off, here is how you repro the problem:

• Cargo.toml: https://gist.github.com/banool/f86dd2bb4ce2f2388c7fbf4850a98c3f
• src/main.rs: https://gist.github.com/banool/fb3639310dc0ae9f2d01aecdd26a1e44
• src/mymacro.rs: https://gist.github.com/banool/0d5b1e4cb1e9055795d75a3e6631d454

Once you've got this, cargo run and check the generated YAML spec:

curl localhost:8888/spec.yaml

In the spec you can see this:

    MyStruct:
      type: object
      required:
      - public_key
      - number
      properties:
        public_key:
          type: string
          description: some docs
        number:
          $ref: '#/components/schemas/U64'

Now if you change line 63 in src/main.rs to a doc comment (///) and generate the spec, you get this:

    MyStruct:
      type: object
      required:
      - public_key
      - number
      properties:
        public_key:
          type: string
          description: some docs
        number:
          allOf:
          - $ref: '#/components/schemas/U64'
          - description: if you make this a doc string, this field becomes allof

As you can see, if you add a docstring to the field in this struct, the spec generator uses allOf so it can include the description.

According to this validator (https://apitools.dev/swagger-parser/online/), you can just include the description side by side with the ref like this:

    MyStruct:
      type: object
      required:
      - public_key
      - number
      properties:
        public_key:
          type: string
          description: some docs
        number:
          $ref: '#/components/schemas/U64'
          description: if you make this a doc string, this field becomes allof

1. If the validator is correct and this is allowed (where you have the description right there without the allOf), could we change the spec generator to work like this instead?
2. If that's not true and this is not allowed, could we instead make the spec generator consider it an error to include a docstring alongside a field in a struct, since this allOf behavior feels like a real footgun. Or at least include some way to opt in to making it a compilation error.

I understand it's possible that the macro I'm using is also a bit messed up, but it feels like Poem should still do either one of these two things above.

Thanks a lot!

For context, here is the PR where we unexpectedly ran into this problem: https://github.com/aptos-labs/aptos-core/pull/3774. Here is the PR where we fixed it for now by reverting these changes: https://github.com/aptos-labs/aptos-core/pull/4007.

[sunli829]

This looks like a bug, I'll dig into it later.

[sunli829]

This is the correct result and is intentional.

     // if you make this a doc string, this field becomes allof
     pub number: U64,

This line defines a new description for number field, so use allOf to merge them.

[banool]

But why can't you just put the description alongside it, like in my last snippet? The validator I used online says it's valid, and that way we don't add an unnecessary extra type just for the sake of having a doc string.

[sunli829]

But why can't you just put the description alongside it, like in my last snippet? The validator I used online says it's valid, and that way we don't add an unnecessary extra type just for the sake of having a doc string.

I tried it and Swagger UI doesn't seem to recognize this new description field.

    MyStruct:
      type: object
      required:
      - public_key
      - number
      properties:
        public_key:
          type: string
          description: some docs
        number:
          $ref: '#/components/schemas/U64'
          description: if you make this a doc string, this field becomes allof   ///<<< This seems to be not allowed.

[banool]

Hmmm yeah you're right, I tried a different UI generator and it didn't like it either.

I feel like this is a potential foot gun in the future, when people add a docstring to a field in a struct they don't expect that that will result in a change in the actual representation of the types. Is there a way to make including a doc string a compile time error instead? For example how it's an error in clap to add a docstring to a flatten field.

[DDtKey]

I want to revive this issue, it's actually problematic behavior - by adding a doc comment, in fact, we're changing the structure expected. Btw I observe similar behavior for default

  "allOf": [
              {
                "$ref": "#/components/schemas/SomeObj"
              },
              {
                "description": "Some comment",
                "default": "y"
              }
            ]

For example, this causes problems when generating clients from such openapi-specs. And 3.1 openapi seems to support description + ref, but allOf is dirty workaround: https://github.com/OAI/OpenAPI-Specification/issues/1514

Probably it should be just ignored in openapi spec until 3.1 is supported? 🤔
However, that's true that UI shows such structs correctly

cc @sunli829