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

[eklavya]

No description provided.

[dottybot]

Hello, and thank you for opening this PR! 🎉

All contributors have signed the CLA, thank you! ❤️

Commit Messages

We want to keep history, but for that to actually be useful we have
some rules on how to format our commit messages (relevant xkcd).

Please stick to these guidelines for commit messages:

1. Separate subject from body with a blank line
2. When fixing an issue, start your commit message with Fix #<ISSUE-NBR>:
3. Limit the subject line to 72 characters
4. Capitalize the subject line
5. Do not end the subject line with a period
6. Use the imperative mood in the subject line ("Add" instead of "Added")
7. Wrap the body at 80 characters
8. Use the body to explain what and why vs. how

adapted from https://chris.beams.io/posts/git-commit

Have an awesome day! ☀️

[eklavya]

I will fix the tests if this fix is ok.

[Blaisorblade]

I think one must keep the current output in examples like

scala> final val a = "foo"
val a: String("foo") = "foo"

If there aren't tests enforcing this behavior, this is a good time to add them, but I hope those are the failing tests.

[allanrenucci]

Closing. Will be fixed by #4537. Thanks @eklavya!

[allanrenucci]

My mistake #4537 is unrelated. @eklavya do you plan to keep working on this?

[eklavya]

@allanrenucci I am not able to differentiate between x: a.type being singleton and final val x being singleton.

[Blaisorblade]

TL;DR
Talking with @allanrenucci we agreed that for constants like "foo" or 1 (in code, ConstantType) we want to use the constant as a singleton type (no .type added, so that they're legal syntax):

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

but for other singleton types (SingletonType) that refer to terms we need to follow the request in #3683:

scala> val c: a.type = a 
val c: a.type = "foo"

ConstantType extends SingletonType, so this requires adding a new branch.

(Also wondering what happens on the other subclasses of SingletonType, but I hope adding .type works there too).

Details

@allanrenucci convinced me to reconsider. We can replace String("a") with "a", since those are SIP-23 literal types, which are all primitives, hence it's reasonable to assume users can find their types from the literals. Also, writing "a" means we output legal syntax. However, "a".type is not valid syntax, so we shouldn't add .type there.

I am not able to differentiate between x: a.type being singleton and final val x being singleton.

Sorry for being unclear, you don't need to find whether final was used; I just wrote final val a = "foo" as a shortcut for writing val a: "foo" = "foo".

[eklavya]

Adding ConstantType case breaks defs :(

In the test "methodDoesNotTakeMorePrameters" -
expected:<[((a: Int): Unit)(Scope.foo)]> but was:<[Scope.foo.type]>

[allanrenucci]

@eklavya Can you push your current solution to see all the test cases that break

[eklavya]

@allanrenucci pushed, I am messing around with the types of SingletonType to figure out a way to make it work without changing existing behaviour on defs, which I can't seem to find.

[allanrenucci]

@eklavya I pushed a commit that fixes the error message test failures. For the other test failures you should update the tests

[allanrenucci]

I believe the changes should go into RefinedPrinter instead

[allanrenucci]

Should be:

case ConstantType(value) =>
 toText(value)

[allanrenucci]

Why special case TermRef?

[eklavya]

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 😛

[Blaisorblade]

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?

[eklavya]

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

[OlivierBlanvillain]

@eklavya The PR is getting really old... If you plan to work again on it please reopen or make a new one, I will close it for now.