codegen: various minor fixes and improvements (#5374)

A few sundry things:
- The discriminator of each child of a oneOf is reified as `def
$discriminatorName: String = "..."`, but the parent type didn't declare
the abstract method, which was irritating. Parent will now declare the
method.
- Fixes a reference bug when inheriting an enum using circe serdes,
where parent didn't use enum in json bodies but the child does, and
`seperateFilesForModels=true`
- Adds a `openapiAlwaysGenerateParamSupport` boolean flag, so that enums
can be defined in an inherited openapi and used in path position,
without having to redeclare them (and everything that transitively
contains them)
- Fixes client encoding of non-exploded array params (was using a `.map`
instead of a `.flatMap` and producing e.g.
`?myParam=List(uuid1),List(uuid2)` instead of `?myParam=uuid1,uuid2`
when encoding)
- Remove redundant backticks from discriminator method names when the
property name is not a reserved name
- Tries harder to construct type aliases for inline definitions when
'deduplicating' between schemas
- Creates val aliases for 'dedupped' case class definitions

Author: hughsimpson

doc/generator/sbt-openapi-codegen.md

@@ -57,6 +57,7 @@ openapiPackageDependencies            Map.empty[String, String]            Allow
                                                                            aliases to the duplicates in the 'value' package. This is still experimental - significantly, the type is likely to change
                                                                            to a Map[String, Seq[String]] in the near future to permit multiple 'inheritance', and there may be bugs in the implementation.
 openapiSeperateFilesForModels         false                                When true, models will be written to individual files under $pkg.models, with type aliases and helpers living under `package.scala` in a package object
+openapiAlwaysGenerateParamSupport     false                                When true, all enums will be generated with param & json support, even if not used in those positions. This is useful for definitions that will be reused with `openapiPackageDependencies`
 ===================================== ==================================== ==================================================================================================
 ```
 

openapi-codegen/cli/src/main/scala/sttp/tapir/codegen/GenScala.scala

@@ -6,6 +6,7 @@ import cats.implicits._
 
 import com.monovore.decline._
 
+import sttp.tapir.codegen.dedup.PackageReuseContext
 import sttp.tapir.codegen.openapi.models.OpenapiModels.OpenapiDocument
 import sttp.tapir.codegen.{RootGenerator, YamlParser}
 
@@ -145,7 +146,10 @@ object GenScala {
                 maxSchemasPerFile.getOrElse(400),
                 generateEndpointTypes,
                 !disableValidatorGeneration,
-                useCustomJsoniterSerdes
+                useCustomJsoniterSerdes,
+                PackageReuseContext.none,
+                false,
+                false
               )
             )
             destFiles <- contents.allFiles.toVector.traverse { case (fileName, content) => writeGeneratedFile(destDir, fileName, content) }

openapi-codegen/core/src/main/scala/sttp/tapir/codegen/ClassDefinitionGenerator.scala

@@ -7,7 +7,7 @@ import sttp.tapir.codegen.json.JsonSerdeLib.{Circe, Jsoniter}
 import sttp.tapir.codegen.openapi.models.OpenapiModels.OpenapiDocument
 import sttp.tapir.codegen.openapi.models.{DefaultValueRenderer, OpenapiSchemaType, RenderConfig}
 import sttp.tapir.codegen.openapi.models.OpenapiSchemaType._
-import sttp.tapir.codegen.util.NameHelpers.indent
+import sttp.tapir.codegen.util.NameHelpers.{indent, safeVariableName}
 import sttp.tapir.codegen.util.{DocUtils, VersionedHelpers}
 
 case class GeneratedClassDefinitions(
@@ -82,7 +82,8 @@ class ClassDefinitionGenerator {
       xmlParamRefs: Set[String] = Set.empty,
       useCustomJsoniterSerdes: Boolean = true,
       packageReuse: PackageReuseContext = PackageReuseContext.none,
-      seperateFilesForModels: Boolean = false
+      seperateFilesForModels: Boolean = false,
+      alwaysGenerateParamSupport: Boolean = false
   ): Option[GeneratedClassDefinitions] = {
     val allSchemas: Map[String, OpenapiSchemaType] = doc.components.toSeq.flatMap(_.schemas).toMap
     val allOneOfSchemas = allSchemas.collect { case (name, oneOf: OpenapiSchemaOneOf) => name -> oneOf }.toSeq
@@ -123,7 +124,7 @@ class ClassDefinitionGenerator {
     val generatesQueryOrPathParamEnums = enumsDefinedOnEndpointParams ||
       allSchemas
         .collect { case (name, _: OpenapiSchemaEnum) => name }
-        .exists(queryOrPathParamRefs.contains)
+        .exists(alwaysGenerateParamSupport || queryOrPathParamRefs.contains(_))
     val enumSerdeHelper = if (!generatesQueryOrPathParamEnums) "" else EnumGenerator.enumSerdeHelperDefn(targetScala3)
 
     def fetchTransitiveParamRefs(initialSet: Set[String], toCheck: Seq[OpenapiSchemaType]): Set[String] = toCheck match {
@@ -136,14 +137,27 @@ class ClassDefinitionGenerator {
       jsonParamRefs.toSeq.flatMap(ref => allSchemas.get(ref.stripPrefix("#/components/schemas/")))
     )
 
+    def allChildrenDefineDiscriminator(d: String, s: OpenapiSchemaOneOf): Boolean =
+      s.types.forall {
+        case t: OpenapiSchemaRef =>
+          t.maybeResolved(doc).exists {
+            case o: OpenapiSchemaObject => o.properties.contains(d)
+            case _                      => false
+          }
+        case _ => false
+      }
+
     val adtTypes =
       adtInheritanceMap
         .flatMap(_._2)
         .toSeq
-        .map(_._1)
+        .map { s => s._1 -> s._2.discriminator.map(_.propertyName).filter(d => allChildrenDefineDiscriminator(d, s._2)) }
         .distinct
-        .filterNot(PackageReuseContext.isReusedSchema(_, packageReuse))
-        .map(name => s"sealed trait $name")
+        .filterNot(s => PackageReuseContext.isReusedSchema(s._1, packageReuse))
+        .map { case (name, maybeDiscriminator) =>
+          val maybeBody = maybeDiscriminator.map(d => s" { def ${safeVariableName(d)}: String }").getOrElse("")
+          s"sealed trait $name$maybeBody"
+        }
         .sorted
         .mkString("", "\n", "\n")
     val schemasWithAny = allSchemas.filter { case (_, schema) => schemaContainsAny(schema) }
@@ -174,7 +188,8 @@ class ClassDefinitionGenerator {
       schemasContainAny,
       useCustomJsoniterSerdes,
       packageReuse,
-      resolvableNonClassyOneOfSchemas
+      resolvableNonClassyOneOfSchemas,
+      seperateFilesForModels
     )
     val allTransitiveXmlParamRefs = fetchTransitiveParamRefs(
       xmlParamRefs,
@@ -206,12 +221,45 @@ class ClassDefinitionGenerator {
         _.schemas.map {
           case (name, _: OpenapiSchemaEnum) if PackageReuseContext.isReusedSchema(name, packageReuse) =>
             (name, (0, Seq(PackageReuseContext.enumAliasType(name, packageReuse, seperateFilesForModels))))
+          case (name, obj: OpenapiSchemaObject) =>
+            val isReusedSchema = PackageReuseContext.isReusedSchema(name, packageReuse)
+            (
+              name,
+              (
+                if (isReusedSchema) 0 else 2,
+                generateClass(
+                  allSchemas,
+                  name,
+                  obj,
+                  allTransitiveJsonParamRefs,
+                  adtInheritanceMap,
+                  jsonSerdeLib,
+                  targetScala3,
+                  isReusedSchema,
+                  packageReuse,
+                  seperateFilesForModels,
+                  alwaysGenerateParamSupport
+                )
+              )
+            )
           case (name, s) if PackageReuseContext.isReusedSchema(name, packageReuse) =>
             (name, (0, Seq(PackageReuseContext.aliasType(name, packageReuse, seperateFilesForModels))))
-          case (name, obj: OpenapiSchemaObject) =>
-            (name, (2, generateClass(allSchemas, name, obj, allTransitiveJsonParamRefs, adtInheritanceMap, jsonSerdeLib, targetScala3)))
           case (name, obj: OpenapiSchemaEnum) =>
-            (name, (1, EnumGenerator.generateEnum(name, obj, targetScala3, queryOrPathParamRefs, jsonSerdeLib, allTransitiveJsonParamRefs)))
+            (
+              name,
+              (
+                1,
+                EnumGenerator.generateEnum(
+                  name,
+                  obj,
+                  targetScala3,
+                  queryOrPathParamRefs,
+                  jsonSerdeLib,
+                  allTransitiveJsonParamRefs,
+                  alwaysGenerateParamSupport
+                )
+              )
+            )
           case (name, OpenapiSchemaMap(valueSchema, _, _)) =>
             (name, (0, generateMap(name, valueSchema)))
           case (name, OpenapiSchemaArray(valueSchema, _, _, rs)) =>
@@ -256,14 +304,28 @@ class ClassDefinitionGenerator {
           addModel(name, defns.mkString("\n"))
       }
 
-      adtGroups.toSeq.filterNot(p => isTypeAlias(p._1)).foreach { case (fileName, parentTraits) =>
-        val traits = parentTraits.map(p => s"sealed trait $p").mkString("\n")
+      adtGroups.toSeq.foreach { case (fileName, parentTraits) =>
+        val isReused = isTypeAlias(fileName)
+        val traits =
+          if (isReused) parentTraits.map(p => s"type $p = ${packageReuse.modelRoot(seperateFilesForModels)}.$p").mkString("\n")
+          else parentTraits.map(p => s"sealed trait $p").mkString("\n")
         val children = childToAdtFile.filter(_._2 == fileName).keys.toSeq.sorted
         val childContent = children
           .flatMap { child =>
             allSchemas.get(child).collect { case obj: OpenapiSchemaObject =>
-              generateClass(allSchemas, child, obj, allTransitiveJsonParamRefs, adtInheritanceMap, jsonSerdeLib, targetScala3)
-                .mkString("\n")
+              generateClass(
+                allSchemas,
+                child,
+                obj,
+                allTransitiveJsonParamRefs,
+                adtInheritanceMap,
+                jsonSerdeLib,
+                targetScala3,
+                isReused,
+                packageReuse,
+                seperateFilesForModels,
+                alwaysGenerateParamSupport
+              ).mkString("\n")
             }
           }
           .mkString("\n")
@@ -357,7 +419,11 @@ class ClassDefinitionGenerator {
       jsonParamRefs: Set[String],
       adtInheritanceMap: Map[String, Seq[(String, OpenapiSchemaOneOf)]],
       jsonSerdeLib: JsonSerdeLib.JsonSerdeLib,
-      targetScala3: Boolean
+      targetScala3: Boolean,
+      isReused: Boolean,
+      packageReuse: PackageReuseContext,
+      seperateFilesForModels: Boolean,
+      alwaysGenerateParamSupport: Boolean
   ): Seq[String] = try {
     val isJson = jsonParamRefs contains name
     def rec(className: String, schemaKey: String, obj: OpenapiSchemaObject, acc: List[String]): Seq[String] = {
@@ -395,7 +461,7 @@ class ClassDefinitionGenerator {
       val discriminatorDefBody = discriminatorDefFields.filter { case (n, _) => obj.properties.map(_._1).toSet.contains(n) } match {
         case Nil    => ""
         case fields =>
-          val fs = fields.map { case (k, v) => s"""def `$k`: String = "$v"""" }.mkString("\n")
+          val fs = fields.map { case (k, v) => s"""def ${safeVariableName(k)}: String = "$v"""" }.mkString("\n")
           s""" {
              |${indent(2)(fs)}
              |}""".stripMargin
@@ -406,7 +472,7 @@ class ClassDefinitionGenerator {
         .map { case (key, OpenapiSchemaField(schemaType, maybeDefault, _)) =>
           val (tpe, maybeEnum) =
             mapSchemaTypeToType(className, key, obj.required.contains(key), schemaType, isJson, jsonSerdeLib, targetScala3)
-          val fixedKey = fixKey(key)
+          val fixedKey = safeVariableName(key)
           val optional = schemaType.nullable || !obj.required.contains(key)
           val maybeExplicitDefault =
             maybeDefault.map(
@@ -415,14 +481,24 @@ class ClassDefinitionGenerator {
                   .render(allModels = allSchemas, thisType = schemaType, optional, RenderConfig(maybeEnum.map(_.enumName)))(_)
             )
           val default = maybeExplicitDefault getOrElse (if (optional) " = None" else "")
-          s"$fixedKey: $tpe$default" -> maybeEnum.map(_.impl)
+          if (isReused) "" -> maybeEnum.map { e =>
+            val alias = s"${e.enumName} = ${packageReuse.modelRoot(seperateFilesForModels)}.${e.enumName}"
+            s"type $alias\nval $alias"
+          }
+          else s"$fixedKey: $tpe$default" -> maybeEnum.map(_.impl)
         }
         .unzip
 
       val enumDefn = maybeEnums.flatten.toList
-      s"""|case class $className (
-          |${indent(2)(properties.mkString(",\n"))}
-          |)$parents$discriminatorDefBody""".stripMargin :: innerClasses ::: enumDefn ::: acc
+      val modelDefn =
+        if (isReused) {
+          val alias = s"$className = ${packageReuse.modelRoot(seperateFilesForModels)}.$className"
+          s"type $alias\nval $alias"
+        } else
+          s"""|case class $className (
+              |${indent(2)(properties.mkString(",\n"))}
+              |)$parents$discriminatorDefBody""".stripMargin
+      modelDefn :: innerClasses ::: enumDefn ::: acc
     }
 
     rec(addName("", name), name, obj, Nil)
@@ -485,7 +561,8 @@ class ClassDefinitionGenerator {
             targetScala3,
             Set.empty,
             jsonSerdeLib,
-            if (isJson) Set(enumName) else Set.empty
+            if (isJson) Set(enumName) else Set.empty,
+            false
           )
           (enumName -> e.nullable, Some(InlineEnumDefn(enumName, enumDefn.mkString("\n"))))
 
@@ -500,13 +577,6 @@ class ClassDefinitionGenerator {
 
   private val reservedKeys = VersionedHelpers.reservedKeys
 
-  private def fixKey(key: String) = {
-    if (reservedKeys.contains(key))
-      s"`$key`"
-    else
-      key
-  }
-
   private def schemaContainsAny(schema: OpenapiSchemaType): Boolean = schema match {
     case _: OpenapiSchemaAny                => true
     case OpenapiSchemaArray(items, _, _, _) => schemaContainsAny(items)

openapi-codegen/core/src/main/scala/sttp/tapir/codegen/EndpointGenerator.scala

@@ -419,7 +419,7 @@ class EndpointGenerator {
       jsonSerdeLib: JsonSerdeLib,
       param: OpenapiParameter,
       e: OpenapiSchemaEnum,
-      isArray: Boolean
+      isArray: Boolean,
   ): (String, Some[Seq[String]], String, String) = {
     val enumName = endpointName.capitalize + strippedToCamelCase(param.name).capitalize
     val enumParamRefs = if (param.in == "query" || param.in == "path") Set(enumName) else Set.empty[String]
@@ -429,7 +429,8 @@ class EndpointGenerator {
       targetScala3,
       enumParamRefs,
       jsonSerdeLib,
-      Set.empty
+      Set.empty,
+      false
     )
 
     def arrayType = if (param.isExploded) "ExplodedValues" else "CommaSeparatedValues"

openapi-codegen/core/src/main/scala/sttp/tapir/codegen/EnumGenerator.scala

@@ -14,15 +14,16 @@ object EnumGenerator {
       targetScala3: Boolean,
       queryParamRefs: Set[String],
       jsonSerdeLib: JsonSerdeLib.JsonSerdeLib,
-      jsonParamRefs: Set[String]
+      jsonParamRefs: Set[String],
+      alwaysGenerateParamSupport: Boolean
   ): Seq[String] = {
     def maybeEscaped(s: String) = s match {
       case legalEnumName(l) => l
       case illegal          => s"`$illegal`"
     }
     if (targetScala3) {
       val maybeCompanion =
-        if (queryParamRefs contains name) {
+        if (alwaysGenerateParamSupport || queryParamRefs.contains(name)) {
           def helperImpls =
             s"""  given enumCodecSupport${name.capitalize}: ExtraParamSupport[$name] =
                |    extraCodecSupport[$name]""".stripMargin
@@ -32,14 +33,22 @@ object EnumGenerator {
              |}""".stripMargin
         } else ""
       val maybeCodecExtensions = jsonSerdeLib match {
-        case _ if !jsonParamRefs.contains(name) && !queryParamRefs.contains(name) => ""
-        case _ if !jsonParamRefs.contains(name)                                   => " derives enumextensions.EnumMirror"
-        case JsonSerdeLib.Circe if !queryParamRefs.contains(name) => " derives org.latestbit.circe.adt.codec.JsonTaggedAdt.PureCodec"
-        case JsonSerdeLib.Circe => " derives org.latestbit.circe.adt.codec.JsonTaggedAdt.PureCodec, enumextensions.EnumMirror"
-        case JsonSerdeLib.Jsoniter if !queryParamRefs.contains(name) => ""
-        case JsonSerdeLib.Jsoniter                                   => " derives enumextensions.EnumMirror"
-        case JsonSerdeLib.Zio if !queryParamRefs.contains(name)      => " derives zio.json.JsonCodec"
-        case JsonSerdeLib.Zio                                        => " derives zio.json.JsonCodec, enumextensions.EnumMirror"
+        case _ if !alwaysGenerateParamSupport && !jsonParamRefs.contains(name) && !queryParamRefs.contains(name) =>
+          ""
+        case _ if !alwaysGenerateParamSupport && !jsonParamRefs.contains(name) =>
+          " derives enumextensions.EnumMirror"
+        case JsonSerdeLib.Circe if !alwaysGenerateParamSupport && !queryParamRefs.contains(name) =>
+          " derives org.latestbit.circe.adt.codec.JsonTaggedAdt.PureCodec"
+        case JsonSerdeLib.Circe =>
+          " derives org.latestbit.circe.adt.codec.JsonTaggedAdt.PureCodec, enumextensions.EnumMirror"
+        case JsonSerdeLib.Jsoniter if !alwaysGenerateParamSupport && !queryParamRefs.contains(name) =>
+          ""
+        case JsonSerdeLib.Jsoniter =>
+          " derives enumextensions.EnumMirror"
+        case JsonSerdeLib.Zio if !alwaysGenerateParamSupport && !queryParamRefs.contains(name) =>
+          " derives zio.json.JsonCodec"
+        case JsonSerdeLib.Zio =>
+          " derives zio.json.JsonCodec, enumextensions.EnumMirror"
       }
       s"""$maybeCompanion
          |enum $name$maybeCodecExtensions {
@@ -48,12 +57,15 @@ object EnumGenerator {
     } else {
       val members = obj.items.map { i => s"case object ${maybeEscaped(i.value)} extends $name" }
       val maybeCodecExtension = jsonSerdeLib match {
-        case _ if !jsonParamRefs.contains(name) && !queryParamRefs.contains(name) => ""
-        case JsonSerdeLib.Circe                                                   => s" with enumeratum.CirceEnum[$name]"
-        case JsonSerdeLib.Jsoniter | JsonSerdeLib.Zio                             => ""
+        case _ if !alwaysGenerateParamSupport && !jsonParamRefs.contains(name) && !queryParamRefs.contains(name) =>
+          ""
+        case JsonSerdeLib.Circe =>
+          s" with enumeratum.CirceEnum[$name]"
+        case JsonSerdeLib.Jsoniter | JsonSerdeLib.Zio =>
+          ""
       }
       val maybeQueryCodecDefn =
-        if (queryParamRefs contains name) {
+        if (alwaysGenerateParamSupport || queryParamRefs.contains(name)) {
           s"""
                |  implicit val enumCodecSupport${name.capitalize}: ExtraParamSupport[$name] =
                |    extraCodecSupport[$name]("${name}", ${name})""".stripMargin