[scaladoc] Changed TreeFactory so that it doesn...

[scaladoc] Changed TreeFactory so that it doesn't consume exceptions.
Required fixes to makeAnnotation and makeTree which were throwing
exceptions as a matter of fact. Also code refactoring around annotations
and trees. Closes #4284. No review.

2 files changed, 40 insertions, 48 deletions

diff --git a/src/compiler/scala/tools/nsc/doc/model/ModelFactory.scala b/src/compiler/scala/tools/nsc/doc/model/ModelFactory.scala
index 7583201e75..f8924a6d87 100644
--- a/src/compiler/scala/tools/nsc/doc/model/ModelFactory.scala
+++ b/src/compiler/scala/tools/nsc/doc/model/ModelFactory.scala
@@ -380,30 +380,30 @@ class ModelFactory(val global: Global, val settings: doc.Settings) { thisFactory
   def makeAnnotation(annot: AnnotationInfo): Annotation = {
     val aSym = annot.atp.typeSymbol
     new EntityImpl(aSym, makeTemplate(aSym.owner)) with Annotation {
-      def annotationClass =
+      lazy val annotationClass =
         makeTemplate(annot.atp.typeSymbol)
-      def arguments =
-        annotationClass match {
+      val arguments = { // lazy
+        def noParams = annot.args map { _ => None }
+        val params: List[Option[ValueParam]] = annotationClass match {
           case aClass: Class =>
-            aClass.valueParams match {
-              case Nil => Nil
-              case vp :: vps =>
-                (vp zip annot.args) map { case (param, arg) =>
-                  new ValueArgument {
-                    def parameter = Some(param)
-                    def value = makeTree(arg)
-                  }
-                }
-            }
-          case _ =>
-            annot.args map { arg =>
-              new ValueArgument {
-                def parameter = None
-                def value = makeTree(arg)
-              }
+            (aClass.primaryConstructor map { _.valueParams.head }) match {
+              case Some(vps) => vps map { Some(_) }
+              case None => noParams
             }
+          case _ => noParams
         }
-
+        assert(params.length == annot.args.length)
+        (params zip annot.args) flatMap { case (param, arg) =>
+          makeTree(arg) match {
+            case Some(tree) =>
+              Some(new ValueArgument {
+                def parameter = param
+                def value = tree
+              })
+            case None => None
+          }
+        }
+      }
     }
   }
 
@@ -494,7 +494,7 @@ class ModelFactory(val global: Global, val settings: doc.Settings) { thisFactory
           (currentRun.units filter (_.source.file == aSym.sourceFile)).toList match {
             case List(unit) =>
               (unit.body find (_.symbol == aSym)) match {
-                case Some(ValDef(_,_,_,rhs)) => Some(makeTree(rhs))
+                case Some(ValDef(_,_,_,rhs)) => makeTree(rhs)
                 case _ => None
               }
             case _ => None
diff --git a/src/compiler/scala/tools/nsc/doc/model/TreeFactory.scala b/src/compiler/scala/tools/nsc/doc/model/TreeFactory.scala
index 5ed0aa740e..dd519feed0 100755
--- a/src/compiler/scala/tools/nsc/doc/model/TreeFactory.scala
+++ b/src/compiler/scala/tools/nsc/doc/model/TreeFactory.scala
@@ -3,6 +3,7 @@ package doc
 package model
 
 import scala.collection._
+import util.{RangePosition, SourceFile}
 
 /** The goal of this trait is , using makeTree,
   * to browse a tree to
@@ -18,22 +19,21 @@ trait TreeFactory { thisTreeFactory: ModelFactory with TreeFactory =>
   val global: Global
   import global._
 
-  def makeTree(rhs: Tree): TreeEntity = {
+  def makeTree(rhs: Tree): Option[TreeEntity] = {
 
-    var expr: String = null
+    var expr = new StringBuilder
     var refs = new immutable.TreeMap[Int, (Entity, Int)] // start, (Entity to be linked to , end)
 
-    try {
+    val pos: Position = rhs.pos
 
-      val firstIndex = rhs.pos.startOrPoint
+    if (pos.isInstanceOf[RangePosition]) {
 
-      /** Gets the full string of the right hand side of a parameter, without links */
-      for (i <- firstIndex until rhs.pos.endOrPoint)
-        expr += rhs.pos.source.content.apply(i)
-      rhs match {
-        case Block(r,s) => expr += "}"
-        case _ =>
-      }
+      val source: SourceFile = pos.source
+      val firstIndex = pos.startOrPoint
+      val lastIndex = pos.endOrPoint
+
+      assert(firstIndex < lastIndex, "Invalid position indices for tree " + rhs + " (" + firstIndex + ", " + lastIndex + ")")
+      expr.appendAll(source.content, firstIndex, lastIndex - firstIndex)
 
       val traverser = new Traverser {
 
@@ -41,8 +41,8 @@ trait TreeFactory { thisTreeFactory: ModelFactory with TreeFactory =>
           * stores it in tree.refs with its position
           */
         def makeLink(rhs: Tree){
-          var start = rhs.pos.point - firstIndex
-          val end = rhs.pos.endOrPoint - firstIndex
+          var start = pos.startOrPoint - firstIndex
+          val end = pos.endOrPoint - firstIndex
           if(start != end) {
             var asym = rhs.symbol
             if (asym.isClass) makeTemplate(asym) match{
@@ -85,24 +85,16 @@ trait TreeFactory { thisTreeFactory: ModelFactory with TreeFactory =>
 
       traverser.traverse(rhs)
 
-      assert(expr != null, "No expression constructed for rhs=" + rhs.toString)
-      new TreeEntity {
-        val expression = expr
+      Some(new TreeEntity {
+        val expression = expr.toString
         val refEntity = refs
-      }
+      })
 
     }
 
-    catch {
-      case e: Throwable =>
-        //println("Bad tree: " + rhs)
-       // TODO: This exception should be rethrown and no dummy tree entity should be created.
-        new TreeEntity {
-          val expression = "?"
-          val refEntity = new immutable.TreeMap[Int, (Entity, Int)]
-        }
-        //throw e
-    }
+     // If there is no position for the tree it means it has been obtained through unpickling and cannot be
+     // printed as a tree.
+    else None
 
   }