Skip to content

Add :doc command in REPL to show documentation #4669

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

Merged
merged 4 commits into from
Jul 9, 2018
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions compiler/src/dotty/tools/repl/ParseResult.scala
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,15 @@ object TypeOf {
val command = ":type"
}

/**
* A command that is used to display the documentation associated with
* the given expression.
*/
case class DocOf(expr: String) extends Command
object DocOf {
val command = ":doc"
}

/** `:imports` lists the imports that have been explicitly imported during the
* session
*/
Expand Down Expand Up @@ -89,6 +98,7 @@ case object Help extends Command {
|:load <path> interpret lines in a file
|:quit exit the interpreter
|:type <expression> evaluate the type of the given expression
|:doc <expression> print the documentation for the given expresssion
|:imports show import history
|:reset reset the repl to its initial state, forgetting all session entries
""".stripMargin
Expand Down Expand Up @@ -117,6 +127,7 @@ object ParseResult {
case Imports.command => Imports
case Load.command => Load(arg)
case TypeOf.command => TypeOf(arg)
case DocOf.command => DocOf(arg)
case _ => UnknownCommand(cmd)
}
case _ =>
Expand Down
52 changes: 52 additions & 0 deletions compiler/src/dotty/tools/repl/ReplCompiler.scala
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,15 @@ package dotty.tools.repl
import dotty.tools.backend.jvm.GenBCode
import dotty.tools.dotc.ast.Trees._
import dotty.tools.dotc.ast.{tpd, untpd}
import dotty.tools.dotc.core.Comments.CommentsContext
import dotty.tools.dotc.core.Contexts._
import dotty.tools.dotc.core.Decorators._
import dotty.tools.dotc.core.Flags._
import dotty.tools.dotc.core.Names._
import dotty.tools.dotc.core.Phases
import dotty.tools.dotc.core.Phases.Phase
import dotty.tools.dotc.core.StdNames._
import dotty.tools.dotc.core.Symbols._
import dotty.tools.dotc.reporting.diagnostic.messages
import dotty.tools.dotc.typer.{FrontEnd, ImportInfo}
import dotty.tools.dotc.util.Positions._
Expand Down Expand Up @@ -164,6 +166,56 @@ class ReplCompiler(val directory: AbstractFile) extends Compiler {
}
}

def docOf(expr: String)(implicit state: State): Result[String] = {
implicit val ctx: Context = state.context

/** Extract the (possibly) documented symbol from `tree` */
def extractSymbol(tree: tpd.Tree): Symbol = {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you explain why this is needed? Can you give an example

tree match {
case tpd.closureDef(defdef) => defdef.rhs.symbol
case _ => tree.symbol
}
}

/**
* Adapt `symbol` so that we get the one that holds the documentation.
* Return also the symbols that `symbol` overrides`.
*/
def pickSymbols(symbol: Symbol): Iterator[Symbol] = {
Copy link
Contributor

@allanrenucci allanrenucci Jun 29, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this needed in the REPL but not the IDE? In the IDE you do:

val symbol = Interactive.enclosingSourceSymbol(trees, pos)
val docComment = ctx.docCtx.flatMap(_.docstring(symbol))

val selectedSymbol = {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nitpick: unnecessary braces

if (symbol.is(Module, butNot = ModuleClass)) symbol.moduleClass
if (symbol.isConstructor) symbol.owner
else symbol
}

Iterator(selectedSymbol) ++ selectedSymbol.allOverriddenSymbols
}

typeCheck(expr).map {
case v @ ValDef(_, _, Block(stats, _)) if stats.nonEmpty =>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you maybe change typeOf to return a Result[Type] and use typeOf instead of typeCheck

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We're not interested in getting the type of the expression, but rather the typed AST so that we can extract the symbol that we're interested in from it:

scala> /** bar */ def foo: Int = 2
scala> :doc foo // prints: /** bar */

I want to get the symbol of foo so that I can retrieve its documentation; I don't care about its type here.

val stat = stats.last.asInstanceOf[tpd.Tree]
if (stat.tpe.isError) stat.tpe.show
else {
val doc =
ctx.docCtx.flatMap { docCtx =>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe in the REPL we should always have a docCtx

val symbols = pickSymbols(extractSymbol(stat))
symbols.collectFirst {
case sym if docCtx.docstrings.contains(sym) =>
docCtx.docstrings(sym).raw
}
}

doc.getOrElse(s"// No doc for `${expr}`")
}

case _ =>
"""Couldn't display the documentation for your expression, so sorry :(
|
|Please report this to my masters at github.com/lampepfl/dotty
""".stripMargin
}
}

final def typeCheck(expr: String, errorsAllowed: Boolean = false)(implicit state: State): Result[tpd.ValDef] = {

def wrapped(expr: String, sourceFile: SourceFile, state: State)(implicit ctx: Context): Result[untpd.PackageDef] = {
Expand Down
9 changes: 8 additions & 1 deletion compiler/src/dotty/tools/repl/ReplDriver.scala
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ class ReplDriver(settings: Array[String],

/** Create a fresh and initialized context with IDE mode enabled */
private[this] def initialCtx = {
val rootCtx = initCtx.fresh.addMode(Mode.ReadPositions).addMode(Mode.Interactive)
val rootCtx = initCtx.fresh.addMode(Mode.ReadPositions).addMode(Mode.Interactive).addMode(Mode.ReadComments)
val ictx = setup(settings, rootCtx)._2
ictx.base.initialize()(ictx)
ictx
Expand Down Expand Up @@ -338,6 +338,13 @@ class ReplDriver(settings: Array[String],
)
state

case DocOf(expr) =>
compiler.docOf(expr)(newRun(state)).fold(
displayErrors,
res => out.println(SyntaxHighlighting(res))
)
state

case Quit =>
// end of the world!
state
Expand Down
209 changes: 209 additions & 0 deletions compiler/test/dotty/tools/repl/DocTests.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,209 @@
package dotty.tools
package repl

import org.junit.Test
import org.junit.Assert.assertEquals

class DocTests extends ReplTest {
@Test def docOfDef =
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you can factor out all this duplication. This could look something like:

eval("/** doc */ def foo = 0").andThen {
  assertEquals("/** doc */", doc("foo"))
}

fromInitialState { implicit s => run("/** doc */ def foo = 0") }
.andThen { implicit s =>
storedOutput()
run(":doc foo")
assertEquals("/** doc */", storedOutput().trim)
}

@Test def docOfVal =
fromInitialState { implicit s => run("/** doc */ val foo = 0") }
.andThen { implicit s =>
storedOutput()
run(":doc foo")
assertEquals("/** doc */", storedOutput().trim)
}

@Test def docOfObject =
fromInitialState { implicit s => run("/** doc */ object Foo") }
.andThen { implicit s =>
storedOutput()
run(":doc Foo")
assertEquals("/** doc */", storedOutput().trim)
}

@Test def docOfClass =
fromInitialState { implicit s => run("/** doc */ class Foo") }
.andThen { implicit s =>
storedOutput()
run(":doc new Foo")
assertEquals("/** doc */", storedOutput().trim)
}

@Test def docOfTrait =
fromInitialState { implicit s => run("/** doc */ trait Foo") }
.andThen { implicit s =>
storedOutput()
run(":doc new Foo")
assertEquals("/** doc */", storedOutput().trim)
}

@Test def docOfDefInObject =
fromInitialState { implicit s => run("object O { /** doc */ def foo = 0 }") }
.andThen { implicit s =>
storedOutput()
run(":doc O.foo")
assertEquals("/** doc */", storedOutput().trim)
}

@Test def docOfValInObject =
fromInitialState { implicit s => run("object O { /** doc */ val foo = 0 }") }
.andThen { implicit s =>
storedOutput()
run(":doc O.foo")
assertEquals("/** doc */", storedOutput().trim)
}

@Test def docOfObjectInObject =
fromInitialState { implicit s => run("object O { /** doc */ object Foo }") }
.andThen { implicit s =>
storedOutput()
run(":doc O.Foo")
assertEquals("/** doc */", storedOutput().trim)
}

@Test def docOfClassInObject =
fromInitialState { implicit s => run("object O { /** doc */ class Foo }") }
.andThen { implicit s =>
storedOutput()
run(":doc new O.Foo")
assertEquals("/** doc */", storedOutput().trim)
}

@Test def docOfTraitInObject =
fromInitialState { implicit s => run("object O { /** doc */ trait Foo }") }
.andThen { implicit s =>
storedOutput()
run(":doc new O.Foo")
assertEquals("/** doc */", storedOutput().trim)
}

@Test def docOfDetInClass =
fromInitialState { implicit s => run("class C { /** doc */ def foo = 0 }") }
.andThen { implicit s => run("val c = new C") }
Copy link
Contributor

@allanrenucci allanrenucci Jun 29, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is no need for separate runs. You can have muti-line input:

val source =
  """class C { /** doc */ def foo = 0 }
    |
    |val c = new C""".stripMargin
run(source)

There is value in using multiple runs if you want to overwrite a definition. E.g:

scala> /** doc 1 */ class Foo
// defined class Foo

scala> :doc new Foo
/**  doc 1 */

scala> /** doc 2 */ class Foo
// defined class Foo

scala> :doc new Foo
/**  doc 2 */

.andThen { implicit s =>
storedOutput()
run(":doc c.foo")
assertEquals("/** doc */", storedOutput().trim)
}

@Test def docOfVatInClass =
fromInitialState { implicit s => run("class C { /** doc */ val foo = 0 }") }
.andThen { implicit s => run("val c = new C") }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same (use a single run)

.andThen { implicit s =>
storedOutput()
run(":doc c.foo")
assertEquals("/** doc */", storedOutput().trim)
}

@Test def docOfObjectInClass =
fromInitialState { implicit s => run("class C { /** doc */ object Foo }") }
.andThen { implicit s => run("val c = new C") }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same (use a single run)

.andThen { implicit s =>
storedOutput()
run(":doc c.Foo")
assertEquals("/** doc */", storedOutput().trim)
}

@Test def docOfClassInClass =
fromInitialState { implicit s => run("class C { /** doc */ class Foo }") }
.andThen { implicit s => run("val c = new C") }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same (use a single run)

.andThen { implicit s =>
storedOutput()
run(":doc new c.Foo")
assertEquals("/** doc */", storedOutput().trim)
}

@Test def docOfTraitInClass =
fromInitialState { implicit s => run("class C { /** doc */ trait Foo }") }
.andThen { implicit s => run("val c = new C") }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same (use a single run)

.andThen { implicit s =>
storedOutput()
run(":doc new c.Foo")
assertEquals("/** doc */", storedOutput().trim)
}

@Test def docOfOverloadedDef =
fromInitialState { implicit s =>
run("""object O {
|/** doc0 */ def foo(x: Int) = x
|/** doc1 */ def foo(x: String) = x
|}""".stripMargin)
}
.andThen { implicit s =>
storedOutput()
run(":doc O.foo(_: Int)")
assertEquals("/** doc0 */", storedOutput().trim)
s
}
.andThen { implicit s =>
run(":doc O.foo(_: String)")
assertEquals("/** doc1 */", storedOutput().trim)
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would merge this andThen block with the previous one


@Test def docOfInherited =
fromInitialState { implicit s => run("class C { /** doc */ def foo = 0 }") }
.andThen { implicit s => run("object O extends C") }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same (use a single run)

.andThen { implicit s =>
storedOutput()
run(":doc O.foo")
assertEquals("/** doc */", storedOutput().trim)
}

@Test def docOfOverride =
fromInitialState { implicit s =>
run("""abstract class A {
|/** doc0 */ def foo(x: Int): Int = x + 1
|/** doc1 */ def foo(x: String): String = x + "foo"
|}""".stripMargin)
}
.andThen { implicit s =>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same (use a single run)

run("""object O extends A {
| override def foo(x: Int): Int = x
| /** overridden doc */ override def foo(x: String): String = x
|}""".stripMargin)
}
.andThen { implicit s =>
storedOutput()
run(":doc O.foo(_: Int)")
assertEquals("/** doc0 */", storedOutput().trim)
s
}
.andThen { implicit s =>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same (merge andThen block)

run(":doc O.foo(_: String)")
assertEquals("/** overridden doc */", storedOutput().trim)
}

@Test def docOfOverrideObject =
fromInitialState { implicit s =>
run("""abstract class A {
| abstract class Companion { /** doc0 */ def bar: Int }
| /** companion */ def foo: Companion
|}""".stripMargin)
.andThen { implicit s =>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same (use a single run)

run("""object O extends A {
| override object foo extends Companion {
| override def bar: Int = 0
| }
|}""".stripMargin)
}
.andThen { implicit s =>
storedOutput()
run(":doc O.foo")
assertEquals("/** companion */", storedOutput().trim)
s
}
.andThen { implicit s =>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same (merge andThen block)

run(":doc O.foo.bar")
assertEquals("/** doc0 */", storedOutput().trim)
}
}

}