disneystreaming / smithy4s

https://disneystreaming.github.io/smithy4s/
Other
340 stars 68 forks source link

Refine member Scaladoc logic to only include structs/operations #773

Open kubukoz opened 1 year ago

kubukoz commented 1 year ago

Another follow-up to #713.

We only check for unions here:

https://github.com/disneystreaming/smithy4s/blob/series/0.17/modules/codegen/src/smithy4s/codegen/internals/SmithyToIR.scala#L760-L780

I believe we should be stricter and only hit else if shape is a struct OR operation (see #772).

Currently, it also matches:

enums

$version: "2"

namespace demo

/// This is an enum
enum AnEnum {
  /// this is a tomato
  TOMATO = "tomat"
}
/** This is an enum
  * @param TOMATO
  *   this is a tomato
  */
sealed abstract class AnEnum(_value: String, _name: String, _intValue: Int, _hints: Hints) extends Enumeration.Value {

collections (lists/maps)

/// this is a list
list AList {
  /// this is a member
  member: String
}

/// this is a map
map AMap {
  /// this is a key
  key: String
  /// this is a value
  value: String
}
/** this is a map
  * @param key
  *   this is a key
  * @param value
  *   this is a value
  */
object AMap extends Newtype[Map[String, String]] {
zetashift commented 1 year ago

Wouldn't operations rendering also be affected if it's only structs?

kubukoz commented 1 year ago

good catch :) that's true.

Althouuuuugh... for operations, it's not really "operation members", but rather members of the input shape, which is 100% a struct. So the if will probably still refer to structs

Baccata commented 1 year ago

For collections, the content of the documentation is mostly correct, it's just that @param is the wrong annotation. We just need to drop it in favour of a custom header.

kubukoz commented 1 year ago

Any suggestions for the header? I'd start with empty string.

Baccata commented 1 year ago

Yeah that's fine. As long as Maps have got key and value, and list have "member" or "content", it's all good