Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable fatal warnings and fix them all across codebase #457

Merged
merged 12 commits into from
Mar 11, 2023
1 change: 1 addition & 0 deletions .mill-jvm-opts
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
-Xss10m
6 changes: 4 additions & 2 deletions bench/src-jvm/Main.scala
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Loading