Skip to content

New tapiro imports #373

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,17 @@ import scala.meta.internal.io.PlatformFileIO
object Metarpheus {

def run(paths: List[String], config: Config): intermediate.API = {
val files = paths
.flatMap(path => PlatformFileIO.listAllFilesRecursively(AbsolutePath(path)))
.filter(_.toString.endsWith(".scala"))
val parsed = files.map(File(_).parse[Source].get)
val parsed = parseFiles(paths)
extractors
.extractFullAPI(parsed = parsed)
.stripUnusedModels(config.modelsForciblyInUse, config.discardRouteErrorModels)
}

def parseFiles(paths: List[String]) : List[Source] = {
val files = paths
.flatMap(path => PlatformFileIO.listAllFilesRecursively(AbsolutePath(path)))
.filter(_.toString.endsWith(".scala"))
files.map(File(_).parse[Source].get)
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,11 @@ package object extractors {
intermediate.API(models, routes)
}

def extractImports(source: scala.meta.Source): List[scala.meta.Import] =
source.collect {
case imp: scala.meta.Import => imp
}

/**
* Extract all terms from a sequence of applications of an infix operator
* (which translates to nested `ApplyInfix`es).
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import scala.meta._
object AkkaHttpMeta {
val `class` = (
`package`: Term.Ref,
imports: Set[Term.Ref],
imports: Set[Import],
controllerName: Type.Name,
tapirEndpointsName: Term.Name,
authTokenName: Type.Name,
Expand All @@ -21,7 +21,7 @@ object AkkaHttpMeta {
val tapirEndpoints = q"val endpoints = $tapirEndpointsName.create[$authTokenName](statusCodes)"
q"""
package ${`package`} {
..${imports.toList.sortWith(_.toString < _.toString).map(i => q"import $i._")}
..${imports.toList.sortWith(_.toString < _.toString)}
import akka.http.scaladsl.server._
import akka.http.scaladsl.server.Directives._
import io.circe.{ Decoder, Encoder }
Expand Down
4 changes: 2 additions & 2 deletions tapiro/core/src/main/scala/io/buildo/tapiro/Http4sMeta.scala
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import scala.meta._
object Http4sMeta {
val `class` = (
`package`: Term.Ref,
imports: Set[Term.Ref],
imports: Set[Import],
controllerName: Type.Name,
tapirEndpointsName: Term.Name,
authTokenName: Type.Name,
Expand All @@ -21,7 +21,7 @@ object Http4sMeta {
val tapirEndpoints = q"val endpoints = $tapirEndpointsName.create[$authTokenName](statusCodes)"
q"""
package ${`package`} {
..${imports.toList.sortWith(_.toString < _.toString).map(i => q"import $i._")}
..${imports.toList.sortWith(_.toString < _.toString)}
import cats.effect._
import cats.implicits._
import cats.data.NonEmptyList
Expand Down
27 changes: 25 additions & 2 deletions tapiro/core/src/main/scala/io/buildo/tapiro/Meta.scala
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,29 @@ import scala.meta.contrib._
import cats.data.NonEmptyList

object Meta {

val controllerImports =(routes: List[TapiroRoute],imports: Set[Importer],outputPackage:String) => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function is a bit hard to read. I'll suggest a few small changes below, but also: how much of the complexity is done to avoid redundant imports? Since we're getting redundant imports anyway, maybe we can simplify a bit even if we get more of them? (and then either remove them with scalafix or suppress them during compilation)

val types = routes.flatMap(tr => tr.route.params.map(r=> r.tpe) ++ (if (tr.route.error.isDefined) List(tr.route.error.get,tr.route.returns) else List(tr.route.returns)))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
val types = routes.flatMap(tr => tr.route.params.map(r=> r.tpe) ++ (if (tr.route.error.isDefined) List(tr.route.error.get,tr.route.returns) else List(tr.route.returns)))
val types = routes.flatMap(tr => tr.route.params.map(_.tpe) ++ tr.route.error.toList ++ List(tr.route.returns)))

should be equivalent I think?

val typeNameList = types.flatMap(typeNameExtractor(_)).toSet
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
val typeNameList = types.flatMap(typeNameExtractor(_)).toSet
val typeNameList = types.flatMap(typeNameExtractor).toSet

val (wildcardImportList,importList) = imports.collect{
case i : Importer if i.syntax.endsWith("._") => List(i)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we could extract a function isWildcardImport instead of repeating this check here and on line 18?

case Importer(ref,importees) => importees.filter(p=> typeNameList.contains(p.syntax)).map(p=> Importer(ref,List(p)))
}.flatten.partition(_.syntax.endsWith("._"))
val controllerPackageStrings = routes.map(r=> s"import ${r.route.controllerPackage.mkString(".")}._")
val controllerPackage : List[Import] =controllerPackageStrings.flatMap(_.parse[Source].getOrElse(Source(List())).tree.children).collect{case i: Import => i}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a bit strange since we're first building the strings and then constructing the imports by parsing the strings. I think it would be nicer to build the imports directly using quasiquotes (and maybe extract the strings from them). But I'm not sure if this is too cumbersome, otherwise it's fine to leave as is.

val importers = (if (importList.flatMap(i=> typeNameList.diff(i.importees.map(_.syntax).toSet)).isEmpty) importList else {
(wildcardImportList ++ importList)
}).map(i=>Import(List(i)))
val result= if (controllerPackageStrings.filter(_.endsWith(outputPackage+"._")).isEmpty) controllerPackage ++ importers else importers
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe use forall or exists instead of filter/isEmpty?

Also, I'm not sure I've understood what the condition in the if means, maybe we can rewrite to

val someMeaningfulNameForTheCondition = controllerPackageStrings.filter(_.endsWith(outputPackage+"._")
val result= if (someMeaningfulNameForTheCondition) controllerPackage ++ importers else importers

using some meaningful name to make this more understandable.

deduplicate(result.toList).toSet
} : Set[Import]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's move the type annotation to after the arguments


def typeNameExtractor (tpe : MetarpheusType) : Set[String]=
tpe match {
case MetarpheusType.Name(name) => Set(name)
case MetarpheusType.Apply(head,args) => (head :: args.map(typeNameExtractor(_)).flatten.toList).toSet
}

val codecsImplicits = (routes: List[TapiroRoute], authTokenName: String) => {
val notUnit = (t: MetarpheusType) => t != MetarpheusType.Name("Unit")
val toDecoder = (t: Type) => t"Decoder[${extractListType(t)}]"
Expand Down Expand Up @@ -44,10 +67,10 @@ object Meta {
case _ => t
}

private[this] val deduplicate: List[Type] => List[Type] = (ts: List[Type]) =>
private[this] def deduplicate[A<:Tree](ts: List[A]): List[A] =
ts match {
case Nil => Nil
case head :: tail => head :: deduplicate(tail.filter(!_.isEqual(head)))
case head :: tail => head :: deduplicate(tail.filter(!_.syntax.equals(head.syntax)))
}

private[this] val isAuthToken = (t: MetarpheusType, authTokenName: String) => t == MetarpheusType.Name(authTokenName)
Expand Down
4 changes: 2 additions & 2 deletions tapiro/core/src/main/scala/io/buildo/tapiro/TapirMeta.scala
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ object TapirMeta {

val `class` = (
`package`: Term.Ref,
imports: Set[Term.Ref],
imports: Set[Import],
tapirEndpointsName: Term.Name,
authTokenName: Type.Name,
implicits: List[Term.Param],
Expand All @@ -26,7 +26,7 @@ object TapirMeta {
Type.Param(List(), authTokenName, List(), Type.Bounds(None, None), List(), List())
q"""
package ${`package`} {
..${imports.toList.sortWith(_.toString < _.toString).map(i => q"import $i._")}
..${imports.toList.sortWith(_.toString < _.toString)}
import io.circe.{ Decoder, Encoder }
import io.circe.generic.semiauto.{ deriveDecoder, deriveEncoder }
import sttp.tapir._
Expand Down
41 changes: 14 additions & 27 deletions tapiro/core/src/main/scala/io/buildo/tapiro/Util.scala
Original file line number Diff line number Diff line change
@@ -1,9 +1,7 @@
package io.buildo.tapiro

import io.buildo.metarpheus.core.extractors._
import io.buildo.metarpheus.core.{Config, Metarpheus}
import io.buildo.metarpheus.core.intermediate.{
CaseClass,
CaseEnum,
Route,
TaggedUnion,
Type => MetarpheusType,
Expand Down Expand Up @@ -55,7 +53,7 @@ class Util() {
NonEmptyList.fromList(`package`) match {
case Some(nonEmptyPackage) =>
val config = Config(Set.empty)
val models = Metarpheus.run(modelsPaths, config).models
val models = Metarpheus.run(modelsPaths, config).models
//this is needed because Metarpheus removes auth from params when authentication type is "Auth"
//see https://github.com/buildo/retro/blob/dfe62fa54d4f34c1861d694ac0cd8fa82f0a8703/metarpheus/core/src/main/scala/io.buildo.metarpheus/core/extractors/controller.scala#L35
val routesWithAuthParams: List[Route] = Metarpheus
Expand All @@ -74,41 +72,30 @@ class Util() {
)
else r,
})
val routes: List[TapiroRoute] =
val routes: List[TapiroRoute] =
routesWithAuthParams.map(toTapiroRoute(models))
val imports = Metarpheus.parseFiles(routesPaths).map(extractImports).flatten.map(_.importers).flatten.toSet
val controllersRoutes =
routes.groupBy(
route => (route.route.controllerType, route.route.pathName),
)
val modelsPackages = models.map {
case c: CaseClass => c.`package`
case c: CaseEnum => c.`package`
case t: TaggedUnion => t.`package`
}.collect {
case head :: tail => NonEmptyList(head, tail)
}
controllersRoutes.foreach {
case ((controllerType, pathName), routes) =>
val controllerName = typeNameString(controllerType)
val authTypeString = Meta.authTypeString(controllerType).getOrElse("AuthToken")
val pathNameOrController = pathName.getOrElse(controllerName)
val tapirEndpointsName = s"${pathNameOrController}TapirEndpoints".capitalize
val httpEndpointsName = s"${pathNameOrController}HttpEndpoints".capitalize
val controllerImports = Meta.controllerImports(routes,imports,nonEmptyPackage.toList.mkString("."))
val tapirEndpoints =
createTapirEndpoints(
tapirEndpointsName,
authTypeString,
routes,
nonEmptyPackage,
modelsPackages,
controllerImports,
)
writeToFile(outputPath, tapirEndpoints, tapirEndpointsName)

val routesPackages = routes
.map(_.route.controllerPackage)
.collect {
case head :: tail => NonEmptyList(head, tail)
}
server match {
case Server.Http4s =>
val http4sEndpoints =
Expand All @@ -119,7 +106,7 @@ class Util() {
tapirEndpointsName,
authTypeString,
httpEndpointsName,
modelsPackages ++ routesPackages,
controllerImports,
routes,
)
http4sEndpoints.foreach(writeToFile(outputPath, _, httpEndpointsName))
Expand All @@ -132,7 +119,7 @@ class Util() {
tapirEndpointsName,
authTypeString,
httpEndpointsName,
modelsPackages ++ routesPackages,
controllerImports,
routes,
)
akkaHttpEndpoints.foreach(
Expand All @@ -150,12 +137,12 @@ class Util() {
authTokenName: String,
routes: List[TapiroRoute],
`package`: NonEmptyList[String],
requiredPackages: List[NonEmptyList[String]],
requiredPackages: Set[Import],
): String = {
format(
TapirMeta.`class`(
Meta.packageFromList(`package`),
requiredPackages.toSet.map(Meta.packageFromList),
requiredPackages,
Term.Name(tapirEndpointsName),
Type.Name(authTokenName),
Meta.codecsImplicits(routes, authTokenName),
Expand All @@ -173,7 +160,7 @@ class Util() {
tapirEndpointsName: String,
authTokenName: String,
httpEndpointsName: String,
requiredPackages: List[NonEmptyList[String]],
requiredPackages: Set[Import],
tapiroRoutes: List[TapiroRoute],
): Option[String] = {
val routes = tapiroRoutes.map(_.route)
Expand All @@ -184,7 +171,7 @@ class Util() {
format(
Http4sMeta.`class`(
Meta.packageFromList(`package`),
requiredPackages.toSet.map(Meta.packageFromList),
requiredPackages,
Type.Name(controllerName),
Term.Name(tapirEndpointsName),
Type.Name(authTokenName),
Expand All @@ -206,7 +193,7 @@ class Util() {
tapirEndpointsName: String,
authTokenName: String,
httpEndpointsName: String,
requiredPackages: List[NonEmptyList[String]],
requiredPackages: Set[Import],
tapiroRoutes: List[TapiroRoute],
): Option[String] = {
val routes = tapiroRoutes.map(_.route)
Expand All @@ -217,7 +204,7 @@ class Util() {
format(
AkkaHttpMeta.`class`(
Meta.packageFromList(`package`),
requiredPackages.toSet.map(Meta.packageFromList),
requiredPackages,
Type.Name(controllerName),
Term.Name(tapirEndpointsName),
Type.Name(authTokenName),
Expand Down