Merge pull request #430 from jvican/cleanup-previous-fixes

Address fixes pointed out in #419

core/src/main/scala/scala/pickling/PicklingErrors.scala

@@ -101,8 +101,9 @@ object PicklingErrors {
   /** Exception thrown when there is no type hint and the type in the
     * unpickler hasn't been elided.
     */
-  case object NoTypeHint extends ParsingException(
-    "Type is elided in pickle, but no elide hint was provided by the unpickler.")
+  case class NoTypeHint(suffix: String) extends ParsingException(
+    s"Type is elided in pickle, but no elide hint was provided by the unpickler$suffix")
+  object NoTypeHint extends NoTypeHint(".")
 
   /** Represent any error that violates an assumption made by scala pickling. */
   class LogicException(msg: String) extends BasePicklingException(msg)
@@ -165,8 +166,19 @@ object PicklingErrors {
     * @param extra Extra information that enriches the feedback
     */
   final case class UnrecognizedClass(clz: Class[_], extra: Option[String])
-    extends PicklingRuntimeException(s"Class $clz not recognized by pickler" +
-      s"""${if(extra.isDefined) ", " + extra + "." else "." }""" )
+    extends PicklingRuntimeException(s"Class ${clz.getName} not recognized" +
+      s""" by pickler. ${if(extra.isDefined) ", " + extra + "." else "." }"""
+    )
+
+  /** Exception thrown when a pickler is unable to recognize a tag.
+    *
+    * @param tagKey The string representation of a tag
+    * @param context Information about where or how has happened
+    */
+  final case class UnrecognizedTag(tagKey: String, context: String)
+    extends PicklingRuntimeException(
+      s"Error when $context. Unexpected tag $tagKey could not be recognized."
+    )
 
   /** Used to add some top message to a captured exception, that usually
     * gives some information on the context in which it happened.

core/src/main/scala/scala/pickling/generator/sourcegen.scala

@@ -11,6 +11,7 @@ private[pickling] trait SourceGenerator extends Macro with tags.FastTypeTagMacro
   val errorsPath = q"$picklingPath.PicklingErrors"
   val basePicklingException = q"$errorsPath.BasePicklingException"
   val basePicklingExceptionType = tq"$errorsPath.BasePicklingException"
+  val unrecognizedTagPath = q"$errorsPath.UnrecognizedTag"
 
   def pickleNull(builder: c.Tree): c.Tree =
     q"""_root_.scala.pickling.Defaults.nullPickler.pickle(null, $builder)"""
@@ -325,8 +326,11 @@ private[pickling] trait SourceGenerator extends Macro with tags.FastTypeTagMacro
   def genSubclassUnpickler(x: SubclassUnpicklerDelegation): c.Tree = {
     val tpe = x.parent.tpe[c.universe.type](c.universe)
     val defaultCase =
-      if(x.lookupRuntime) CaseDef(Ident(nme.WILDCARD), EmptyTree, q"_root_.scala.pickling.internal.`package`.currentRuntime.picklers.genUnpickler(_root_.scala.pickling.internal.`package`.currentMirror, tagKey)")
-      else CaseDef(Ident(nme.WILDCARD), EmptyTree, q"""throw $basePicklingException("Cannot unpickle, Unexpected tag: " + tagKey + " not recognized.")""")
+      if(x.lookupRuntime)
+        CaseDef(Ident(nme.WILDCARD), EmptyTree,
+          q"_root_.scala.pickling.internal.`package`.currentRuntime.picklers.genUnpickler(_root_.scala.pickling.internal.`package`.currentMirror, tagKey)")
+      else CaseDef(Ident(nme.WILDCARD), EmptyTree,
+        q"""throw $unrecognizedTagPath(tagKey, "unpickling")""")
     val subClassCases =
        x.subClasses.toList map { sc =>
          val stpe = sc.tpe[c.universe.type](c.universe)

core/src/main/scala/scala/pickling/json/JSONPickleFormat.scala

@@ -8,16 +8,18 @@ package object json extends JsonFormats
 
 package json {
 
-  import scala.pickling.PicklingErrors.{FieldNotFound, LogicPicklingError, JsonParseFailed, BasePicklingException}
+  import scala.pickling.PicklingErrors._
   import scala.reflect.runtime.universe._
   import definitions._
+
   import scala.util.parsing.json._
-  import scala.collection.mutable.{StringBuilder, Stack}
+  import scala.collection.mutable.{Stack => MutableStack}
 
   trait JsonFormats {
     implicit val pickleFormat: JSONPickleFormat = new JSONPickleFormat
     implicit def toJSONPickle(value: String): JSONPickle = JSONPickle(value)
-    implicit def jsonPickleToUnpickleOps(value: String): UnpickleOps = new UnpickleOps(JSONPickle(value))   
+    implicit def jsonPickleToUnpickleOps(value: String): UnpickleOps =
+      new UnpickleOps(JSONPickle(value))
   }
 
   case class JSONPickle(value: String) extends Pickle {
@@ -40,8 +42,8 @@ package json {
     }
   }
 
-  class JSONPickleBuilder(format: JSONPickleFormat, buf: Output[String]) extends PBuilder with PickleTools {
-    // private val buf = new StringBuilder()
+  class JSONPickleBuilder(format: JSONPickleFormat, buf: Output[String])
+    extends PBuilder with PickleTools {
     private var nindent = 0
     private def indent() = nindent += 1
     private def unindent() = nindent -= 1
@@ -64,7 +66,7 @@ package json {
       append(s + "\n")
       pendingIndent = true
     }
-    private val tags = new Stack[FastTypeTag[_]]()
+    private val tags = new MutableStack[FastTypeTag[_]]()
     private def pickleArray(arr: Array[_], tag: FastTypeTag[_]) = {
       unindent()
       appendLine("[")
@@ -226,10 +228,14 @@ package json {
           }
         } else {
           datum match {
-            case JSONObject(fields) if fields.contains("$ref") => FastTypeTag.Ref.key
-            case JSONObject(fields) if fields.contains("$type") => fields("$type").asInstanceOf[String]
-            case JSONObject(fields) => throw LogicPicklingError(s"Could not find a type tag, and no elided type was hinted: ${fields}")
-            case value => throw LogicPicklingError(s"Logic pickling error:  Could not find a type tag on primitive, and no elided type was hinted: $value")
+            case JSONObject(fields)
+              if fields.contains("$ref") => FastTypeTag.Ref.key
+            case JSONObject(fields)
+              if fields.contains("$type") => fields("$type").asInstanceOf[String]
+            case JSONObject(fields) =>
+              throw NoTypeHint(s". Found fields: ${fields}")
+            case value =>
+              throw NoTypeHint(s" for the primitive $value.")
           }
         }
       }

core/src/main/scala/scala/pickling/pickler/Any.scala

@@ -1,7 +1,6 @@
 package scala.pickling
 package pickler
 
-import scala.pickling.internal.GRL
 import scala.pickling.internal.currentRuntime
 import scala.reflect.runtime.currentMirror
 
@@ -30,9 +29,7 @@ object AnyPicklerUnpickler extends AbstractPicklerUnpickler[Any]
     val pickler = if (picklee == null) nullPickler else {
       val clazz = picklee.getClass
       val classLoader = this.getClass.getClassLoader
-      GRL.lock()
-      val tag = try FastTypeTag.makeRaw(clazz)
-      finally GRL.unlock()
+      val tag = FastTypeTag.makeRaw(clazz)
       val p = currentRuntime.picklers.genPickler(classLoader, clazz, tag)
       p.asInstanceOf[Pickler[Any]]
     }

core/src/test/scala/scala/pickling/generation/SealedTraitStaticAnnotatedTest.scala

@@ -46,11 +46,11 @@ class SealedTraitStaticAnnotatedTest extends FunSuite {
     val apple = Apple("Fuji")
     try {
       val pickled = (apple: Fruit).pickle.value
-      throw new Exception(s"We should have failed to pickle Apple but we pickled as: $pickled")
+      fail(s"We should have failed to pickle Apple but we pickled as: $pickled")
     } catch {
       case PicklingException(message, cause) =>
         if (!message.contains("Apple not recognized"))
-          throw new Exception(s"Not the expected exception: $message")
+          fail (s"Not the expected exception: $message")
     }
 
     // if we are only using static (un)picklers, then the Banana
@@ -63,23 +63,23 @@ class SealedTraitStaticAnnotatedTest extends FunSuite {
     // it should fail because it doesn't know to duck-type into Banana
     try {
       val f = JSONPickle(bananaString.replace("Banana", "Cucumber")).unpickle[Fruit]
-      throw new Exception(s"Should have thrown on unpickle but instead parsed $f")
+      fail(s"Should have thrown on unpickle but instead parsed $f")
     } catch {
       case PicklingException(message, cause) =>
-        if (!message.contains("Cucumber not recognized"))
-          throw new Exception(s"Not the expected exception: $message")
+        if (!message.contains("Cucumber could not be recognized"))
+          fail(s"Not the expected exception: $message")
     }
 
     // due to the annotation, the Fruit unpickler should not know
     // a thing about Apple either, even though it's a subtype of
     // Fruit.
     try {
       val f = JSONPickle(bananaString.replace("Banana", "Apple")).unpickle[Fruit]
-      throw new Exception(s"Should have thrown on unpickle but instead parsed $f")
+      fail(s"Should have thrown on unpickle but instead parsed $f")
     } catch {
       case PicklingException(message, cause) =>
-        if (!message.contains("Apple not recognized"))
-          throw new Exception(s"Not the expected exception: $message")
+        if (!message.contains("Apple could not be recognized"))
+          fail(s"Not the expected exception: $message")
     }
   }
 

core/src/test/scala/scala/pickling/generation/SealedTraitStaticTest.scala

@@ -42,7 +42,7 @@ class SealedTraitStaticTest extends FunSuite {
       throw new Exception(s"Should have thrown on unpickle but instead parsed $f")
     } catch {
       case PicklingException(message, cause) =>
-        if (!message.contains("Cucumber not recognized"))
+        if (!message.contains("Cucumber could not be recognized"))
           throw new Exception(s"Not the expected exception: $message")
     }