From ad8b22b9adbe1958f33b08cf133a9baac2c0aa9a Mon Sep 17 00:00:00 2001 From: Felix Bruckmeier Date: Wed, 13 Oct 2021 21:25:53 +0000 Subject: [PATCH] util/util-core: Fix conversions to units from Int in Scala3 This fixes lost source-compatibility for unit conversions from Int values in Scala3. As discussed in #295 I extracted/adapted this from #290 (previous discussion). Previously something like 1.toInt.second did compile in Scala2 (with default settings) but didn't in Scala3. I added forwarders for all units which use Long as base, as that seemed reasonable and safe. I didn't add an implicit conversion from Float to Double because floating-point types are dumb and I don't trust the conversion to be lossless in all cases. Signed-off-by: Yufan Gong Differential Revision: https://phabricator.twitter.biz/D759923 --- .../scala/com/twitter/conversions/DurationOps.scala | 12 ++++++++++++ .../com/twitter/conversions/StorageUnitOps.scala | 13 +++++++++++++ .../main/scala/com/twitter/conversions/U64Ops.scala | 13 +++++++++++++ .../com/twitter/conversions/DurationOpsTest.scala | 9 ++++++++- .../twitter/conversions/StorageUnitOpsTest.scala | 9 ++++++++- .../scala/com/twitter/conversions/U64OpsTest.scala | 6 ++++++ 6 files changed, 60 insertions(+), 2 deletions(-) diff --git a/util-core/src/main/scala/com/twitter/conversions/DurationOps.scala b/util-core/src/main/scala/com/twitter/conversions/DurationOps.scala index 8ca387897b..ea104ce058 100644 --- a/util-core/src/main/scala/com/twitter/conversions/DurationOps.scala +++ b/util-core/src/main/scala/com/twitter/conversions/DurationOps.scala @@ -2,6 +2,7 @@ package com.twitter.conversions import com.twitter.util.Duration import java.util.concurrent.TimeUnit +import scala.language.implicitConversions /** * Implicits for writing readable [[com.twitter.util.Duration]]s. @@ -37,4 +38,15 @@ object DurationOps { def day: Duration = days } + /** + * Forwarder for Int, as Scala 3.0 doesn't seem to do the implicit conversion to Long anymore. + * This is not a bug, as Scala 2.13 already had a flag ("-Ywarn-implicit-widening") to turn on warnings/errors + * for that. + * + * The implicit conversion from Int to Long here doesn't lose precision and keeps backwards source compatibliity + * with previous releases. + */ + implicit def richDurationFromInt(numNanos: Int): RichDuration = + new RichDuration(numNanos.toLong) + } diff --git a/util-core/src/main/scala/com/twitter/conversions/StorageUnitOps.scala b/util-core/src/main/scala/com/twitter/conversions/StorageUnitOps.scala index 28ee0ef9a5..c7eac3846f 100644 --- a/util-core/src/main/scala/com/twitter/conversions/StorageUnitOps.scala +++ b/util-core/src/main/scala/com/twitter/conversions/StorageUnitOps.scala @@ -2,6 +2,8 @@ package com.twitter.conversions import com.twitter.util.StorageUnit +import scala.language.implicitConversions + /** * Implicits for writing readable [[com.twitter.util.StorageUnit]]s. * @@ -34,4 +36,15 @@ object StorageUnitOps { def million: Long = numBytes * 1000 * 1000 def billion: Long = numBytes * 1000 * 1000 * 1000 } + + /** + * Forwarder for Int, as Scala 3.0 doesn't seem to do the implicit conversion to Long anymore. + * This is not a bug, as Scala 2.13 already had a flag ("-Ywarn-implicit-widening") to turn on warnings/errors + * for that. + * + * The implicit conversion from Int to Long here doesn't lose precision and keeps backwards source compatibliity + * with previous releases. + */ + implicit def richStorageUnitFromInt(numBytes: Int): RichStorageUnit = + new RichStorageUnit(numBytes.toLong) } diff --git a/util-core/src/main/scala/com/twitter/conversions/U64Ops.scala b/util-core/src/main/scala/com/twitter/conversions/U64Ops.scala index c4cee43411..75599ffe57 100644 --- a/util-core/src/main/scala/com/twitter/conversions/U64Ops.scala +++ b/util-core/src/main/scala/com/twitter/conversions/U64Ops.scala @@ -1,5 +1,7 @@ package com.twitter.conversions +import scala.language.implicitConversions + object U64Ops { /** @@ -19,4 +21,15 @@ object U64Ops { def toU64HexString: String = "%016x".format(self) } + /** + * Forwarder for Int, as Scala 3.0 doesn't seem to do the implicit conversion to Long anymore. + * This is not a bug, as Scala 2.13 already had a flag ("-Ywarn-implicit-widening") to turn on warnings/errors + * for that. + * + * The implicit conversion from Int to Long here doesn't lose precision and keeps backwards source compatibliity + * with previous releases. + */ + implicit def richU64LongFromInt(self: Int): RichU64Long = + new RichU64Long(self.toLong) + } diff --git a/util-core/src/test/scala/com/twitter/conversions/DurationOpsTest.scala b/util-core/src/test/scala/com/twitter/conversions/DurationOpsTest.scala index bc2143bc76..9e9538173d 100644 --- a/util-core/src/test/scala/com/twitter/conversions/DurationOpsTest.scala +++ b/util-core/src/test/scala/com/twitter/conversions/DurationOpsTest.scala @@ -3,8 +3,9 @@ package com.twitter.conversions import com.twitter.util.Duration import com.twitter.conversions.DurationOps._ import org.scalatest.funsuite.AnyFunSuite +import org.scalatestplus.scalacheck.ScalaCheckPropertyChecks -class DurationOpsTest extends AnyFunSuite { +class DurationOpsTest extends AnyFunSuite with ScalaCheckPropertyChecks { test("converts Duration.Zero") { assert(0.seconds eq Duration.Zero) assert(0.milliseconds eq Duration.Zero) @@ -17,4 +18,10 @@ class DurationOpsTest extends AnyFunSuite { assert(100L.nanoseconds == Duration.fromNanoseconds(100L)) } + test("conversion works for Int values") { + forAll { (intValue: Int) => + assert(intValue.seconds == intValue.toLong.seconds) + } + } + } diff --git a/util-core/src/test/scala/com/twitter/conversions/StorageUnitOpsTest.scala b/util-core/src/test/scala/com/twitter/conversions/StorageUnitOpsTest.scala index c46bfc7093..587b25cf73 100644 --- a/util-core/src/test/scala/com/twitter/conversions/StorageUnitOpsTest.scala +++ b/util-core/src/test/scala/com/twitter/conversions/StorageUnitOpsTest.scala @@ -3,8 +3,9 @@ package com.twitter.conversions import com.twitter.util.StorageUnit import com.twitter.conversions.StorageUnitOps._ import org.scalatest.funsuite.AnyFunSuite +import org.scalatestplus.scalacheck.ScalaCheckPropertyChecks -class StorageUnitOpsTest extends AnyFunSuite { +class StorageUnitOpsTest extends AnyFunSuite with ScalaCheckPropertyChecks { test("converts") { assert(StorageUnit.fromBytes(1) == 1.byte) @@ -26,4 +27,10 @@ class StorageUnitOpsTest extends AnyFunSuite { assert(StorageUnit.fromPetabytes(7) == 7.petabytes) } + test("conversion works for Int values") { + forAll { (intValue: Int) => + assert(intValue.bytes == intValue.toLong.bytes) + } + } + } diff --git a/util-core/src/test/scala/com/twitter/conversions/U64OpsTest.scala b/util-core/src/test/scala/com/twitter/conversions/U64OpsTest.scala index 942c41bb8c..f6c20fc168 100644 --- a/util-core/src/test/scala/com/twitter/conversions/U64OpsTest.scala +++ b/util-core/src/test/scala/com/twitter/conversions/U64OpsTest.scala @@ -16,4 +16,10 @@ class U64OpsTest extends AnyFunSuite with ScalaCheckDrivenPropertyChecks { assert(s.toU64Long == java.lang.Long.parseUnsignedLong(s, 16)) } } + + test("conversion works for Int values") { + forAll { (intValue: Int) => + assert(intValue.toU64HexString == intValue.toLong.toU64HexString) + } + } }