Skip to content

Commit

Permalink
Enable fatal warnings and fix them all across codebase (#457)
Browse files Browse the repository at this point in the history
- Delete `@deprecated` members
- Increase stack size for jvm to mitigate compiler stack overflow
- Introduce `IterableOnce` type alias to `TraversableOnce` in Scala 2.12
to fix deprecation in Scala 2.13 while maintaining Scala 2.12
compatibility
- Add `@nowarn("cat=deprecation")` in Scala 2 macro code
  • Loading branch information
lolgab committed Mar 11, 2023
1 parent 1e3061c commit 0724f89
Show file tree
Hide file tree
Showing 29 changed files with 90 additions and 85 deletions.
1 change: 1 addition & 0 deletions .mill-jvm-opts
@@ -0,0 +1 @@
-Xss10m
6 changes: 4 additions & 2 deletions bench/src-jvm/Main.scala
Expand Up @@ -2,6 +2,7 @@ package upickle

import upack.MsgPackWriter

import scala.annotation.nowarn

object Main{
import ADTs.ADT0
Expand Down Expand Up @@ -53,6 +54,7 @@ object Main{
println()
}
}
@nowarn("cat=deprecation")
def benchParsingRendering(duration: Int, bytes: Boolean, strings: Boolean, streams: Boolean, msgpack: Boolean) = {
import java.nio.file.{Files, Paths}
import collection.JavaConverters._
Expand Down Expand Up @@ -126,7 +128,7 @@ object Main{

def circeJsonAst(duration: Int) = {
Common.bench0[String, io.circe.Json](duration, Common.benchmarkSampleJson)(
io.circe.parser.parse(_).right.get,
io.circe.parser.parse(_).getOrElse(???),
_.toString()
)
}
Expand All @@ -139,7 +141,7 @@ object Main{

def argonautJsonAst(duration: Int) = {
Common.bench0[String, argonaut.Json](duration, Common.benchmarkSampleJson)(
argonaut.Parse.parse(_).right.get,
argonaut.Parse.parse(_).getOrElse(???),
_.toString()
)
}
Expand Down
4 changes: 2 additions & 2 deletions bench/src/NonNative.scala
Expand Up @@ -32,7 +32,7 @@ object NonNative{
implicit def _w9: Encoder[ADT0] = deriveEncoder

Common.bench[String](duration)(
decode[Seq[Data]](_).right.get,
decode[Seq[Data]](_).getOrElse(???),
implicitly[Encoder[Seq[Data]]].apply(_).toString()
)

Expand Down Expand Up @@ -92,7 +92,7 @@ object NonNative{
implicit lazy val _w9: Encoder[ADT0] = deriveEncoder

Common.bench[String](duration)(
decode[Seq[Data]](_).right.get,
decode[Seq[Data]](_).getOrElse(???),
implicitly[Encoder[Seq[Data]]].apply(_).toString()
)
}
Expand Down
35 changes: 21 additions & 14 deletions build.sc
Expand Up @@ -32,12 +32,7 @@ val scalaJVMVersions = scala2JVMVersions ++ Seq(scala3) ++ dottyCustomVersion
val scalaJSVersions = scalaJVMVersions.map((_, scalaJS))
val scalaNativeVersions = scalaJVMVersions.map((_, scalaNative))

trait CommonModule extends ScalaModule {
override def scalacOptions = T{
super.scalacOptions() ++ {
if (scalaVersion() == scala212) Seq("-opt:l:method") else Nil
}
}
trait CommonBaseModule extends ScalaModule {
def platformSegment: String

override def sources = T.sources{
Expand All @@ -54,6 +49,19 @@ trait CommonModule extends ScalaModule {
}
}

trait CommonModule extends CommonBaseModule {
override def scalacOptions = T{
super.scalacOptions() ++ {
if (scalaVersion() == scala212) Seq("-opt:l:method") else Nil
} ++ Seq(
"-unchecked",
"-deprecation",
"-encoding", "utf8",
"-feature",
"-Xfatal-warnings"
)
}
}

trait CommonPublishModule extends CommonModule with PublishModule with Mima with CrossScalaModule {

Expand Down Expand Up @@ -100,7 +108,7 @@ trait CommonPublishModule extends CommonModule with PublishModule with Mima with
}
}

trait CommonTestModule extends CommonModule with TestModule.Utest{
trait CommonTestModule extends CommonBaseModule with TestModule.Utest{
override def ivyDeps = Agg(ivy"com.lihaoyi::utest::0.8.1") ++ (
if (isScala3(scalaVersion())) Agg.empty[mill.scalalib.Dep]
else Agg(ivy"com.lihaoyi:::acyclic:$acyclic")
Expand All @@ -111,6 +119,11 @@ trait CommonTestModule extends CommonModule with TestModule.Utest{
os.makeDir.all(javadocDir)
mill.modules.Jvm.createJar(Agg(javadocDir))(outDir)
}
override def scalacOptions = super.scalacOptions() ++
(if (isScala3(scalaVersion())) Seq(
"-Ximplicit-search-limit",
"200000"
) else Seq.empty[String])
}

trait CommonJvmModule extends CommonPublishModule{
Expand Down Expand Up @@ -373,12 +386,6 @@ trait UpickleModule extends CommonPublishModule{
ivy"org.scala-lang:scala-compiler:${scalaVersion()}"
)
else Agg.empty[Dep]
override def scalacOptions = super.scalacOptions() ++ Seq(
"-unchecked",
"-deprecation",
"-encoding", "utf8",
"-feature",
)
}


Expand All @@ -387,7 +394,7 @@ object upickle extends Module{
class JvmModule(val crossScalaVersion: String) extends UpickleModule with CommonJvmModule{
override def moduleDeps = Seq(ujson.jvm(), upack.jvm(), implicits.jvm())

object test extends Tests with CommonModule{
object test extends Tests with CommonBaseModule{
override def moduleDeps = {
(super.moduleDeps :+ core.jvm().test) ++ (
if (isDotty) Nil else Seq(
Expand Down
4 changes: 4 additions & 0 deletions core/src-2.12/upickle/core/compat/package.scala
Expand Up @@ -27,6 +27,10 @@ package object compat {
*/
type Factory[-A, +C] = CanBuildFrom[Nothing, A, C]

type IterableOnce[T] = TraversableOnce[T]

def toIterator[T](iterable: IterableOnce[T]): IterableOnce[T] = iterable

implicit class FactoryOps[-A, +C](private val factory: Factory[A, C]) {

/**
Expand Down
2 changes: 2 additions & 0 deletions core/src-2.13+/upickle/core/compat/package.scala
Expand Up @@ -4,4 +4,6 @@ package object compat {

type Factory[-A, +C] = collection.Factory[A, C]

def toIterator[T](iterable: IterableOnce[T]) = iterable.iterator

}
4 changes: 2 additions & 2 deletions core/src/upickle/core/LinkedHashMap.scala
Expand Up @@ -46,9 +46,9 @@ object LinkedHashMap {
def apply[K, V](): LinkedHashMap[K, V] =
new LinkedHashMap[K, V](new ju.LinkedHashMap[K, V])

def apply[K, V](items: TraversableOnce[(K, V)]): LinkedHashMap[K, V] = {
def apply[K, V](items: IterableOnce[(K, V)]): LinkedHashMap[K, V] = {
val map = LinkedHashMap[K, V]()
for ((key, value) <- items) {
toIterator(items).foreach { case (key, value) =>
map._put(key, value)
}
map
Expand Down
6 changes: 3 additions & 3 deletions core/src/upickle/core/TraceVisitor.scala
Expand Up @@ -3,7 +3,7 @@ package upickle.core
import upickle.core.TraceVisitor._

import scala.annotation.tailrec
import scala.util.control.NoStackTrace
import scala.util.control.{NonFatal, NoStackTrace}

/**
* Adds a JSON Path to exceptions thrown by the delegate Visitor.
Expand All @@ -19,7 +19,7 @@ object TraceVisitor {
else{
val wrapper = new upickle.core.TraceVisitor.Wrapper[T, J]()
try f(wrapper.visitor(v))
catch{case e => throw new upickle.core.TraceVisitor.TraceException(wrapper.lastHasPath.toString, e) }
catch{case NonFatal(e) => throw new upickle.core.TraceVisitor.TraceException(wrapper.lastHasPath.toString, e) }
}
}

Expand Down Expand Up @@ -122,7 +122,7 @@ class TraceVisitor[T, J](
objVisitor.visitEnd(index)
}

override def pathComponent: Option[String] = Option(key).map("'" + _.replaceAllLiterally("'", "\\'") + "'")
override def pathComponent: Option[String] = Option(key).map("'" + _.replace("'", "\\'") + "'")

override def parent: Option[HasPath] = Some(TraceVisitor.this.parentPath)
}
Expand Down
1 change: 1 addition & 0 deletions implicits/src-2/upickle/implicits/MacroImplicits.scala
@@ -1,6 +1,7 @@
package upickle.implicits

import language.experimental.macros
import scala.language.higherKinds

/**
* Stupid hacks to work around scalac not forwarding macro type params properly
Expand Down
3 changes: 2 additions & 1 deletion implicits/src-2/upickle/implicits/internal/Macros.scala
@@ -1,6 +1,6 @@
package upickle.implicits.internal

import scala.annotation.StaticAnnotation
import scala.annotation.{nowarn, StaticAnnotation}
import scala.language.experimental.macros
import compat._
import acyclic.file
Expand All @@ -15,6 +15,7 @@ import language.existentials
* directly, since they are called implicitly when trying to read/write
* types you don't have a Reader/Writer in scope for.
*/
@nowarn("cat=deprecation")
object Macros {

trait DeriveDefaults[M[_]] {
Expand Down
4 changes: 2 additions & 2 deletions implicits/src-3/upickle/implicits/macros.scala
Expand Up @@ -218,7 +218,7 @@ def defineEnumVisitorsImpl[T0, T <: Tuple](prefix: Expr[Any], macroX: String)(us

def handleType(tpe: TypeRepr, name: String, skipTrait: Boolean): Option[(ValDef, Symbol)] = {

val AppliedType(typePrefix, List(arg)) = tpe
val AppliedType(typePrefix, List(arg)) = tpe: @unchecked

if (skipTrait && arg.typeSymbol.flags.is(Flags.Trait)) None
else {
Expand All @@ -231,7 +231,7 @@ def defineEnumVisitorsImpl[T0, T <: Tuple](prefix: Expr[Any], macroX: String)(us
)

val macroCall = TypeApply(
Select(prefix.asTerm, prefix.asTerm.tpe.typeSymbol.memberMethod(macroX).head),
Select(prefix.asTerm, prefix.asTerm.tpe.typeSymbol.methodMember(macroX).head),
List(TypeTree.of(using arg.asType))
)

Expand Down
5 changes: 3 additions & 2 deletions implicits/src/upickle/implicits/Readers.scala
Expand Up @@ -8,6 +8,7 @@ import upickle.core._
import upickle.core.compat._
import scala.collection.mutable
import scala.concurrent.duration.{Duration, FiniteDuration}
import scala.language.higherKinds
import scala.reflect.ClassTag

trait Readers extends upickle.core.Types
Expand Down Expand Up @@ -57,8 +58,8 @@ trait Readers extends upickle.core.Types
override def expectedMsg = "expected number"
override def visitString(s: CharSequence, index: Int) = visitFloat64String(s.toString, index)
override def visitInt32(d: Int, index: Int) = d
override def visitInt64(d: Long, index: Int) = d
override def visitUInt64(d: Long, index: Int) = d
override def visitInt64(d: Long, index: Int) = d.toDouble
override def visitUInt64(d: Long, index: Int) = d.toDouble
override def visitFloat32(d: Float, index: Int) = d
override def visitFloat64(d: Double, index: Int) = d
override def visitFloat64StringParts(s: CharSequence, decIndex: Int, expIndex: Int, index: Int) = {
Expand Down
1 change: 1 addition & 0 deletions implicits/src/upickle/implicits/Writers.scala
Expand Up @@ -5,6 +5,7 @@ import java.util.UUID
import upickle.core.{ Visitor, Annotator }

import scala.concurrent.duration.{Duration, FiniteDuration}
import scala.language.higherKinds

trait Writers extends upickle.core.Types
with TupleReadWriters
Expand Down
2 changes: 2 additions & 0 deletions ujson/play/src/ujson/play/PlayJson.scala
Expand Up @@ -9,6 +9,8 @@ import scala.collection.mutable.ArrayBuffer
object PlayJson extends ujson.AstTransformer[JsValue] {
def transform[T](j: JsValue, f: Visitor[_, T]): T = j match{
case JsArray(xs) => transformArray(f, xs)
case JsFalse => f.visitFalse(-1)
case JsTrue => f.visitTrue(-1)
case JsBoolean(b) => if (b) f.visitTrue(-1) else f.visitFalse(-1)
case JsNull => f.visitNull(-1)
case JsNumber(d) => f.visitFloat64String(d.toString, -1)
Expand Down
2 changes: 1 addition & 1 deletion ujson/src-js/ujson/WebJson.scala
Expand Up @@ -12,7 +12,7 @@ object WebJson extends ujson.Transformer[js.Any]{
case true => f.visitTrue(-1)
case false => f.visitFalse(-1)
case null => f.visitNull(-1)
case s: js.Array[js.Any] =>
case s: js.Array[js.Any] @unchecked =>
val ctx = f.visitArray(-1, -1).narrow
for(i <- s) ctx.visitValue(transform(i, ctx.subVisitor), -1)
ctx.visitEnd(-1)
Expand Down
4 changes: 3 additions & 1 deletion ujson/src/ujson/AstTransformer.scala
Expand Up @@ -3,6 +3,8 @@ import upickle.core._

import upickle.core.compat._

import scala.language.higherKinds

trait AstTransformer[I] extends Transformer[I] with JsVisitor[I, I]{
def apply(t: Readable): I = t.transform(this)

Expand Down Expand Up @@ -32,7 +34,7 @@ trait AstTransformer[I] extends Transformer[I] with JsVisitor[I, I]{

def visitValue(v: I, index: Int): Unit = vs += (key -> v)

def visitEnd(index: Int) = build(vs.result)
def visitEnd(index: Int) = build(vs.result())
}
class AstArrVisitor[T[_]](build: T[I] => I)
(implicit factory: Factory[I, T[I]]) extends ArrVisitor[I, I]{
Expand Down
4 changes: 2 additions & 2 deletions ujson/src/ujson/JsVisitor.scala
Expand Up @@ -30,11 +30,11 @@ trait JsVisitor[-T, +J] extends Visitor[T, J]{
def visitInt32(i: Int, index: Int): J = visitFloat64(i, index)
def visitInt64(i: Long, index: Int): J = {
if (math.abs(i) > math.pow(2, 53) || i == -9223372036854775808L) visitString(i.toString, index)
else visitFloat64(i, index)
else visitFloat64(i.toDouble, index)
}
def visitUInt64(i: Long, index: Int): J = {
if (i > math.pow(2, 53) || i < 0) visitString(java.lang.Long.toUnsignedString(i), index)
else visitFloat64(i, index)
else visitFloat64(i.toDouble, index)
}

def visitFloat64String(s: String, index: Int): J = {
Expand Down
3 changes: 3 additions & 0 deletions ujson/src/ujson/Readable.scala
Expand Up @@ -3,6 +3,9 @@ package ujson
import java.nio.ByteBuffer
import java.nio.channels.ReadableByteChannel
import upickle.core.{Visitor, ObjArrVisitor}

import scala.language.implicitConversions

trait Readable {
def transform[T](f: Visitor[_, T]): T
}
Expand Down
43 changes: 9 additions & 34 deletions ujson/src/ujson/Value.scala
Expand Up @@ -3,6 +3,7 @@ package ujson
import upickle.core.{LinkedHashMap, ObjArrVisitor, ParseUtils, Visitor}
import upickle.core.compat._

import scala.language.implicitConversions
import scala.collection.mutable

sealed trait Value extends Readable with geny.Writable{
Expand Down Expand Up @@ -139,36 +140,10 @@ object Value extends AstTransformer[Value]{
}
}

@deprecated("use ujson.Str")
val Str = ujson.Str
@deprecated("use ujson.Str")
type Str = ujson.Str
@deprecated("use ujson.Obj")
val Obj = ujson.Obj
@deprecated("use ujson.Obj")
type Obj = ujson.Obj
@deprecated("use ujson.Arr")
val Arr = ujson.Arr
@deprecated("use ujson.Arr")
type Arr = ujson.Arr
@deprecated("use ujson.Num")
val Num = ujson.Num
@deprecated("use ujson.Num")
type Num = ujson.Num
@deprecated("use ujson.Bool")
val Bool = ujson.Bool
@deprecated("use ujson.Bool")
type Bool = ujson.Bool
@deprecated("use ujson.True")
val True = ujson.True
@deprecated("use ujson.False")
val False = ujson.False
@deprecated("use ujson.Null")
val Null = ujson.Null
implicit def JsonableSeq[T](items: TraversableOnce[T])
(implicit f: T => Value): Arr = Arr.from(items.map(f))
implicit def JsonableDict[T](items: TraversableOnce[(String, T)])
(implicit f: T => Value): Obj = Obj.from(items.map(x => (x._1, f(x._2))))
implicit def JsonableSeq[T](items: IterableOnce[T])
(implicit f: T => Value): Arr = Arr.from(toIterator(items).map(f))
implicit def JsonableDict[T](items: IterableOnce[(String, T)])
(implicit f: T => Value): Obj = Obj.from(toIterator(items).map(x => (x._1, f(x._2))))
implicit def JsonableBoolean(i: Boolean): Bool = if (i) ujson.True else ujson.False
implicit def JsonableByte(i: Byte): Num = Num(i)
implicit def JsonableShort(i: Short): Num = Num(i)
Expand Down Expand Up @@ -206,7 +181,7 @@ object Value extends AstTransformer[Value]{
override def visitFloat64StringParts(s: CharSequence, decIndex: Int, expIndex: Int, index: Int) = {
ujson.Num(
if (decIndex != -1 || expIndex != -1) s.toString.toDouble
else ParseUtils.parseIntegralNum(s, decIndex, expIndex, index)
else ParseUtils.parseIntegralNum(s, decIndex, expIndex, index).toDouble
)
}

Expand All @@ -229,7 +204,7 @@ object Value extends AstTransformer[Value]{
case class Str(value: String) extends Value
case class Obj(value: LinkedHashMap[String, Value]) extends Value
object Obj{
implicit def from(items: TraversableOnce[(String, Value)]): Obj = {
implicit def from(items: IterableOnce[(String, Value)]): Obj = {
Obj(LinkedHashMap(items))
}

Expand All @@ -249,9 +224,9 @@ object Obj{
case class Arr(value: mutable.ArrayBuffer[Value]) extends Value

object Arr{
implicit def from[T](items: TraversableOnce[T])(implicit conv: T => Value): Arr = {
implicit def from[T](items: IterableOnce[T])(implicit conv: T => Value): Arr = {
val buf = new mutable.ArrayBuffer[Value]()
items.foreach{ item =>
toIterator(items).foreach{ item =>
buf += (conv(item): Value)
}
Arr(buf)
Expand Down

0 comments on commit 0724f89

Please sign in to comment.