disneystreaming / smithy4s

https://disneystreaming.github.io/smithy4s/
Other
347 stars 69 forks source link

Generate member Scaladoc for structs that don't have their own documentation #771

Closed kubukoz closed 1 year ago

kubukoz commented 1 year ago

After #731, if you try to generate code from

$version: "2"

namespace demo

structure Hello {
  /// the field
  field: String
}

then you get this:

package demo

/* imports omitted */
case class Hello(field: Option[String] = None)
object Hello extends ShapeTag.Companion[Hello] {
/* omitted */
}

However, if you add docs to Hello itself:

namespace demo

+/// the struct
 structure Hello {
   /// the field
   field: String

then you get full-blown docs on the struct:

/** the struct
  * @param field
  *   the field
  */
case class Hello(field: Option[String] = None)

Expectation

I think the behavior with the original Smithy snippet should be that the struct gets this Scaladoc:

/** @param field
  *   the field
  */
daddykotex commented 1 year ago

I'm not sure I follow, you intentionally want to discard the documentation message: the struct?

kubukoz commented 1 year ago

no, that doesn't exist in the original snippet.

I want this:

structure Hello {
  /// the field
  field: String
}

to be generated as:

/** @param field
  *   the field
  */
case class Hello(field: Option[String] = None)

because currently it's:

case class Hello(field: Option[String] = None)
kubukoz commented 1 year ago

(I'm 90% sure) This happens because the following code doesn't look at memberDocs if there's no Documentation trait on the struct:

https://github.com/disneystreaming/smithy4s/blob/f81b362b6531272b454ee0e21b9567e9688bf7ba/modules/codegen/src/smithy4s/codegen/internals/SmithyToIR.scala#L782-L785

daddykotex commented 1 year ago

ahhh I see, I missed the initial comment inside of Hello.

zetashift commented 1 year ago

(I'm 90% sure) This happens because the following code doesn't look at memberDocs if there's no Documentation trait on the struct:

https://github.com/disneystreaming/smithy4s/blob/f81b362b6531272b454ee0e21b9567e9688bf7ba/modules/codegen/src/smithy4s/codegen/internals/SmithyToIR.scala#L782-L785

Yeah I think this is correct, if shape.getTrait returns an empty value we'll get a None. Not sure how to write out an elegant, if shape.getTrait.orElse(hasMemberTraits) specifically for everything that is not a union or a enum or a collection.

msosnicki commented 1 year ago

I can have a look into it