SI-7109 SI-7153 Generalize the API to get docComments: allow to force docTrees

for given fragments. Don't type-check when forcing doc comments, but rather
 do it directly. Test the new functionality as well as better tests for
the old one.

11 files changed, 171 insertions, 145 deletions

diff --git a/src/compiler/scala/tools/nsc/ast/DocComments.scala b/src/compiler/scala/tools/nsc/ast/DocComments.scala
index e635c5e87d..6e39fc9aa1 100755
--- a/src/compiler/scala/tools/nsc/ast/DocComments.scala
+++ b/src/compiler/scala/tools/nsc/ast/DocComments.scala
@@ -56,6 +56,11 @@ trait DocComments { self: Global =>
     else sym.owner.ancestors map (sym overriddenSymbol _) filter (_ != NoSymbol)
   }
 
+  def fillDocComment(sym: Symbol, comment: DocComment) {
+    docComments(sym) = comment
+    comment.defineVariables(sym)
+  }
+
   /** The raw doc comment of symbol `sym`, minus usecase and define sections, augmented by
    *  missing sections of an inherited doc comment.
    *  If a symbol does not have a doc comment but some overridden version of it does,
diff --git a/src/compiler/scala/tools/nsc/interactive/CompilerControl.scala b/src/compiler/scala/tools/nsc/interactive/CompilerControl.scala
index 73738ebd21..af82957a2e 100644
--- a/src/compiler/scala/tools/nsc/interactive/CompilerControl.scala
+++ b/src/compiler/scala/tools/nsc/interactive/CompilerControl.scala
@@ -164,17 +164,22 @@ trait CompilerControl { self: Global =>
 
   /** Sets sync var `response` to doc comment information for a given symbol.
    *
-   *  @param   sym      The symbol whose doc comment should be retrieved (might come from a classfile)
-   *  @param   site     The place where sym is observed.
-   *  @param   source   The source file that's supposed to contain the definition
-   *  @param   response A response that will be set to the following:
-   *                    If `source` contains a definition of a given symbol that has a doc comment,
-   *                    the (expanded, raw, position) triplet for a comment, otherwise ("", "", NoPosition).
-   *  Note: This operation does not automatically load `source`. If `source`
-   *  is unloaded, it stays that way.
+   *  @param   sym        The symbol whose doc comment should be retrieved (might come from a classfile)
+   *  @param   source     The source file that's supposed to contain the definition
+   *  @param   site       The symbol where 'sym' is observed
+   *  @param   fragments  All symbols that can contribute to the generated documentation
+   *                      together with their source files.
+   *  @param   response   A response that will be set to the following:
+   *                      If `source` contains a definition of a given symbol that has a doc comment,
+   *                      the (expanded, raw, position) triplet for a comment, otherwise ("", "", NoPosition).
+   *  Note: This operation does not automatically load sources that are not yet loaded.
    */
-  def askDocComment(sym: Symbol, site: Symbol, source: SourceFile, response: Response[(String, String, Position)]) =
-    postWorkItem(new AskDocCommentItem(sym, site, source, response))
+  def askDocComment(sym: Symbol, source: SourceFile, site: Symbol, fragments: List[(Symbol,SourceFile)], response: Response[(String, String, Position)]): Unit =
+    postWorkItem(new AskDocCommentItem(sym, source, site, fragments, response))
+
+  @deprecated("Use method that accepts fragments", "2.10.2")
+  def askDocComment(sym: Symbol, site: Symbol, source: SourceFile, response: Response[(String, String, Position)]): Unit =
+    askDocComment(sym, source, site, (sym,source)::Nil, response)
 
   /** Sets sync var `response` to list of members that are visible
    *  as members of the tree enclosing `pos`, possibly reachable by an implicit.
@@ -390,9 +395,9 @@ trait CompilerControl { self: Global =>
       response raise new MissingResponse
   }
 
-  case class AskDocCommentItem(val sym: Symbol, val site: Symbol, val source: SourceFile, response: Response[(String, String, Position)]) extends WorkItem {
-    def apply() = self.getDocComment(sym, site, source, response)
-    override def toString = "doc comment "+sym+" in "+source
+  case class AskDocCommentItem(val sym: Symbol, val source: SourceFile, val site: Symbol, val fragments: List[(Symbol,SourceFile)], response: Response[(String, String, Position)]) extends WorkItem {
+    def apply() = self.getDocComment(sym, source, site, fragments, response)
+    override def toString = "doc comment "+sym+" in "+source+" with fragments:"+fragments.mkString("(", ",", ")")
 
     def raiseMissing() =
       response raise new MissingResponse
diff --git a/src/compiler/scala/tools/nsc/interactive/Global.scala b/src/compiler/scala/tools/nsc/interactive/Global.scala
index 4c2c3e35f8..1f2245abb5 100644
--- a/src/compiler/scala/tools/nsc/interactive/Global.scala
+++ b/src/compiler/scala/tools/nsc/interactive/Global.scala
@@ -704,8 +704,8 @@ class Global(settings: Settings, _reporter: Reporter, projectName: String = "")
    *  If we do just removeUnit, some problems with default parameters can ensue.
    *  Calls to this method could probably be replaced by removeUnit once default parameters are handled more robustly.
    */
-  private def afterRunRemoveUnitOf(source: SourceFile) {
-    toBeRemovedAfterRun += source.file
+  private def afterRunRemoveUnitsOf(sources: List[SourceFile]) {
+    toBeRemovedAfterRun ++= sources map (_.file)
   }
 
   /** A fully attributed tree located at position `pos` */
@@ -713,7 +713,7 @@ class Global(settings: Settings, _reporter: Reporter, projectName: String = "")
     case None =>
       reloadSources(List(pos.source))
       try typedTreeAt(pos)
-      finally afterRunRemoveUnitOf(pos.source)
+      finally afterRunRemoveUnitsOf(List(pos.source))
     case Some(unit) =>
       informIDE("typedTreeAt " + pos)
       parseAndEnter(unit)
@@ -763,14 +763,23 @@ class Global(settings: Settings, _reporter: Reporter, projectName: String = "")
     respond(response)(typedTree(source, forceReload))
   }
 
+  private def withTempUnits[T](sources: List[SourceFile])(f: (SourceFile => RichCompilationUnit) => T): T = {
+    val unitOfSrc: SourceFile => RichCompilationUnit = src => unitOfFile(src.file)
+    sources filterNot (getUnit(_).isDefined) match {
+      case Nil =>
+        f(unitOfSrc)
+      case unknown =>
+        reloadSources(unknown)
+        try {
+          f(unitOfSrc)
+        } finally
+          afterRunRemoveUnitsOf(unknown)
+    }
+  }
+
   private def withTempUnit[T](source: SourceFile)(f: RichCompilationUnit => T): T =
-    getUnit(source) match {
-      case None =>
-        reloadSources(List(source))
-        try f(getUnit(source).get)
-        finally afterRunRemoveUnitOf(source)
-      case Some(unit) =>
-        f(unit)
+    withTempUnits(List(source)){ srcToUnit =>
+      f(srcToUnit(source))
     }
 
   /** Find a 'mirror' of symbol `sym` in unit `unit`. Pre: `unit is loaded. */
@@ -834,50 +843,36 @@ class Global(settings: Settings, _reporter: Reporter, projectName: String = "")
     }
   }
 
+  private def forceDocComment(sym: Symbol, unit: RichCompilationUnit) {
+    unit.body foreachPartial {
+      case DocDef(comment, defn) if defn.symbol == sym =>
+        fillDocComment(defn.symbol, comment)
+        EmptyTree
+      case _: ValOrDefDef =>
+        EmptyTree
+    }
+  }
+
   /** Implements CompilerControl.askDocComment */
-  private[interactive] def getDocComment(sym: Symbol, site: Symbol, source: SourceFile, response: Response[(String, String, Position)]) {
-    informIDE("getDocComment "+sym+" "+source)
+  private[interactive] def getDocComment(sym: Symbol, source: SourceFile, site: Symbol, fragments: List[(Symbol,SourceFile)],
+                                         response: Response[(String, String, Position)]) {
+    informIDE(s"getDocComment $sym at $source site $site")
     respond(response) {
-      withTempUnit(source){ u =>
-        val mirror = findMirrorSymbol(sym, u)
+      withTempUnits(fragments.toList.unzip._2){ units =>
+        for((sym, src) <- fragments) {
+          val mirror = findMirrorSymbol(sym, units(src))
+          if (mirror ne NoSymbol) forceDocComment(mirror, units(src))
+        }
+        val mirror = findMirrorSymbol(sym, units(source))
         if (mirror eq NoSymbol)
           ("", "", NoPosition)
         else {
-          forceDocComment(mirror, u)
-          (expandedDocComment(mirror), rawDocComment(mirror), docCommentPos(mirror))
+          (expandedDocComment(mirror, site), rawDocComment(mirror), docCommentPos(mirror))
         }
       }
     }
   }
 
-  private def forceDocComment(sym: Symbol, unit: RichCompilationUnit) {
-    // Either typer has been run and we don't find DocDef,
-    // or we force the targeted typecheck here.
-    // In both cases doc comment maps should be filled for the subject symbol.
-    val docTree =
-      unit.body find {
-        case DocDef(_, defn) if defn.symbol eq sym => true
-        case _ => false
-      }
-
-    for (t <- docTree) {
-      debugLog("Found DocDef tree for "+sym)
-      // Cannot get a typed tree at position since DocDef range is transparent.
-      val prevPos = unit.targetPos
-      val prevInterruptsEnabled = interruptsEnabled
-      try {
-        unit.targetPos = t.pos
-        interruptsEnabled = true
-        typeCheck(unit)
-      } catch {
-        case _: TyperResult => // ignore since we are after the side effect.
-      } finally {
-        unit.targetPos = prevPos
-        interruptsEnabled = prevInterruptsEnabled
-      }
-    }
-  }
-
   def stabilizedType(tree: Tree): Type = tree match {
     case Ident(_) if tree.symbol.isStable =>
       singleType(NoPrefix, tree.symbol)
diff --git a/src/compiler/scala/tools/nsc/interactive/Picklers.scala b/src/compiler/scala/tools/nsc/interactive/Picklers.scala
index 84cb03c140..2b389158c3 100644
--- a/src/compiler/scala/tools/nsc/interactive/Picklers.scala
+++ b/src/compiler/scala/tools/nsc/interactive/Picklers.scala
@@ -166,8 +166,8 @@ trait Picklers { self: Global =>
       .asClass (classOf[AskLinkPosItem])
 
   implicit def askDocCommentItem: CondPickler[AskDocCommentItem] =
-    (pkl[Symbol] ~ pkl[Symbol] ~ pkl[SourceFile])
-      .wrapped { case sym ~ site ~ source => new AskDocCommentItem(sym, site, source, new Response) } { item => item.sym ~ item.site ~ item.source }
+    (pkl[Symbol] ~ pkl[SourceFile] ~ pkl[Symbol] ~ pkl[List[(Symbol,SourceFile)]])
+      .wrapped { case sym ~ source ~ site ~ fragments => new AskDocCommentItem(sym, source, site, fragments, new Response) } { item => item.sym ~ item.source ~ item.site ~ item.fragments }
       .asClass (classOf[AskDocCommentItem])
 
   implicit def askLoadedTypedItem: CondPickler[AskLoadedTypedItem] =
diff --git a/src/compiler/scala/tools/nsc/typechecker/Typers.scala b/src/compiler/scala/tools/nsc/typechecker/Typers.scala
index f97582f45c..24857e4ab0 100644
--- a/src/compiler/scala/tools/nsc/typechecker/Typers.scala
+++ b/src/compiler/scala/tools/nsc/typechecker/Typers.scala
@@ -5289,8 +5289,7 @@ trait Typers extends Modes with Adaptations with Tags {
       def typedDocDef(docdef: DocDef) = {
         if (forScaladoc && (sym ne null) && (sym ne NoSymbol)) {
           val comment = docdef.comment
-          docComments(sym) = comment
-          comment.defineVariables(sym)
+          fillDocComment(sym, comment)
           val typer1 = newTyper(context.makeNewScope(tree, context.owner))
           for (useCase <- comment.useCases) {
             typer1.silent(_.typedUseCase(useCase)) match {
diff --git a/test/files/presentation/doc.check b/test/files/presentation/doc.check
index e33756773d..5a3ff13151 100644
--- a/test/files/presentation/doc.check
+++ b/test/files/presentation/doc.check
@@ -1,49 +1 @@
-reload: Test.scala
-body:Body(List(Paragraph(Chain(List(Summary(Chain(List(Text(This is a test comment), Text(.)))), Text(
-))))))
-@example:Body(List(Paragraph(Chain(List(Summary(Monospace(Text("abb".permutations = Iterator(abb, bab, bba)))))))))
-@version:
-@since:
-@todo:
-@note:
-@see:
-body:Body(List(Paragraph(Chain(List(Summary(Chain(List(Text(This is a test comment), Text(.)))), Text(
-))))))
-@example:Body(List(Paragraph(Chain(List(Summary(Monospace(Text("abb".permutations = Iterator(abb, bab, bba)))))))))
-@version:Body(List(Paragraph(Chain(List(Summary(Chain(List(Text(1), Text(.)))), Text(0, 09/07/2012))))))
-@since:
-@todo:
-@note:
-@see:
-body:Body(List(Paragraph(Chain(List(Summary(Chain(List(Text(This is a test comment), Text(.)))), Text(
-))))))
-@example:Body(List(Paragraph(Chain(List(Summary(Monospace(Text("abb".permutations = Iterator(abb, bab, bba)))))))))
-@version:Body(List(Paragraph(Chain(List(Summary(Chain(List(Text(1), Text(.)))), Text(0, 09/07/2012))))))
-@since:Body(List(Paragraph(Chain(List(Summary(Chain(List(Text(2), Text(.)))), Text(10))))))
-@todo:
-@note:
-@see:
-body:Body(List(Paragraph(Chain(List(Summary(Chain(List(Text(This is a test comment), Text(.)))), Text(
-))))))
-@example:Body(List(Paragraph(Chain(List(Summary(Monospace(Text("abb".permutations = Iterator(abb, bab, bba)))))))))
-@version:Body(List(Paragraph(Chain(List(Summary(Chain(List(Text(1), Text(.)))), Text(0, 09/07/2012))))))
-@since:Body(List(Paragraph(Chain(List(Summary(Chain(List(Text(2), Text(.)))), Text(10))))))
-@todo:Body(List(Paragraph(Chain(List(Summary(Text(this method is unsafe)))))))
-@note:
-@see:
-body:Body(List(Paragraph(Chain(List(Summary(Chain(List(Text(This is a test comment), Text(.)))), Text(
-))))))
-@example:Body(List(Paragraph(Chain(List(Summary(Monospace(Text("abb".permutations = Iterator(abb, bab, bba)))))))))
-@version:Body(List(Paragraph(Chain(List(Summary(Chain(List(Text(1), Text(.)))), Text(0, 09/07/2012))))))
-@since:Body(List(Paragraph(Chain(List(Summary(Chain(List(Text(2), Text(.)))), Text(10))))))
-@todo:Body(List(Paragraph(Chain(List(Summary(Text(this method is unsafe)))))))
-@note:Body(List(Paragraph(Chain(List(Summary(Text(Don't inherit!)))))))
-@see:
-body:Body(List(Paragraph(Chain(List(Summary(Chain(List(Text(This is a test comment), Text(.)))), Text(
-))))))
-@example:Body(List(Paragraph(Chain(List(Summary(Monospace(Text("abb".permutations = Iterator(abb, bab, bba)))))))))
-@version:Body(List(Paragraph(Chain(List(Summary(Chain(List(Text(1), Text(.)))), Text(0, 09/07/2012))))))
-@since:Body(List(Paragraph(Chain(List(Summary(Chain(List(Text(2), Text(.)))), Text(10))))))
-@todo:Body(List(Paragraph(Chain(List(Summary(Text(this method is unsafe)))))))
-@note:Body(List(Paragraph(Chain(List(Summary(Text(Don't inherit!)))))))
-@see:Body(List(Paragraph(Chain(List(Summary(Text(some other method)))))))
+reload: Base.scala, Class.scala, Derived.scala
diff --git a/test/files/presentation/doc/doc.scala b/test/files/presentation/doc/doc.scala
index 5855f488b8..371b825026 100755
--- a/test/files/presentation/doc/doc.scala
+++ b/test/files/presentation/doc/doc.scala
@@ -4,27 +4,40 @@ import scala.tools.nsc.doc.base.comment._
 import scala.tools.nsc.interactive._
 import scala.tools.nsc.interactive.tests._
 import scala.tools.nsc.util._
-import scala.tools.nsc.io._
 
 object Test extends InteractiveTest {
   val tags = Seq(
     "@example  `\"abb\".permutations = Iterator(abb, bab, bba)`",
     "@version 1.0, 09/07/2012",
     "@since 2.10",
-    "@todo this method is unsafe",
+    "@todo this is unsafe!",
     "@note Don't inherit!",
-    "@see some other method"
+    "@see something else"
   )
 
-  val comment = "This is a test comment."
-  val caret = "<caret>"
+  val names = Seq("Class", "Def", "Val", "Var", "AbstracType", "TypeAlias", "Trait", "InnerClass")
+  val bareText =
+    """abstract class %s {
+    |  def %s = ""
+    |  val %s = ""
+    |  var %s: String = _
+    |  type %s
+    |  type %s = String
+    |  class %s
+    |}
+    |trait %s""".stripMargin.format(names: _*)
+
+  def docComment(nTags: Int) = "/**\n%s*/".format(tags.take(nTags).mkString("\n"))
+
+  def text(name: String, nTags: Int) = {
+    val nameIndex = bareText.indexOf(name)
+    val (pre, post) = bareText.splitAt(nameIndex)
+    val crIndex = pre.lastIndexOf("\n")
+    val (prepre, prepost) = pre.splitAt(crIndex)
+    prepre + docComment(nTags) + prepost + post
+  }
+
 
-  def text(nTags: Int) = 
-    """|/** %s
-       |
-       | *  %s */
-       |trait Commented {}
-       |class User(c: %sCommented)""".stripMargin.format(comment, tags take nTags mkString "\n", caret)
 
   override lazy val compiler = {
     prepareSettings(settings)
@@ -38,9 +51,9 @@ object Test extends InteractiveTest {
 
       override def forScaladoc = true
 
-      def getComment(sym: Symbol, source: SourceFile) = {
+      def getComment(sym: Symbol, source: SourceFile, fragments: List[(Symbol,SourceFile)]): Option[Comment] = {
         val docResponse = new Response[(String, String, Position)]
-        askDocComment(sym, sym.owner, source, docResponse)
+        askDocComment(sym, source, sym.owner, fragments, docResponse)
         docResponse.get.left.toOption flatMap {
           case (expanded, raw, pos) =>
             if (expanded.isEmpty)
@@ -53,37 +66,74 @@ object Test extends InteractiveTest {
   }
 
   override def runDefaultTests() {
-    for (i <- 1 to tags.length) {
-      val markedText = text(i)
-      val idx = markedText.indexOf(caret)
-      val fileText = markedText.substring(0, idx) + markedText.substring(idx + caret.length)
-      val source = sourceFiles(0) 
-      val batch = new BatchSourceFile(source.file, fileText.toCharArray)
+    import compiler._
+    def findSource(name: String) = sourceFiles.find(_.file.name == name).get
+
+    val className = names.head
+    for (name <- names;
+         i <- 1 to tags.length) {
+      val newText = text(name, i)
+      val source = findSource("Class.scala")
+      val batch = new BatchSourceFile(source.file, newText.toCharArray)
       val reloadResponse = new Response[Unit]
       compiler.askReload(List(batch), reloadResponse)
       reloadResponse.get.left.toOption match {
         case None =>
-          reporter.println("Couldn't reload")
+          println("Couldn't reload")
         case Some(_) =>
-          val treeResponse = new compiler.Response[compiler.Tree]
-          val pos = compiler.rangePos(batch, idx, idx, idx)
-          compiler.askTypeAt(pos, treeResponse)
-          treeResponse.get.left.toOption match {
-            case Some(tree) =>
-              val sym = tree.tpe.typeSymbol
-              compiler.getComment(sym, batch) match {
-                case None => println("Got no doc comment")
+          val parseResponse = new Response[Tree]
+          askParsedEntered(batch, true, parseResponse)
+          parseResponse.get.left.toOption match {
+            case None =>
+              println("Couldn't parse")
+            case Some(_) =>
+              val sym = compiler.ask { () =>
+                val toplevel = definitions.EmptyPackage.info.decl(newTypeName(name))
+                if (toplevel eq NoSymbol) {
+                  val clazz = definitions.EmptyPackage.info.decl(newTypeName(className))
+
+                  val term = clazz.info.decl(newTermName(name))
+                  if (term eq NoSymbol) clazz.info.decl(newTypeName(name)) else
+                    if (term.isAccessor) term.accessed else term
+                } else toplevel
+              }
+
+              getComment(sym, batch, (sym,batch)::Nil) match {
+                case None => println(s"Got no doc comment for $name")
                 case Some(comment) =>
                   import comment._
-                  val tags: List[(String, Iterable[Body])] =
-                    List(("@example", example), ("@version", version), ("@since", since.toList), ("@todo", todo), ("@note",  note), ("@see", see))
-                  val str = ("body:" + body + "\n") + 
-                    tags.map{ case (name, bodies) => name + ":" + bodies.mkString("\n") }.mkString("\n")
-                  println(str)
+                  def cnt(bodies: Iterable[Body]) = bodies.size
+                  val actual = cnt(example) + cnt(version) + cnt(since) + cnt(todo) + cnt(note) + cnt(see)
+                  if (actual != i)
+                    println(s"Got docComment with $actual tags instead of $i, file text:\n$newText")
               }
-            case None => println("Couldn't find a typedTree")
           }
       }
     }
+
+    // Check inter-classes documentation one-time retrieved ok.
+    val baseSource = findSource("Base.scala")
+    val derivedSource = findSource("Derived.scala")
+    def existsText(where: Any, text: String): Boolean = where match {
+      case `text` => true
+      case s: Seq[_] => s exists (existsText(_, text))
+      case p: Product => p.productIterator exists (existsText(_, text))
+    }
+    val (derived, base) = compiler.ask { () => 
+      val derived = definitions.RootPackage.info.decl(newTermName("p")).info.decl(newTypeName("Derived"))
+      (derived, derived.ancestors(0))
+    }
+    val cmt1 = getComment(derived, derivedSource, (base, baseSource)::(derived, derivedSource)::Nil)
+    if (!existsText(cmt1, "Derived comment."))
+      println("Unexpected Derived class comment:"+cmt1)
+
+    val (fooDerived, fooBase) = compiler.ask { () =>
+      val decl = derived.tpe.decl(newTermName("foo"))
+      (decl, decl.allOverriddenSymbols(0))
+    }
+
+    val cmt2 = getComment(fooDerived, derivedSource, (fooBase, baseSource)::(fooDerived, derivedSource)::Nil)
+    if (!existsText(cmt2, "Base method has documentation."))
+      println("Unexpected foo method comment:"+cmt2)
   }
 }
diff --git a/test/files/presentation/doc/src/Class.scala b/test/files/presentation/doc/src/Class.scala
new file mode 100755
index 0000000000..a974bd6f5c
--- /dev/null
+++ b/test/files/presentation/doc/src/Class.scala
@@ -0,0 +1 @@
+object Class
\ No newline at end of file
diff --git a/test/files/presentation/doc/src/Test.scala b/test/files/presentation/doc/src/Test.scala
deleted file mode 100755
index fcc1554994..0000000000
--- a/test/files/presentation/doc/src/Test.scala
+++ /dev/null
@@ -1 +0,0 @@
-object Test
\ No newline at end of file
diff --git a/test/files/presentation/doc/src/p/Base.scala b/test/files/presentation/doc/src/p/Base.scala
new file mode 100755
index 0000000000..9031de3e3e
--- /dev/null
+++ b/test/files/presentation/doc/src/p/Base.scala
@@ -0,0 +1,11 @@
+package p
+
+/**
+ * @define BaseComment $BaseVar comment.
+ */
+trait Base {
+  /**
+   * Base method has documentation.
+   */
+  def foo: String
+}
diff --git a/test/files/presentation/doc/src/p/Derived.scala b/test/files/presentation/doc/src/p/Derived.scala
new file mode 100755
index 0000000000..1a9c9a26d1
--- /dev/null
+++ b/test/files/presentation/doc/src/p/Derived.scala
@@ -0,0 +1,9 @@
+package p
+
+/**
+ * $BaseComment
+ * @define BaseVar Derived
+ */
+class Derived extends Base {
+  def foo = ""
+}