Skip to content

Fixes #3683 REPL: Incorrect pretty-printing of singleton types #4132

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.

Sign up for GitHub

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

Closed
wants to merge 6 commits into from
Closed

Fixes #3683 REPL: Incorrect pretty-printing of singleton types #4132

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
13fd0d9
Merge branch 'master' of https://github.com/lampepfl/dotty
eklavya Feb 8, 2018
0d9c324
Merge branch 'master' of https://github.com/lampepfl/dotty
eklavya Mar 17, 2018
95e7c5a
Fixes #3683 REPL: Incorrect pretty-printing of singleton types
eklavya Mar 17, 2018
eae6dca
Merge branch 'master' of github.com:lampepfl/dotty
eklavya May 28, 2018
27780b4
attempt to case ConstantType
eklavya May 30, 2018
f51ac6f
Refactor "method does not take parameter" error message
allanrenucci May 30, 2018
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
4 changes: 4 additions & 0 deletions compiler/src/dotty/tools/dotc/printing/PlainPrinter.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,10 @@ class PlainPrinter(_ctx: Context) extends Printer {
ParamRefNameString(tp) ~ ".type"
case tp: TypeParamRef =>
ParamRefNameString(tp) ~ lambdaHash(tp.binder)
case tp: ConstantType =>
Copy link
Contributor

Choose a reason for hiding this comment

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

Should be:

case ConstantType(value) =>
 toText(value)

toTextRef(tp)
case tp: TermRef =>
toTextPrefix(tp.prefix) ~ selectionString(tp) ~ ".type"
Copy link
Contributor

Choose a reason for hiding this comment

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

Why special case TermRef?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is for a.type singleton types.

scala> final val a = "foo"
val a: "foo" = foo

scala> val x: a.type = a
val x: a.type = foo

a.type is made by special casing TermRef. I am sorry if that's nonsensical 😛

Copy link
Contributor

Choose a reason for hiding this comment

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

That makes some sense, but there are other cases above for tp: TermRef which already produce toTextRef(tp) ~ ".type" (which is equivalent to the code here), do they not apply?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I had missed that. No, those cases (in PlainPrinter) are not applied because of the guards.

case tp: SingletonType =>
toTextLocal(tp.underlying) ~ "(" ~ toTextRef(tp) ~ ")"
case AppliedType(tycon, args) =>
Expand Down
20 changes: 9 additions & 11 deletions compiler/src/dotty/tools/dotc/reporting/diagnostic/messages.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1371,27 +1371,25 @@ object messages {
|"""
}

case class MethodDoesNotTakeParameters(tree: tpd.Tree, methPartType: Types.Type)(err: typer.ErrorReporting.Errors)(implicit ctx: Context)
case class MethodDoesNotTakeParameters(tree: tpd.Tree, methodStr: String)(implicit ctx: Context)
extends Message(MethodDoesNotTakeParametersId) {
private val more = tree match {
case Apply(_, _) => " more"
case _ => ""
}

val msg = hl"${err.refStr(methPartType)} does not take$more parameters"
val msg = hl"$methodStr does not take$more parameters"

val kind = "Reference"

private val noParameters = if (methPartType.widenSingleton.isInstanceOf[ExprType])
hl"""|As ${err.refStr(methPartType)} is defined without parenthesis, you may
|not use any at call-site, either.
|"""
else
""
val explanation = {
val isNullary = tpd.methPart(tree).tpe.widenSingleton.isInstanceOf[ExprType]
val addendum =
if (isNullary) "\nNullary methods may not be called with parenthesis"
else ""

val explanation =
s"""|You have specified more parameter lists as defined in the method definition(s).
|$noParameters""".stripMargin
"You have specified more parameter lists as defined in the method definition(s)." + addendum
}

}

Expand Down
2 changes: 1 addition & 1 deletion compiler/src/dotty/tools/dotc/typer/Typer.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2138,7 +2138,7 @@ class Typer extends Namer
else
tree
case _ => tryInsertApplyOrImplicit(tree, pt, locked) {
errorTree(tree, MethodDoesNotTakeParameters(tree, methPart(tree).tpe)(err))
errorTree(tree, MethodDoesNotTakeParameters(tree, methodStr))
}
}

Expand Down
9 changes: 4 additions & 5 deletions compiler/test/dotty/tools/dotc/reporting/ErrorMessagesTests.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -347,10 +347,10 @@ class ErrorMessagesTests extends ErrorMessagesTest {
implicit val ctx: Context = ictx

assertMessageCount(1, messages)
val MethodDoesNotTakeParameters(tree, methodPart) :: Nil = messages
val MethodDoesNotTakeParameters(tree, methodStr) :: Nil = messages

assertEquals("Scope.foo", tree.show)
assertEquals("=> Unit(Scope.foo)", methodPart.show)
assertEquals("method foo in object Scope", methodStr)
}

@Test def methodDoesNotTakeMorePrameters =
Expand All @@ -365,11 +365,10 @@ class ErrorMessagesTests extends ErrorMessagesTest {
.expect { (ictx, messages) =>
implicit val ctx: Context = ictx

assertMessageCount(1, messages)
val MethodDoesNotTakeParameters(tree, methodPart) :: Nil = messages
val MethodDoesNotTakeParameters(tree, methodStr) :: Nil = messages

assertEquals("Scope.foo(1)", tree.show)
assertEquals("((a: Int): Unit)(Scope.foo)", methodPart.show)
assertEquals("method foo in object Scope", methodStr)
}

@Test def ambiugousOverloadWithWildcard =
Expand Down
5 changes: 5 additions & 0 deletions compiler/test/dotty/tools/repl/ReplCompilerTests.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,4 +125,9 @@ class ReplCompilerTests extends ReplTest {
compile("def f(g: => Int): Int = g")
assertTrue(storedOutput().startsWith("def f(g: => Int): Int"))
}

@Test def singletonType: Unit = fromInitialState { implicit state =>
compile("""val a = "foo"; val x: a.type = a""")
assertTrue(storedOutput().contains("""val x: a.type = "foo""""))
}
}