Skip to content

Commit

Permalink
Merge pull request #83 from twilio/scalameta-bugfix
Browse files Browse the repository at this point in the history
Scalameta bugfix
  • Loading branch information
blast-hardcheese authored Jul 31, 2018
2 parents 5a11f90 + 1629ce6 commit 0c679dd
Show file tree
Hide file tree
Showing 11 changed files with 79 additions and 155 deletions.
2 changes: 1 addition & 1 deletion build.sbt
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ val codegenSettings = Seq(
Resolver.bintrayRepo("scalameta", "maven")
),
libraryDependencies ++= testDependencies ++ Seq(
"org.scalameta" %% "scalameta" % "3.7.4",
"org.scalameta" %% "scalameta" % "4.0.0-M7",
"io.swagger" % "swagger-parser" % "1.0.34",
"org.tpolecat" %% "atto-core" % "0.6.1",
"org.typelevel" %% "cats-core" % catsVersion,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ object ClientGenerator {
client
)

Client(pkg, clientName, stats.map(SwaggerUtil.escapeTree))
Client(pkg, clientName, stats)
}
})
} yield Clients(clients)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,9 @@ object Common {

packageObject = WriteTree(
dtoPackagePath.resolve("package.scala"),
source"""package ${Term.Name(dtoComponents.dropRight(1).mkString("."))}
source"""package ${dtoComponents.init.tail.foldLeft[Term.Ref](Term.Name(dtoComponents.head)) {
case (acc, next) => Term.Select(acc, Term.Name(next))
}}
..${customImports ++ packageObjectImports ++ protocolImports}

package object ${Term.Name(dtoComponents.last)} {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ object ProtocolGenerator {

defn <- renderClass(clsName, tpe)
companion <- renderCompanion(clsName, members, accessors, values, encoder, decoder)
} yield EnumDefinition(clsName, Type.Name(clsName), elems, SwaggerUtil.escapeTree(defn), SwaggerUtil.escapeTree(companion))
} yield EnumDefinition(clsName, Type.Name(clsName), elems, defn, companion)
}

for {
Expand Down Expand Up @@ -109,7 +109,7 @@ object ProtocolGenerator {
encoder <- encodeModel(clsName, needCamelSnakeConversion, params)
decoder <- decodeModel(clsName, needCamelSnakeConversion, params)
cmp <- renderDTOCompanion(clsName, List.empty, encoder, decoder)
} yield ClassDefinition(clsName, Type.Name(clsName), SwaggerUtil.escapeTree(defn), SwaggerUtil.escapeTree(cmp))
} yield ClassDefinition(clsName, Type.Name(clsName), defn, cmp)
}

for {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ object ServerGenerator {
responseDefinitions,
renderedRoutes.flatMap(_.supportDefinitions))
} yield {
Server(className, frameworkImports ++ extraImports, List(SwaggerUtil.escapeTree(handlerSrc), SwaggerUtil.escapeTree(classSrc)))
Server(className, frameworkImports ++ extraImports, List(handlerSrc, classSrc))
}
}
} yield Servers(servers)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -222,14 +222,15 @@ object SwaggerUtil {
t
}
def liftCustomType(s: String): Option[Type] = {
val terms = s.split('.').toList
val (init, last) = (terms.init, terms.last)
init.map(Term.Name.apply _) match {
case Nil if last == "" => None
case Nil => Some(Type.Name(last))
case rest =>
Some(Type.Select(rest.reduceLeft(Term.Select.apply _), Type.Name(last)))
}
val tpe = s.trim
if (tpe.nonEmpty) {
tpe
.parse[Type]
.fold({ err =>
println(s"Warning: Unparsable x-scala-type: ${tpe} ${err}")
None
}, Option.apply _)
} else None
}

customType.flatMap(liftCustomType _).getOrElse {
Expand All @@ -256,82 +257,6 @@ object SwaggerUtil {
}
}

def escapeTree[T <: Tree]: T => T =
_.transform({
case Term.Name(name) => Term.Name(escapeReserved(name))
case p @ Term.Param(_, Term.Name(name), _, _) =>
p.copy(name = Term.Name(escapeReserved(name)))
case Type.Name(name) => Type.Name(escapeReserved(name))
case ctor @ Init(Type.Name(name), _, _) if name != "this" =>
ctor.copy(tpe = Type.Name(escapeReserved(name))) // Literal "this" in ctor names is OK
}).asInstanceOf[T]

val unbacktick = "^`(.*)`$".r
val leadingNumeric = "^[0-9\"]".r
val invalidSymbols = "[-`\"'()\\.]".r
val reservedWords = Set(
"abstract",
"case",
"catch",
"class",
"def",
"do",
"else",
"extends",
"false",
"final",
"finally",
"for",
"forSome",
"if",
"implicit",
"import",
"lazy",
"macro",
"match",
"new",
"null",
"object",
"override",
"package",
"private",
"protected",
"return",
"sealed",
"super",
"this",
"throw",
"trait",
"try",
"true",
"type",
"val",
"var",
"while",
"with",
"yield",
"_",
":",
"=",
"=>",
"<-",
"<:",
"<%",
">:",
"#",
"@"
)

def escapeReserved: String => String = {
case name if unbacktick.findFirstMatchIn(name).nonEmpty => name
case name if name.contains(' ') =>
name // scala.meta will automatically escape. See `EscapeTreeSpec.scala`
case name if reservedWords.contains(name) => s"`${name}`"
case name if invalidSymbols.findFirstMatchIn(name).nonEmpty => s"`${name}`"
case name if leadingNumeric.findFirstMatchIn(name).nonEmpty => s"`${name}`"
case name => name
}

def propMeta[T <: Property](property: T): Target[ResolvedType] =
Target.getGeneratorSettings.flatMap { implicit gs =>
property match {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,20 +62,16 @@ object AkkaHttpGenerator {
object TextPlain {
def apply(value: String): TextPlain = new TextPlain(value)
implicit final def textTEM: ToEntityMarshaller[TextPlain] =
Marshaller.withFixedContentType(ContentTypes.${Term
.Name("`text/plain(UTF-8)`")}) { text =>
HttpEntity(ContentTypes.${Term
.Name("`text/plain(UTF-8)`")}, text.value)
Marshaller.withFixedContentType(ContentTypes.`text/plain(UTF-8)`) { text =>
HttpEntity(ContentTypes.`text/plain(UTF-8)`, text.value)
}
}

implicit final def jsonMarshaller(
implicit printer: Printer = Printer.noSpaces
): ToEntityMarshaller[${gs.jsonType}] =
Marshaller.withFixedContentType(MediaTypes.${Term
.Name("`application/json`")}) { json =>
HttpEntity(MediaTypes.${Term
.Name("`application/json`")}, printer.pretty(json))
Marshaller.withFixedContentType(MediaTypes.`application/json`) { json =>
HttpEntity(MediaTypes.`application/json`, printer.pretty(json))
}

implicit final def jsonEntityMarshaller[A](
Expand All @@ -86,7 +82,7 @@ object AkkaHttpGenerator {

implicit final val jsonUnmarshaller: FromEntityUnmarshaller[${gs.jsonType}] =
Unmarshaller.byteStringUnmarshaller
.forContentTypes(MediaTypes.${Term.Name("`application/json`")})
.forContentTypes(MediaTypes.`application/json`)
.map {
case ByteString.empty => throw Unmarshaller.NoContentException
case data => jawn.parseByteBuffer(data.asByteBuffer).valueOr(throw _)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,7 @@ object ScalaParameter {
val Term.Name(name) = param.paramName
if (counts.getOrElse(name, 0) > 1) {
val escapedName =
Term.Name(SwaggerUtil.escapeReserved(param.argName.value))
Term.Name(param.argName.value)
new ScalaParameter(
param.in,
param.param.copy(name = escapedName),
Expand Down
53 changes: 23 additions & 30 deletions src/test/scala/tests/core/BacktickTest.scala
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ class BacktickTest extends FunSuite with Matchers with SwaggerSpecRunner {
val List(cmp, cls) = statements.dropWhile(_.isInstanceOf[Import])

val client = q"""
class ${Type.Name("`Dashy-packageClient`")}(host: String = "http://localhost:1234")(implicit httpClient: HttpRequest => Future[HttpResponse], ec: ExecutionContext, mat: Materializer) {
class `Dashy-packageClient`(host: String = "http://localhost:1234")(implicit httpClient: HttpRequest => Future[HttpResponse], ec: ExecutionContext, mat: Materializer) {
val basePath: String = ""
private[this] def wrap[T: FromEntityUnmarshaller](resp: Future[HttpResponse]): EitherT[Future, Either[Throwable, HttpResponse], T] = {
EitherT(resp.flatMap(resp => if (resp.status.isSuccess) {
Expand All @@ -89,17 +89,15 @@ class BacktickTest extends FunSuite with Matchers with SwaggerSpecRunner {
Left(Left(e))
}))
}
def ${Term.Name("`dashy-op-id`")}(${Term.Name("dashyParameter")}: String, headers: scala.collection.immutable.Seq[HttpHeader] = Nil): EitherT[Future, Either[Throwable, HttpResponse], ${Type
.Name("`dashy-class`")}] = {
def `dashy-op-id`(dashyParameter: String, headers: scala.collection.immutable.Seq[HttpHeader] = Nil): EitherT[Future, Either[Throwable, HttpResponse], `dashy-class`] = {
val allHeaders = headers ++ scala.collection.immutable.Seq[Option[HttpHeader]]().flatten
wrap[${Type.Name("`dashy-class`")}](Marshal(HttpEntity.Empty).to[RequestEntity].flatMap { entity =>
wrap[`dashy-class`](Marshal(HttpEntity.Empty).to[RequestEntity].flatMap { entity =>
httpClient(HttpRequest(method = HttpMethods.GET, uri = host + basePath + "/dashy-route" + "?" + Formatter.addArg("dashy-parameter", dashyParameter), entity = entity, headers = allHeaders))
})
}
def ${Term.Name("`postDashy-op-id`")}(dashyParameter: String, headers: scala.collection.immutable.Seq[HttpHeader] = Nil): EitherT[Future, Either[Throwable, HttpResponse], ${Type
.Name("`dashy-class`")}] = {
def `postDashy-op-id`(dashyParameter: String, headers: scala.collection.immutable.Seq[HttpHeader] = Nil): EitherT[Future, Either[Throwable, HttpResponse], `dashy-class`] = {
val allHeaders = headers ++ scala.collection.immutable.Seq[Option[HttpHeader]]().flatten
wrap[${Type.Name("`dashy-class`")}](Marshal(HttpEntity.Empty).to[RequestEntity].flatMap { entity =>
wrap[`dashy-class`](Marshal(HttpEntity.Empty).to[RequestEntity].flatMap { entity =>
httpClient(HttpRequest(method = HttpMethods.POST, uri = host + basePath + "/dashy-route" + "?" + Formatter.addArg("dashy-parameter", dashyParameter), entity = entity, headers = allHeaders))
})
}
Expand Down Expand Up @@ -133,17 +131,15 @@ class BacktickTest extends FunSuite with Matchers with SwaggerSpecRunner {
) = runSwaggerSpec(swagger)(Context.empty, AkkaHttp, defaults.akkaGeneratorSettings)

val definition = q"""
case class ${Type.Name("`dashy-class`")}(${Term.Name("`dashy-param`")}: Option[Long] = None)
case class `dashy-class`(`dashy-param`: Option[Long] = None)
"""
val companion = q"""
object ${Term.Name("`dashy-class`")} {
implicit val ${Pat.Var(Term.Name("`encodedashy-class`"))} = {
object `dashy-class` {
implicit val `encodedashy-class` = {
val readOnlyKeys = Set[String]()
Encoder.forProduct1("dashy-param")((o: ${Type.Name("`dashy-class`")}) => o.${Term
.Name("`dashy-param`")}).mapJsonObject(_.filterKeys(key => !(readOnlyKeys contains key)))
Encoder.forProduct1("dashy-param")((o: `dashy-class`) => o.`dashy-param`).mapJsonObject(_.filterKeys(key => !(readOnlyKeys contains key)))
}
implicit val ${Pat.Var(Term.Name("`decodedashy-class`"))} = Decoder.forProduct1("dashy-param")(${Term
.Name("`dashy-class`")}.apply _)
implicit val `decodedashy-class` = Decoder.forProduct1("dashy-param")(`dashy-class`.apply _)
}
"""

Expand All @@ -170,29 +166,26 @@ class BacktickTest extends FunSuite with Matchers with SwaggerSpecRunner {
) = runSwaggerSpec(swagger)(Context.empty, AkkaHttp, defaults.akkaGeneratorSettings)

val definition = q"""
sealed abstract class ${Type.Name("`dashy-enum`")}(val value: String) {
sealed abstract class `dashy-enum`(val value: String) {
override def toString: String = value.toString
}
"""
val companion = q"""
object ${Term.Name("`dashy-enum`")} {
object `dashy-enum` {
object members {
case object DashyValueA extends ${Type.Name("`dashy-enum`")}("dashy-value-a")
case object DashyValueB extends ${Type.Name("`dashy-enum`")}("dashy-value-b")
case object DashyValueC extends ${Type.Name("`dashy-enum`")}("dashy-value.c")
case object DashyValueA extends `dashy-enum`("dashy-value-a")
case object DashyValueB extends `dashy-enum`("dashy-value-b")
case object DashyValueC extends `dashy-enum`("dashy-value.c")
}
val DashyValueA: ${Type.Name("`dashy-enum`")} = members.DashyValueA
val DashyValueB: ${Type.Name("`dashy-enum`")} = members.DashyValueB
val DashyValueC: ${Type.Name("`dashy-enum`")} = members.DashyValueC
val DashyValueA: `dashy-enum` = members.DashyValueA
val DashyValueB: `dashy-enum` = members.DashyValueB
val DashyValueC: `dashy-enum` = members.DashyValueC
val values = Vector(DashyValueA, DashyValueB, DashyValueC)
def parse(value: String): Option[${Type.Name("`dashy-enum`")}] = values.find(_.value == value)
implicit val ${Pat.Var(Term.Name("`encodedashy-enum`"))}: Encoder[${Type
.Name("`dashy-enum`")}] = Encoder[String].contramap(_.value)
implicit val ${Pat.Var(Term.Name("`decodedashy-enum`"))}: Decoder[${Type
.Name("`dashy-enum`")}] = Decoder[String].emap(value => parse(value).toRight(s"$$value not a member of dashy-enum"))
implicit val ${Pat.Var(Term.Name("`addPathdashy-enum`"))}: AddPath[${Type
.Name("`dashy-enum`")}] = AddPath.build(_.value)
implicit val ${Pat.Var(Term.Name("`showdashy-enum`"))}: Show[${Type.Name("`dashy-enum`")}] = Show.build(_.value)
def parse(value: String): Option[`dashy-enum`] = values.find(_.value == value)
implicit val `encodedashy-enum`: Encoder[`dashy-enum`] = Encoder[String].contramap(_.value)
implicit val `decodedashy-enum`: Decoder[`dashy-enum`] = Decoder[String].emap(value => parse(value).toRight(s"$$value not a member of dashy-enum"))
implicit val `addPathdashy-enum`: AddPath[`dashy-enum`] = AddPath.build(_.value)
implicit val `showdashy-enum`: Show[`dashy-enum`] = Show.build(_.value)
}
"""

Expand Down
28 changes: 8 additions & 20 deletions src/test/scala/tests/core/EscapeTreeSpec.scala
Original file line number Diff line number Diff line change
Expand Up @@ -8,33 +8,21 @@ import scala.meta._
class EscapeTreeSpec extends FunSuite with Matchers {

test("Assume special characters are not escaped") {
// This test fails as of 1.6.0. If this changes in the future, SwaggerParser.escapeTermTree and friends can all be removed (:yey:)
val q"val $x = 3" = q"val `dashy-thing` = 3"
val x1 @ Pat.Var(Term.Name("dashy-thing")) = x
q"val $x1 = 3".toString shouldNot equal("val `dashy-thing` = 3") // shouldNot -> should (!) This is a bug in scala.meta!
q"val $x1 = 3".syntax should equal("val `dashy-thing` = 3")
}

test("Fiddly cases") {
// If either of the following start failing, this is because scalameta has
// changed the way Term.Names are interpolated. See notes in BacktickTest,
// and re-evaluate whether ' ' should be considered an invalid character
// (or if escapeTree can be removed altogether!)
Term.Name("post /dashy-path").toString shouldEqual ("`post /dashy-path`")
SwaggerUtil
.escapeTree(Term.Name("post /dashy-path"))
.toString shouldEqual ("`post /dashy-path`")
}

List[(Tree, Tree)](
(Init(Type.Name("dashy-enum"), Name("what"), List()), Init(Type.Name("`dashy-enum`"), Name("what"), List())),
(Term.Name("dashy-class"), Term.Name("`dashy-class`")),
List[(Tree, String)](
(Init(Type.Name("dashy-enum"), Name("what"), List()), "`dashy-enum`"),
(Term.Name("dashy-class"), "`dashy-class`"),
(Term.Param(Nil, Term.Name("dashy-param"), Some(Type.Apply(Type.Name("Option"), List(Type.Name("Long")))), Some(Term.Name("None"))),
Term.Param(Nil, Term.Name("`dashy-param`"), Some(Type.Apply(Type.Name("Option"), List(Type.Name("Long")))), Some(Term.Name("None")))),
(Type.Name("dashy-class"), Type.Name("`dashy-class`"))
"`dashy-param`: Option[Long] = None"),
(Type.Name("dashy-class"), "`dashy-class`")
).foreach {
case (x, y) =>
test(s"${x.structure} should be escaped as ${y.structure}") {
SwaggerUtil.escapeTree(x).toString shouldEqual (y.toString)
test(s"${x.structure} should be escaped as ${y}") {
x.syntax shouldEqual (y)
}
}
}
30 changes: 25 additions & 5 deletions src/test/scala/tests/core/TypesTest.scala
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,12 @@ class TypesTest extends FunSuite with Matchers with SwaggerSpecRunner {
| type: integer
| untyped:
| description: Untyped
| custom:
| type: string
| x-scala-type: Foo
| customComplex:
| type: string
| x-scala-type: Foo[Bar]
|""".stripMargin

test("Generate no definitions") {
Expand All @@ -68,18 +74,32 @@ class TypesTest extends FunSuite with Matchers with SwaggerSpecRunner {
) = runSwaggerSpec(swagger)(Context.empty, AkkaHttp, defaults.akkaGeneratorSettings)

val definition = q"""
case class Types(array: Option[IndexedSeq[Boolean]] = Option(IndexedSeq.empty), obj: Option[io.circe.Json] = None, bool: Option[Boolean] = None, string: Option[String] = None, date: Option[java.time.LocalDate] = None, dateTime: Option[java.time.OffsetDateTime] = None, long: Option[Long] = None, int: Option[Int] = None, float: Option[Float] = None, double: Option[Double] = None, number: Option[BigDecimal] = None, integer: Option[BigInt] = None, untyped: Option[io.circe.Json] = None)
case class Types(
array: Option[IndexedSeq[Boolean]] = Option(IndexedSeq.empty),
obj: Option[io.circe.Json] = None,
bool: Option[Boolean] = None,
string: Option[String] = None,
date: Option[java.time.LocalDate] = None,
date_time: Option[java.time.OffsetDateTime] = None,
long: Option[Long] = None,
int: Option[Int] = None,
float: Option[Float] = None,
double: Option[Double] = None,
number: Option[BigDecimal] = None,
integer: Option[BigInt] = None,
untyped: Option[io.circe.Json] = None,
custom: Option[Foo] = None,
customComplex: Option[Foo[Bar]] = None
)
"""

val companion = q"""
object Types {
implicit val encodeTypes = {
val readOnlyKeys = Set[String]()
Encoder.forProduct13("array", "obj", "bool", "string", "date", "date_time", "long", "int", "float", "double", "number", "integer", "untyped")( (o: Types) =>
(o.array, o.obj, o.bool, o.string, o.date, o.dateTime, o.long, o.int, o.float, o.double, o.number, o.integer, o.untyped)
).mapJsonObject(_.filterKeys(key => !(readOnlyKeys contains key)))
Encoder.forProduct15("array", "obj", "bool", "string", "date", "date_time", "long", "int", "float", "double", "number", "integer", "untyped", "custom", "customComplex")((o: Types) => (o.array, o.obj, o.bool, o.string, o.date, o.date_time, o.long, o.int, o.float, o.double, o.number, o.integer, o.untyped, o.custom, o.customComplex)).mapJsonObject(_.filterKeys(key => !(readOnlyKeys contains key)))
}
implicit val decodeTypes = Decoder.forProduct13("array", "obj", "bool", "string", "date", "date_time", "long", "int", "float", "double", "number", "integer", "untyped")(Types.apply _)
implicit val decodeTypes = Decoder.forProduct15("array", "obj", "bool", "string", "date", "date_time", "long", "int", "float", "double", "number", "integer", "untyped", "custom", "customComplex")(Types.apply _)
}
"""

Expand Down

0 comments on commit 0c679dd

Please sign in to comment.