Skip to content

Commit

Permalink
util/util-core: Fix conversions to units from Int in Scala3
Browse files Browse the repository at this point in the history
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 <[email protected]>

Differential Revision: https://phabricator.twitter.biz/D759923
  • Loading branch information
felixbr authored and jenkins committed Oct 13, 2021
1 parent e550817 commit ad8b22b
Show file tree
Hide file tree
Showing 6 changed files with 60 additions and 2 deletions.
12 changes: 12 additions & 0 deletions util-core/src/main/scala/com/twitter/conversions/DurationOps.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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)

}
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*
Expand Down Expand Up @@ -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)
}
13 changes: 13 additions & 0 deletions util-core/src/main/scala/com/twitter/conversions/U64Ops.scala
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
package com.twitter.conversions

import scala.language.implicitConversions

object U64Ops {

/**
Expand All @@ -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)

}
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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)
}
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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)
}
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}
}

0 comments on commit ad8b22b

Please sign in to comment.