From a7e3f9ada2abe2a990cb2cc4156c42d25f738f94 Mon Sep 17 00:00:00 2001 From: Dmytro Obodowsky Date: Mon, 4 Dec 2023 10:40:11 -0600 Subject: [PATCH] Address code review comments. Not all modules should be cross compiled --- build.sc | 48 ++++++++++++++----- .../src/internals/ApiResponseToParams.scala | 6 +-- .../openapi/src/internals/GetExtensions.scala | 5 +- .../src/internals/HeadersToParams.scala | 2 +- .../src/internals/IModelToSmithy.scala | 5 -- .../src/internals/OpenApiToIModel.scala | 4 +- .../src/internals/ParseOperations.scala | 6 +-- .../postprocess/AllOfTransformer.scala | 27 +++++------ 8 files changed, 62 insertions(+), 41 deletions(-) diff --git a/build.sc b/build.sc index 8c2cddf..90d361b 100644 --- a/build.sc +++ b/build.sc @@ -107,8 +107,16 @@ trait BasePublishModule extends BaseModule with CiReleaseModule { } } +trait Scala213VersionModule extends ScalaModule with ScalafmtModule { + override def scalaVersion = T.input("2.13.12") + + def scalacOptions = T { + super.scalacOptions() ++ scalacOptionsFor(scalaVersion()) + } +} + trait ScalaVersionModule extends CrossScalaModule with ScalafmtModule { -// override def scalaVersion = T.input("2.13.12") + override def scalaVersion = T.input("2.13.12") def scalacOptions = T { super.scalacOptions() ++ scalacOptionsFor(scalaVersion()) @@ -122,7 +130,20 @@ trait BaseScalaNoPublishModule extends BaseModule with ScalaVersionModule { ) } +trait BaseScala213NoPublishModule + extends BaseModule + with Scala213VersionModule { + + override def scalacPluginIvyDeps = super.scalacPluginIvyDeps() ++ Agg( + ivy"org.typelevel:::kind-projector:0.13.2" + ) +} + trait BaseScalaModule extends BaseScalaNoPublishModule with BasePublishModule +trait BaseScala213Module + extends BaseScala213NoPublishModule + with BasePublishModule + trait BaseScalaJSModule extends BaseScalaModule with ScalaJSModule { def scalaJSVersion = "1.11.0" def moduleKind = ModuleKind.CommonJSModule @@ -182,8 +203,7 @@ trait OpenApiModule extends BaseScalaModule { } } -object cli extends Cross[CliModule](scalaVersions) -trait CliModule extends BaseScalaModule with buildinfo.BuildInfo { +object cli extends BaseScala213Module with buildinfo.BuildInfo { def ivyDeps = Agg( Deps.decline, Deps.coursier, @@ -204,7 +224,12 @@ trait CliModule extends BaseScalaModule with buildinfo.BuildInfo { ) def moduleDeps = - Seq(openapi(), proto.core(), `json-schema`(), formatter.jvm()) + Seq( + openapi("2.13.12"), + proto.core("2.13.12"), + `json-schema`("2.13.12"), + formatter.jvm("2.13.12") + ) def runProtoAux = T.task { (inputs: List[Path], output: Path) => val inputArgs = inputs.flatMap { p => @@ -331,10 +356,11 @@ object traits extends BaseJavaModule { with BaseMunitTests } -object `readme-validator` - extends Cross[`readme-validator-module`](scalaVersions) -trait `readme-validator-module` extends BaseScalaNoPublishModule { - def moduleDeps = Seq(openapi(), proto.core(), `json-schema`()) +//object `readme-validator` +// extends Cross[`readme-validator-module`](scalaVersions) +object `readme-validator` extends BaseScala213NoPublishModule { + def moduleDeps = + Seq(openapi("2.13.12"), proto.core("2.13.12"), `json-schema`("2.13.12")) def ivyDeps = Agg( Deps.cats.parse, @@ -394,9 +420,7 @@ object proto extends Module { } } - object examples extends Cross[ExamplesModule](scalaVersions) - - trait ExamplesModule extends BaseScalaModule with ScalaPBModule { + object examples extends BaseScala213Module with ScalaPBModule { def scalaPBVersion = Deps.scalapb.version def smithyFiles = T.sources { @@ -415,7 +439,7 @@ object proto extends Module { os.remove.all(cliRunOutput) os.makeDir.all(cliRunOutput) val input = smithyFiles().toList.map(_.path) - val f = cli().runProtoAux() + val f = cli.runProtoAux() f(input, cliRunOutput) cliRunOutput } diff --git a/modules/openapi/src/internals/ApiResponseToParams.scala b/modules/openapi/src/internals/ApiResponseToParams.scala index 36be7ea..f7cc6c7 100644 --- a/modules/openapi/src/internals/ApiResponseToParams.scala +++ b/modules/openapi/src/internals/ApiResponseToParams.scala @@ -39,7 +39,7 @@ object ApiResponseToParams case (contentType, bodySchema: Schema[_]) :: Nil => // TODO: figure out how to deal with reflective calls in scala 2.12 val bodyExts = - GetExtensions.from(bodySchema.asInstanceOf[HasExtensions]) + GetExtensions.from(HasExtensions.unsafeFrom(bodySchema)) Some( Param( "body", @@ -54,7 +54,7 @@ object ApiResponseToParams case list => val bodySchema = ContentTypeDiscriminatedSchema(list.toMap) val bodyExts = - GetExtensions.from(bodySchema.asInstanceOf[HasExtensions]) + GetExtensions.from(HasExtensions.unsafeFrom(bodySchema)) Some( Param( "body", @@ -71,7 +71,7 @@ object ApiResponseToParams maybeRef match { case Some(ref) => Left(ref) case None => - val exts = GetExtensions.from(response.asInstanceOf[HasExtensions]) + val exts = GetExtensions.from(HasExtensions.unsafeFrom(response)) val httpMessageInfo = HttpMessageInfo(opName, allParams, exts) Right(httpMessageInfo) } diff --git a/modules/openapi/src/internals/GetExtensions.scala b/modules/openapi/src/internals/GetExtensions.scala index 8045668..cb63ee3 100644 --- a/modules/openapi/src/internals/GetExtensions.scala +++ b/modules/openapi/src/internals/GetExtensions.scala @@ -28,7 +28,7 @@ object GetExtensions { def transformPattern[A]( local: Local ): OpenApiPattern[A] => OpenApiPattern[A] = { - val maybeHints = from(local.schema.asInstanceOf[HasExtensions]) + val maybeHints = from(HasExtensions.unsafeFrom(local.schema)) (pattern: OpenApiPattern[A]) => pattern.mapContext(_.addHints(maybeHints, retainTopLevel = true)) } @@ -36,6 +36,9 @@ object GetExtensions { // Using reflective calls because openapi does not seem to have a common interface // that exposes the presence of extensions. type HasExtensions = { def getExtensions(): java.util.Map[String, Any] } + object HasExtensions { + def unsafeFrom(s: Any): HasExtensions = s.asInstanceOf[HasExtensions] + } def from(s: HasExtensions): List[Hint] = Option(s) diff --git a/modules/openapi/src/internals/HeadersToParams.scala b/modules/openapi/src/internals/HeadersToParams.scala index b3700fa..a85d6a0 100644 --- a/modules/openapi/src/internals/HeadersToParams.scala +++ b/modules/openapi/src/internals/HeadersToParams.scala @@ -26,7 +26,7 @@ object HeadersToParams extends (Iterable[(String, Header)] => Vector[Param]) { Option(headerMap).toVector.flatten.map { case (name, header) => val requiredHint = if (header.getRequired()) List(Hint.Required) else List.empty - val exts = GetExtensions.from(header.asInstanceOf[HasExtensions]) + val exts = GetExtensions.from(HasExtensions.unsafeFrom(header)) val hints = List(Hint.Header(name)) ++ requiredHint ++ exts val refOrSchema = Option(header.get$ref()) match { case Some(value) => Left(value) diff --git a/modules/openapi/src/internals/IModelToSmithy.scala b/modules/openapi/src/internals/IModelToSmithy.scala index a3221ac..050915c 100644 --- a/modules/openapi/src/internals/IModelToSmithy.scala +++ b/modules/openapi/src/internals/IModelToSmithy.scala @@ -60,11 +60,6 @@ final class IModelToSmithy(useEnumTraitSyntax: Boolean) List(Hint.JsonName(memName)) else List.empty -// ShapeBuilderOps[MemberShape.Builder, MemberShape](MemberShape -// .builder() -// .id(id.toSmithy) -// .target(tpe.toSmithy)).addHints(hints ++ jsonNameHint).build() - MemberShape .builder() .id(id.toSmithy) diff --git a/modules/openapi/src/internals/OpenApiToIModel.scala b/modules/openapi/src/internals/OpenApiToIModel.scala index 27edc84..b74b09d 100644 --- a/modules/openapi/src/internals/OpenApiToIModel.scala +++ b/modules/openapi/src/internals/OpenApiToIModel.scala @@ -372,9 +372,9 @@ private class OpenApiToIModel[F[_]: Parallel: TellShape: TellError]( val schemes = securitySchemes.values.toVector val securityHint = if (schemes.nonEmpty) List(Hint.Security(schemes)) else Nil - val exts = GetExtensions.from(openApi.asInstanceOf[HasExtensions]) + val exts = GetExtensions.from(HasExtensions.unsafeFrom(openApi)) val infoExts = - GetExtensions.from(openApi.getInfo().asInstanceOf[HasExtensions]) + GetExtensions.from(HasExtensions.unsafeFrom(openApi.getInfo())) val externalDocs = Option(openApi.getExternalDocs()).map(e => Hint.ExternalDocs(Option(e.getDescription()), e.getUrl()) ) diff --git a/modules/openapi/src/internals/ParseOperations.scala b/modules/openapi/src/internals/ParseOperations.scala index 2907e13..55903ea 100644 --- a/modules/openapi/src/internals/ParseOperations.scala +++ b/modules/openapi/src/internals/ParseOperations.scala @@ -192,7 +192,7 @@ private class ParseOperationsImpl( ) val hints = GetExtensions.from( - opInfo.op.asInstanceOf[HasExtensions] + HasExtensions.unsafeFrom(opInfo.op) ) ++ securityHint ++ descHint ++ exDocs val suppressions = getHeaderSuppressions(allValidInputParams).toVector ParseOperationsResult( @@ -252,7 +252,7 @@ private class ParseOperationsImpl( Option( op.getRequestBody() ).flatMap { rb => - val exts = GetExtensions.from(rb.asInstanceOf[HasExtensions]) + val exts = GetExtensions.from(HasExtensions.unsafeFrom(rb)) val bodies = ContentToSchemaOpt(rb.getContent()) val (bodyHints, maybeSchema) = bodies.toList match { @@ -295,7 +295,7 @@ private class ParseOperationsImpl( maybeResolvedParam.flatMap { resolvedParam => val name = resolvedParam.getName() - val exts = GetExtensions.from(resolvedParam.asInstanceOf[HasExtensions]) + val exts = GetExtensions.from(HasExtensions.unsafeFrom(resolvedParam)) val httpBinding = resolvedParam.getIn() match { case "query" => Some(Hint.QueryParam(name)) case "path" => Some(Hint.PathParam(name)) diff --git a/modules/openapi/src/internals/postprocess/AllOfTransformer.scala b/modules/openapi/src/internals/postprocess/AllOfTransformer.scala index d7a6cbc..a1951f3 100644 --- a/modules/openapi/src/internals/postprocess/AllOfTransformer.scala +++ b/modules/openapi/src/internals/postprocess/AllOfTransformer.scala @@ -142,13 +142,12 @@ object AllOfTransformer extends IModelPostProcessor { private def moveParentFieldsAndCreateMixins( all: Vector[Definition] ): Vector[Definition] = { - val allShapes: mutable.LinkedHashMap[DefId, Definition] = - all - .map(a => a.id -> a) - .foldLeft(new mutable.LinkedHashMap[DefId, Definition]()) { - case (acc, (id, shape)) => - acc += (id -> shape) - } + val allShapes = new mutable.LinkedHashMap[DefId, Definition]() + all + .map(a => a.id -> a) + .foreach { case (id, shape) => + allShapes += (id -> shape) + } all.foreach { d => // get latest in case modifications have been made to this definition since the @@ -219,15 +218,15 @@ object AllOfTransformer extends IModelPostProcessor { val unused = isAMixin.diff(usedAsMixins) + val idToDef = new mutable.ArrayBuffer[Definition]() allShapes - .foldLeft(new mutable.LinkedHashMap[DefId, Definition]()) { - case (acc, (id, shp)) => - acc += (if (unused(id)) - (id, shp.mapHints(_.filterNot(_ == Hint.IsMixin))) - else (id, shp)) + .foreach { case (id, shp) => + idToDef += (if (unused(id)) + shp.mapHints(_.filterNot(_ == Hint.IsMixin)) + else shp) } - .values - .toVector + + idToDef.toVector } private def transform(in: IModel): Vector[Definition] = {