Skip to content

Presentation compiler autocompletion bug with dependent types #10353

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

Closed
mpollmeier opened this issue Jun 3, 2017 · 7 comments
Closed

Presentation compiler autocompletion bug with dependent types #10353

mpollmeier opened this issue Jun 3, 2017 · 7 comments

Comments

@mpollmeier
Copy link

mpollmeier commented Jun 3, 2017

Original discussion here: https://contributors.scala-lang.org/t/presentation-compiler-autocompletion-bug-with-dependent-types/897

This a bit of an edge case, I guess that's why it wasn't discovered yet: Autocompletion fails when using dependent types AND omitting the (empty) first parameter block. You can reproduce it in the repl as follows:

object Test1 {
  trait Conv[In] {
    type Out
    def apply(in: In): Out
  }
  object Conv {
    type Aux[In, Out0] = Conv[In] { type Out = Out0 }
    implicit val int2String = new Conv[Int] {
      type Out = String
      override def apply(i: Int) = i.toString
    }
  }

  def withParens[Out]()(implicit conv: Conv.Aux[Int, Out]): Out = "5".asInstanceOf[Out]
  def withoutParens[Out](implicit conv: Conv.Aux[Int, Out]): Out = "5".asInstanceOf[Out]
}

Then type Test1.withParens().<TAB> - it does find autocompletions for String. However, if you try Test1.withoutParens.<TAB> it does not. As a workaround, when saving intermediate result, it works:
val a = Test1.withoutParens; a.<TAB>

Without dependent types, autocompletion works fine in both cases:

object Test2 {
  trait A
  implicit val a: A = ???
  def withParens()(implicit a: A): String = "something"
  def withoutParens(implicit a: A): String = "something"
}

The best solution would be to fix this in the presentation compiler, but I didn't manage to pin down the problem there. Instead, as a workaround, I patched the repl autocompletion - if it doesn't find any autocompletions, it interprets the code up until that point and then tries again. Here's a mini repo that's using the presentation compiler and the patched autocompletion:
https://github.com/mpollmeier/scala-presentation-compiler-autocomplete-fix-
If you use PresentationCompilerCompleter instead of MyPresentationCompilerCompleter in the test, you get the old behaviour (no autocompletion).

From @retronym:

I've prototyped a fix for the REPL autocompletion here: https://github.com/retronym/scala/tree/topic/completion-depmet-implicit

It needs a bit more work to see if the fix should really be pushed into typeMembers, rather than having the if/else in completionsAt. A good litmus test would be to try to reproduce the bug in Scala IDE. If that also fails to autocomplete, the root problem is likely in typeMembers, which is finding members of the type of the smallest tree that encloses the search position. There are two trees with the same range position: ApplyImplicitArgs(Select(...), args), and locate finds the inner Select. Not sure why the dependent types figure into the equation yet.

@mpollmeier
Copy link
Author

I just tested this with ScalaIDE and Ensime, both show the same behaviour as the REPL.

@mpollmeier
Copy link
Author

mpollmeier commented Jun 3, 2017

I built your branch and can confirm that it's fixed for the repl (when starting sbt scala in your branch). Then I tested with ScalaIDE and Ensime, but neither one showed any completions.

Here's what I did:

  1. publishLocal
  2. configured the ScalaIDE project to use the newly created scala-library and scala-reflect jars (remove Scala Library, add those two external jars). Not sure if/how I can replace the scala-compiler.jar?
  3. Ensime:
    sed -i "s#/home/mp/.coursier/cache/v1/https/repo1.maven.org/maven2/org/scala-lang/scala-reflect/2.12.2/scala-reflect-2.12.2.jar#/home/mp/.ivy2/local/org.scala-lang/scala-reflect/2.12.3-bin-SNAPSHOT/jars/scala-reflect.jar#g" .ensime
    sed -i "s#/home/mp/.coursier/cache/v1/https/repo1.maven.org/maven2/org/scala-lang/scala-library/2.12.2/scala-library-2.12.2.jar#/home/mp/.ivy2/local/org.scala-lang/scala-library/2.12.3-bin-SNAPSHOT/jars/scala-library.jar#g" .ensime
    sed -i "s#/home/mp/.coursier/cache/v1/https/repo1.maven.org/maven2/org/scala-lang/scala-compiler/2.12.2/scala-compiler-2.12.2.jar#/home/mp/.ivy2/local/org.scala-lang/scala-compiler/2.12.3-bin-SNAPSHOT/jars/scala-compiler.jar#g" .ensime

@retronym
Copy link
Member

retronym commented Jun 7, 2017

@mpollmeier I've pushed an alternative fix to scala/scala#5927. Could you see if that improves the situation with Ensime, too?

@fommil
Copy link

fommil commented Jun 7, 2017

@retronym @mpollmeier there is also https://github.com/ensime/pcplod to set up an easy test for any piece of code in the presentation compiler.

Somewhat related, I've dreamt of writing a presentationCompiler task for sbt that compiles a module with the pc instead of the batch compiler... it is easy to do but I never find the time.

I'm waiting for @yangwen0228 to confirm if https://github.com/ensime/ensime-emacs/issues/624 is a real presentation compiler bug or not (could be in our emacs code) and if so it would be super useful if you could help us to write it up in the scala/scala presentation compiler test format. I'd really like to document that in the ensime docs... I've been trying to encourage people to contribute to improve the PC for a while now but we've seen very little interest.

@mpollmeier
Copy link
Author

@fommil I actually started with pcplod for my analysis, but it doesn't (yet) allow to verify the autocomplete suggestions from the PC, right? I think I can add this to pcplod though if that makes sense.

@fommil
Copy link

fommil commented Jun 7, 2017

ok that's be cool! Usually infer type is enough but that'd be a nice addition.

@mpollmeier
Copy link
Author

(double post)
@retronym I tested with ensime and now the autocompletion works fine in both cases :)
I didn't have to change anything other than the scala jars in .ensime. As @fommil said, ensime uses -Ymacro-expand:discard by default.

Btw as you suggested I tested the combination of give the combination of scala/scala#5929 and scala/scala#5927

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants