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

Comments

Projects
None yet
2 participants
@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

This comment has been minimized.

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

This comment has been minimized.

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

This comment has been minimized.

Copy link
Author

scabug commented Aug 7, 2016

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

@scabug

This comment has been minimized.

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

This comment has been minimized.

Copy link
Author

scabug commented Aug 10, 2016

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

@scabug

This comment has been minimized.

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

This comment has been minimized.

Copy link
Author

scabug commented Jan 19, 2017

@scabug scabug closed this 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

[SPARK-22393][SPARK-SHELL] spark-shell can't find imported types in c…
…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 added 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
You can’t perform that action at this time.