From 53c499bec28ccbc7695c79f56e40f98e7732aeb7 Mon Sep 17 00:00:00 2001 From: Eugene Vigdorchik Date: Tue, 19 Feb 2013 21:29:30 +0400 Subject: 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. --- src/compiler/scala/tools/nsc/ast/DocComments.scala | 5 + .../tools/nsc/interactive/CompilerControl.scala | 31 +++--- .../scala/tools/nsc/interactive/Global.scala | 83 +++++++-------- .../scala/tools/nsc/interactive/Picklers.scala | 4 +- .../scala/tools/nsc/typechecker/Typers.scala | 3 +- test/files/presentation/doc.check | 50 +-------- test/files/presentation/doc/doc.scala | 118 +++++++++++++++------ test/files/presentation/doc/src/Class.scala | 1 + test/files/presentation/doc/src/Test.scala | 1 - test/files/presentation/doc/src/p/Base.scala | 11 ++ test/files/presentation/doc/src/p/Derived.scala | 9 ++ 11 files changed, 171 insertions(+), 145 deletions(-) create mode 100755 test/files/presentation/doc/src/Class.scala delete mode 100755 test/files/presentation/doc/src/Test.scala create mode 100755 test/files/presentation/doc/src/p/Base.scala create mode 100755 test/files/presentation/doc/src/p/Derived.scala 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 = "" + 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 = "" +} -- cgit v1.2.3