smithy4s icon indicating copy to clipboard operation
smithy4s copied to clipboard

Refine member Scaladoc logic to only include structs/operations

Open kubukoz opened this issue 2 years ago • 5 comments

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]] {

kubukoz avatar Jan 30 '23 18:01 kubukoz

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

zetashift avatar Jan 30 '23 18:01 zetashift

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

kubukoz avatar Jan 30 '23 18:01 kubukoz

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.

Baccata avatar Jan 31 '23 08:01 Baccata

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

kubukoz avatar Jan 31 '23 16:01 kubukoz

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

Baccata avatar Feb 01 '23 08:02 Baccata