Merge branch 'master' into issue225

MIGRATING.md

@@ -1,3 +1,28 @@
+Migrating to 0.49.0
+===================
+
+0.49.0 may contain breaking changes for all clients and servers
+---------------------------------------------------------------
+
+The case converters (the code that takes identifiers and turns them into camelCase, snake_case, etc.) has been rewritten to fix bugs and inconsistencies. As a result, some identifiers (parameter, property, class, etc. names) may be slightly different from what they were before and will need to be updated.
+
+This is especially true in Scala code, where invalid identifiers were simply backtick-escaped.  For example, the following specification:
+
+```
+parameters:
+  - name: X-Foo-Bar
+    in: header
+    type: string
+```
+
+which defines a header method parameter, would have generated a method signature such as:
+
+```
+def someMethod(`x-Foo-Bar`: Option[String])
+```
+
+With this change, the parameter name will be the more idiomatic (and backtick-free) `xFooBar`.
+
 Migrating to 0.42.0
 ===================
 

modules/codegen/src/main/scala/com/twilio/guardrail/generators/syntax/package.scala

@@ -1,5 +1,6 @@
 package com.twilio.guardrail.generators
 
+import cats.data.NonEmptyList
 import java.util.Locale
 
 package object syntax {
@@ -8,37 +9,51 @@ package object syntax {
     "Modifications will be overwritten; instead edit the OpenAPI/Swagger spec file."
   )
 
-  private val toPascalRegexes = List(
-    "[\\._-]([a-z0-9])".r, // dotted, snake, or dashed case
-    "\\s+([a-zA-Z])".r, // spaces
-    "^([a-z])".r // initial letter
+  /*
+   * The case converters work as follows.
+   *
+   * First we break up the given string into parts.  This is a several-stage process
+   * while we consider boundaries in precedence order.  First we split on
+   * dash/underscore/space/dot, as those are the "strongest" boundary delimiters.
+   * Then we split on non-uppercase -> uppercase boundaries.  We avoid splitting
+   * just on uppercase because if a part is ALLUPPERCASE then we want to consider it
+   * a single part.  After that, we try to keep runs of uppercase characters (when
+   * they are followed by non-uppercase characters) in the same group; that is,
+   * something like "FOOBar" should get broken up into ("foo", "bar").
+   *
+   * There are a few things we just can't accurately handle, like digits.  If you
+   * consider the source string "foo9Bar", there's no way to know if that should
+   * be grouped as ("foo9", "bar") or ("foo", "9", "bar").  And for "foo9bar", it's
+   * worse: we don't know if it should be  ("foo9bar"), ("foo", "9", "bar"),
+   * ("foo9", "bar"), or ("foo", "9bar").  In these cases we'll choose to assume
+   * that digits are not initial group characters.
+   */
+  private val SPLIT_DELIMITERS = "[-_\\s\\.]+".r
+  private val BOUNDARY_SPLITTERS = List(
+    "([^A-Z])([A-Z])".r,
+    "([A-Z]+)([A-Z][^A-Z]+)".r
   )
 
   implicit class RichString(val s: String) extends AnyVal {
-    def toPascalCase: String =
-      toPascalRegexes.foldLeft(s)(
-        (accum, regex) => regex.replaceAllIn(accum, m => m.group(1).toUpperCase(Locale.US))
-      )
-
-    def toCamelCase: String = {
-      val fromSnakeOrDashed =
-        "[-_ ]([a-z])".r.replaceAllIn(s, m => m.group(1).toUpperCase(Locale.US))
-      "^([A-Z])".r
-        .replaceAllIn(fromSnakeOrDashed, m => m.group(1).toLowerCase(Locale.US))
-    }
-
-    def toSnakeCase: String = {
-      val noPascal  = "^[A-Z]".r.replaceAllIn(s, _.group(0).toLowerCase(Locale.US))
-      val fromCamel = "[A-Z]".r.replaceAllIn(noPascal, "_" + _.group(0))
-      fromCamel.replaceAll("[- ]", "_")
-    }
-
-    def toDashedCase: String = {
-      val lowercased =
-        "^([A-Z])".r.replaceAllIn(s, m => m.group(1).toLowerCase(Locale.US))
-      "([A-Z])".r
-        .replaceAllIn(lowercased, m => '-' +: m.group(1).toLowerCase(Locale.US))
-        .replaceAllLiterally(" ", "-")
-    }
+    private def splitParts(s: String): List[String] =
+      BOUNDARY_SPLITTERS
+        .foldLeft(SPLIT_DELIMITERS.split(s))(
+          (last, splitter) => last.flatMap(part => splitter.replaceAllIn(part, m => m.group(1) + "-" + m.group(2)).split("-"))
+        )
+        .map(_.toLowerCase(Locale.US))
+        .toList
+
+    def toPascalCase: String = splitParts(s).map(_.capitalize).mkString
+
+    def toCamelCase: String =
+      NonEmptyList
+        .fromList(splitParts(s))
+        .fold("")(
+          parts => parts.head + parts.tail.map(_.capitalize).mkString
+        )
+
+    def toSnakeCase: String = splitParts(s).mkString("_")
+
+    def toDashedCase: String = splitParts(s).mkString("-")
   }
 }

modules/codegen/src/test/scala/tests/core/CaseConvertersTest.scala

@@ -0,0 +1,63 @@
+package tests.core
+
+import com.twilio.guardrail.generators.syntax.RichString
+import org.scalatest.{ FreeSpec, Matchers }
+
+object CaseConvertersTest {
+  private case class CaseTest(raw: String, expectedPascal: String, expectedCamel: String, expectedSnake: String, expectedDashed: String) {
+    override val toString: String = s"""(Test case for: "$raw")"""
+  }
+
+  private val TEST_CASES = List(
+    CaseTest("foo", "Foo", "foo", "foo", "foo"),
+    CaseTest("Foo", "Foo", "foo", "foo", "foo"),
+    CaseTest("FOO", "Foo", "foo", "foo", "foo"),
+    CaseTest("fooBar", "FooBar", "fooBar", "foo_bar", "foo-bar"),
+    CaseTest("fooBarBaz", "FooBarBaz", "fooBarBaz", "foo_bar_baz", "foo-bar-baz"),
+    CaseTest("foo-bar-baz", "FooBarBaz", "fooBarBaz", "foo_bar_baz", "foo-bar-baz"),
+    CaseTest("foo.bar.baz", "FooBarBaz", "fooBarBaz", "foo_bar_baz", "foo-bar-baz"),
+    CaseTest("FooBarBaz", "FooBarBaz", "fooBarBaz", "foo_bar_baz", "foo-bar-baz"),
+    CaseTest("Foo-Bar-Baz", "FooBarBaz", "fooBarBaz", "foo_bar_baz", "foo-bar-baz"),
+    CaseTest("Foo.Bar-Baz", "FooBarBaz", "fooBarBaz", "foo_bar_baz", "foo-bar-baz"),
+    CaseTest("foo-Bar-Baz", "FooBarBaz", "fooBarBaz", "foo_bar_baz", "foo-bar-baz"),
+    CaseTest("foo.Bar-Baz", "FooBarBaz", "fooBarBaz", "foo_bar_baz", "foo-bar-baz"),
+    CaseTest("Foo-Bar-BazQuux", "FooBarBazQuux", "fooBarBazQuux", "foo_bar_baz_quux", "foo-bar-baz-quux"),
+    CaseTest("Foo-Bar.BazQuux", "FooBarBazQuux", "fooBarBazQuux", "foo_bar_baz_quux", "foo-bar-baz-quux"),
+    CaseTest("foo9bar", "Foo9bar", "foo9bar", "foo9bar", "foo9bar"),
+    CaseTest("foo9Bar", "Foo9Bar", "foo9Bar", "foo9_bar", "foo9-bar"),
+    CaseTest("9fooBar", "9fooBar", "9fooBar", "9foo_bar", "9foo-bar"),
+    CaseTest("Foo-_Bar__ baz.", "FooBarBaz", "fooBarBaz", "foo_bar_baz", "foo-bar-baz"),
+    CaseTest("FOO BAR BAZ", "FooBarBaz", "fooBarBaz", "foo_bar_baz", "foo-bar-baz"),
+    CaseTest("FOO BAR bazQuux", "FooBarBazQuux", "fooBarBazQuux", "foo_bar_baz_quux", "foo-bar-baz-quux"),
+    CaseTest("FOOBarBaz", "FooBarBaz", "fooBarBaz", "foo_bar_baz", "foo-bar-baz"),
+    CaseTest("FooBARBaz", "FooBarBaz", "fooBarBaz", "foo_bar_baz", "foo-bar-baz")
+  )
+}
+
+class CaseConvertersTest extends FreeSpec with Matchers {
+  import CaseConvertersTest._
+
+  "Pascal case converter should work" in {
+    TEST_CASES.foreach({ testCase =>
+      withClue(testCase)(testCase.raw.toPascalCase shouldBe testCase.expectedPascal)
+    })
+  }
+
+  "Camel case converter should work" in {
+    TEST_CASES.foreach({ testCase =>
+      withClue(testCase)(testCase.raw.toCamelCase shouldBe testCase.expectedCamel)
+    })
+  }
+
+  "Snake case converter should work" in {
+    TEST_CASES.foreach({ testCase =>
+      withClue(testCase)(testCase.raw.toSnakeCase shouldBe testCase.expectedSnake)
+    })
+  }
+
+  "Dashed case converter should work" in {
+    TEST_CASES.foreach({ testCase =>
+      withClue(testCase)(testCase.raw.toDashedCase shouldBe testCase.expectedDashed)
+    })
+  }
+}

modules/sample-http4s/src/test/scala/generators/Http4s/RoundTrip/Http4sFormDataTest.scala

@@ -18,7 +18,7 @@ class Http4sFormDataTest extends FunSuite with Matchers with EitherValues {
         new FooResource[IO]()
           .routes(new FooHandler[IO] {
             def doFoo(respond: DoFooResponse.type)(status: sdefs.definitions.Status): IO[DoFooResponse] =
-              if (status == sdefs.definitions.Status.OK) {
+              if (status == sdefs.definitions.Status.Ok) {
                 IO.pure(respond.Ok)
               } else {
                 IO.pure(respond.NotAcceptable)
@@ -28,15 +28,15 @@ class Http4sFormDataTest extends FunSuite with Matchers with EitherValues {
           .orNotFound
       )
     )
-    fooClient.doFoo(cdefs.definitions.Status.OK).attempt.unsafeRunSync().right.value shouldBe cdefs.foo.DoFooResponse.Ok
+    fooClient.doFoo(cdefs.definitions.Status.Ok).attempt.unsafeRunSync().right.value shouldBe cdefs.foo.DoFooResponse.Ok
   }
 
   test("missing required form param") {
     val client = Client.fromHttpApp(
       new FooResource[IO]()
         .routes(new FooHandler[IO] {
           def doFoo(respond: DoFooResponse.type)(status: sdefs.definitions.Status): IO[DoFooResponse] =
-            if (status == sdefs.definitions.Status.OK) {
+            if (status == sdefs.definitions.Status.Ok) {
               IO.pure(respond.Ok)
             } else {
               IO.pure(respond.NotAcceptable)
@@ -56,7 +56,7 @@ class Http4sFormDataTest extends FunSuite with Matchers with EitherValues {
           .routes(new FooHandler[IO] {
             def doFoo(respond: DoFooResponse.type)(status: sdefs.definitions.Status): IO[DoFooResponse] = ???
             def doBar(respond: DoBarResponse.type)(status: Option[sdefs.definitions.Status]): IO[DoBarResponse] =
-              if (status.contains(sdefs.definitions.Status.OK)) {
+              if (status.contains(sdefs.definitions.Status.Ok)) {
                 IO.pure(respond.Ok)
               } else {
                 IO.pure(respond.NotAcceptable)
@@ -65,7 +65,7 @@ class Http4sFormDataTest extends FunSuite with Matchers with EitherValues {
           .orNotFound
       )
     )
-    fooClient.doBar(Some(cdefs.definitions.Status.OK)).attempt.unsafeRunSync().right.value shouldBe cdefs.foo.DoBarResponse.Ok
+    fooClient.doBar(Some(cdefs.definitions.Status.Ok)).attempt.unsafeRunSync().right.value shouldBe cdefs.foo.DoBarResponse.Ok
   }
 
   test("missing optional form param") {