Skip to content
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

REPL ImportHandler is not tracking importedNames and importedSymbols correctly #9881

Closed
scabug opened this issue Aug 7, 2016 · 7 comments
Closed

Comments

@scabug
Copy link

scabug commented Aug 7, 2016

Under current implementation, both fields are mostly empty. An immediate consequence is :imports doesn't report anything. Also it causes problems to other components that rely on these fields, such as when defining classes in REPL with -Yrepl-class-based enabled. See issue [SI-9880].

There seem to be a few issues here:

  • sym.thisType on a module symbol always returns NoPrefix. Should probably use sym.typeOfThis instead.
  • It doesn't handle renames because it won't be able to find the renames in importable members.
  • importableMembers may not always be able to return complete list (the case I encountered was with scala.reflect.runtime.universe which has a type of scala.reflect.api.JavaUniverse). Thus we probably should not restrict importedNames to only importableMembers for non-wildcard imports.
  • This is caused by typeOfExpression inadvertently changing phase to jvm.
@scabug
Copy link
Author

scabug commented Aug 7, 2016

Imported From: https://issues.scala-lang.org/browse/SI-9881?orig=1
Reporter: @jasonxh
Affected Versions: 2.11.8, 2.12.0-M5
See #9880
Duplicates #7953

@scabug
Copy link
Author

scabug commented Aug 7, 2016

@som-snytt said (edited on Aug 7, 2016 5:59:03 PM UTC):
I took a look, too, with the same conclusion. I didn't notice #2 about renames but verified it. #3 I'm not sure. There's a related ticket for handling imported implicits, which is why I looked at it. Were you planning a PR?

Showing failure on rename. Works on regular module import. Also the failure on package import from the other ticket.

$ scala -Yrepl-class-based
Welcome to Scala 2.12.0-M5 (Java HotSpot(TM) 64-Bit Server VM, Java 1.8.0_92).
Type in expressions for evaluation. Or try :help.

scala> object X { def x = 42 }
defined object X

scala> import X.{x => y}
import X.{x=>y}

scala> class C(i: Int = y)
<console>:11: error: not found: value y
       class C(i: Int = y)
                        ^

scala> class C ; y
<console>:13: error: not found: value y
        y
        ^

scala> import X.x
import X.x

scala> class C(i: Int = x)
defined class C

scala> import concurrent.Future
import concurrent.Future

scala> class C(f: Future[Int])
<console>:11: error: not found: type Future
       class C(f: Future[Int])
                  ^

scala> def f: Future[Int] = ???
f: scala.concurrent.Future[Int]

@scabug
Copy link
Author

scabug commented Aug 7, 2016

@jasonxh said:
@A. P. Marki Yes I'm working on a PR.

@scabug
Copy link
Author

scabug commented Aug 8, 2016

@SethTisue said:
assigning a fix version of 2.11.9 because if :imports doesn't work at all we should at least remove it from the help. at bare minimum. obviously an actual fix would be much better

@scabug
Copy link
Author

scabug commented Aug 10, 2016

@jasonxh said:
Opened PR at scala/scala#5326

@scabug
Copy link
Author

scabug commented Aug 13, 2016

@som-snytt said (edited on Aug 13, 2016 7:04:31 AM UTC):
Sorry I didn't remember this.

Apparently, I started looking at it but then forgot about it.

scala/scala#4698

retronym's link to WIP is probably still relevant.

@scabug
Copy link
Author

scabug commented Jan 19, 2017

@som-snytt said:
scala/scala#5640

@scabug scabug closed this as completed Feb 20, 2017
@scabug scabug added this to the 2.12.2 milestone Apr 7, 2017
asfgit pushed a commit to apache/spark that referenced this issue Dec 1, 2017
…lass constructors, extends clause

## What changes were proposed in this pull request?

[SPARK-22393](https://issues.apache.org/jira/browse/SPARK-22393)

## How was this patch tested?

With a new test case in `RepSuite`

----

This code is a retrofit of the Scala [SI-9881](scala/bug#9881) bug fix, which never made it into the Scala 2.11 branches. Pushing these changes directly to the Scala repo is not practical (see: scala/scala#6195).

Author: Mark Petruska <petruska.mark@gmail.com>

Closes #19846 from mpetruska/SPARK-22393.
adriaanm pushed a commit to mpetruska/scala that referenced this issue Jun 1, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants