Skip to content

Commit

Permalink
util/util-app: scala 3 migration
Browse files Browse the repository at this point in the history
**Problem**

Build util-app with Scala 3

**Solution**

# Change util-app project settings.
# Compile project + test with Scala 3 migration flags
# Remove flags
# Fix incompatibilities
# Run cross-building tests

**Result**

//Auto-rewrite fixes://
Scala 3 requires parentheses around the parameter of a lambda:
- ClassPath.scala
- Flaggable.scala

//Other fixes://
Scala3 requires explicit empty argument list to invoke method:
- ClassPath.scala
- AppTest.scala
- LoadServiceTest.scala

Manifest is no longer available in Scala 3, use ClassTag instead:
- Flags.scala
- GlobalFlag.scala

JavaConverters is deprecated, use CollectionConverters instead:
- CommandExecutor.scala
- RealCommandTest.scala:

JIRA Issues: CSL-11082, CSL-11091

Differential Revision: https://phabricator.twitter.biz/D714780
  • Loading branch information
Michael Sutton authored and jenkins committed Aug 9, 2021
1 parent 0955e5c commit 94f1fa3
Show file tree
Hide file tree
Showing 10 changed files with 35 additions and 16 deletions.
8 changes: 8 additions & 0 deletions CHANGELOG.rst
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,14 @@ Note that ``PHAB_ID=#`` and ``RB_ID=#`` correspond to associated messages in com
Unreleased
----------

New Features
~~~~~~~~~~~~
* util-app: Now builds with Scala 3.0! ``PHAB_ID=D714780``

Breaking API Changes
~~~~~~~~~~~~~~~~~~~~
* util-app: Flags and GlobalFlag now use ClassTag instead of Manifest. ``PHAB_ID=D714780``

21.7.0
------

Expand Down
15 changes: 12 additions & 3 deletions build.sbt
Original file line number Diff line number Diff line change
Expand Up @@ -259,13 +259,22 @@ lazy val utilApp = Project(
id = "util-app",
base = file("util-app")
).settings(
sharedSettings
sharedScala3EnabledSettings
).settings(
name := "util-app",
libraryDependencies ++= Seq(
"org.mockito" % "mockito-core" % mockitoVersion % "test",
"org.scalatestplus" %% "mockito-3-3" % "3.1.2.0" % "test"
)
) ++ {
if (scalaVersion.value.startsWith("2")) {
Seq(
"org.scalatestplus" %% "mockito-3-3" % "3.1.2.0" % "test"
)
} else {
Seq(
"org.scalatestplus" %% "mockito-3-4" % "3.2.9.0" % "test"
)
}
},
).dependsOn(utilAppLifecycle, utilCore, utilRegistry)

lazy val utilAppLifecycle = Project(
Expand Down
4 changes: 2 additions & 2 deletions util-app/src/main/scala/com/twitter/app/ClassPath.scala
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ private[app] sealed abstract class ClassPath[CpInfo <: ClassPath.Info] {
browseUri0(uri, loader, buf, seenUris)
seenUris += uri
}
buf.result
buf.result()
}

// Note: In JDK 9+ URLClassLoader is no longer the default ClassLoader.
Expand All @@ -71,7 +71,7 @@ private[app] sealed abstract class ClassPath[CpInfo <: ClassPath.Info] {
// TODO - add suppport for the ModulePath after dropping JDK 8 support.
private[this] def urlsFromClasspath(): Array[URL] = {
val classpath: String = System.getProperty("java.class.path")
classpath.split(File.pathSeparator).map { pathEntry: String =>
classpath.split(File.pathSeparator).map { (pathEntry: String) =>
Paths.get(pathEntry).toAbsolutePath().toUri().toURL
}
}
Expand Down
8 changes: 4 additions & 4 deletions util-app/src/main/scala/com/twitter/app/Flaggable.scala
Original file line number Diff line number Diff line change
Expand Up @@ -132,19 +132,19 @@ object Flaggable {

implicit val ofInt: Flaggable[Int] = mandatory(_.toInt)
implicit val ofJavaInteger: Flaggable[JInteger] =
mandatory { s: String => JInteger.valueOf(s.toInt) }
mandatory { (s: String) => JInteger.valueOf(s.toInt) }

implicit val ofLong: Flaggable[Long] = mandatory(_.toLong)
implicit val ofJavaLong: Flaggable[JLong] =
mandatory { s: String => JLong.valueOf(s.toLong) }
mandatory { (s: String) => JLong.valueOf(s.toLong) }

implicit val ofFloat: Flaggable[Float] = mandatory(_.toFloat)
implicit val ofJavaFloat: Flaggable[JFloat] =
mandatory { s: String => JFloat.valueOf(s.toFloat) }
mandatory { (s: String) => JFloat.valueOf(s.toFloat) }

implicit val ofDouble: Flaggable[Double] = mandatory(_.toDouble)
implicit val ofJavaDouble: Flaggable[JDouble] =
mandatory { s: String => JDouble.valueOf(s.toDouble) }
mandatory { (s: String) => JDouble.valueOf(s.toDouble) }

// Conversions for common non-primitive types and collections.
implicit val ofDuration: Flaggable[Duration] = mandatory(Duration.parse(_))
Expand Down
3 changes: 2 additions & 1 deletion util-app/src/main/scala/com/twitter/app/Flags.scala
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import scala.collection.immutable.TreeSet
import scala.collection.mutable
import scala.collection.mutable.ArrayBuffer
import scala.jdk.CollectionConverters._
import scala.reflect.ClassTag
import scala.util.control.NonFatal

/**
Expand Down Expand Up @@ -277,7 +278,7 @@ class Flags(argv0: String, includeGlobal: Boolean, failFastUntilParsed: Boolean)
* @param name The name of the flag.
* @param help The help string of the flag.
*/
def apply[T](name: String, help: String)(implicit _f: Flaggable[T], m: Manifest[T]): Flag[T] = {
def apply[T](name: String, help: String)(implicit _f: Flaggable[T], m: ClassTag[T]): Flag[T] = {
val f = new Flag[T](name, help, m.toString, failFastUntilParsed)
add(f)
f
Expand Down
3 changes: 2 additions & 1 deletion util-app/src/main/scala/com/twitter/app/GlobalFlag.scala
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package com.twitter.app

import java.lang.reflect.Modifier
import scala.util.control.NonFatal
import scala.reflect.ClassTag

/**
* Subclasses of GlobalFlag (that are defined in libraries) are "global" in the
Expand Down Expand Up @@ -85,7 +86,7 @@ abstract class GlobalFlag[T] private[app] (
*
* @param help documentation regarding usage of this [[Flag]].
*/
def this(help: String)(implicit _f: Flaggable[T], m: Manifest[T]) =
def this(help: String)(implicit _f: Flaggable[T], m: ClassTag[T]) =
this(Right(m.toString), help)

/**
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
package com.twitter.app.command

import java.io.File
import scala.collection.JavaConverters._
import scala.jdk.CollectionConverters._

/**
* CommandExecutor is a private trait used for testing so that the actual forking of the
Expand Down
2 changes: 1 addition & 1 deletion util-app/src/test/scala/com/twitter/app/AppTest.scala
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,7 @@ class AppTest extends AnyFunSuite {
assert(n2 == 0)
assert(!f.isDefined)

p.setDone
p.setDone()
assert(n2 == 1)
assert(f.isDefined)
}
Expand Down
4 changes: 2 additions & 2 deletions util-app/src/test/scala/com/twitter/app/LoadServiceTest.scala
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ class LoadServiceTest extends AnyFunSuite with MockitoSugar {
f.setReadable(false)

new LoadServiceClassPath().browseUri(f.toURI, loader, buf)
assert(buf.result.isEmpty)
assert(buf.result().isEmpty)
f.delete()
}
}
Expand All @@ -128,7 +128,7 @@ class LoadServiceTest extends AnyFunSuite with MockitoSugar {
subDir.setReadable(false)

new LoadServiceClassPath().browseUri(f.toURI, loader, buf)
assert(buf.result.isEmpty)
assert(buf.result().isEmpty)

subDir.delete()
f.delete()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import java.nio.file.attribute.PosixFilePermission
import java.nio.file.Files
import org.scalatest.funsuite.AnyFunSuite
import org.scalatest.matchers.should.Matchers
import scala.collection.JavaConverters._
import scala.jdk.CollectionConverters._

object RealCommandTest {
// Bazel provides the internal dependency as a JAR, and consequently,
Expand Down

0 comments on commit 94f1fa3

Please sign in to comment.