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

Worksheet does not recognize stuff in the default package at top level #2363

Open
bjornregnell opened this issue Jan 2, 2021 · 18 comments
Open
Labels
improvement Not a bug or a feature, but something general we can improve worksheets Use for tickets related to worksheets

Comments

@bjornregnell
Copy link
Contributor

bjornregnell commented Jan 2, 2021

Describe the bug

When creating modules on top level in src or in src/main/scala without any package (i.e. the default package) then these values are not recognized in any worksheet.

To Reproduce

Steps to reproduce the behavior:

  1. create an object at top level, e.g. an object Main in src/main/scala/Main.scala containing a val msg = "hello"
  2. create a new scala file worksheet called test.worksheet.sc
  3. Try to evaluate Main.msg
  4. Error: not found

Expected behavior

Top level values should be recognized in worksheets.

Example: The test2.worksheet.sc should recognize Main.msg in screenshot below.

Screenshots

image

Installation:

  • Operating system: Linux
  • Editor: Visual Studio Code
  • Metals server version: v0.9.8
@ckipp01 ckipp01 added the worksheets Use for tickets related to worksheets label Jan 2, 2021
@tgodzik
Copy link
Contributor

tgodzik commented Jan 2, 2021

Thanks for reporting! So I think that actually works as intended. Empty package cannot be imported or accessed from outside from what I understand.

And worksheets are outside the compilation unit that contains the empty packages. I am not even sure it is possible to make it work properly.

@tgodzik
Copy link
Contributor

tgodzik commented Jan 2, 2021

From what I found:

Top-level definitions outside a packaging are assumed to be injected into a special empty package. That package cannot be named and therefore cannot be imported

and since we would need to import the members of that empty package, it might be impossible to do so sensibly.

@bjornregnell
Copy link
Contributor Author

But sbt console works with the empty/default package...

And there is something going on in metals; if you start typing Main you'll actually get Main _empty_ as a completion option, so it has access to the stuff that is in an empty package. However, for some reason it's not correctly being included in the visible stuff.

Also just because the empty package cannot be imported does not mean it cannot be "automatically" visible... As it is in "normal" compiled code and as in sbt console.

@bjornregnell
Copy link
Contributor Author

bjornregnell commented Jan 3, 2021

So I don't think it works as intended/expected if you follow how it works in sbt console and in compiled code.

It seems to me like an arbitrary, metals-specific restriction that you must put your code in a package, instead of just in src/main/scala directly as in the example screenshot above, as "normal" Scala can have stuff on top.

@tgodzik
Copy link
Contributor

tgodzik commented Jan 4, 2021

I agree that it would be much better if it worked, I am just not sure how feasible it is to do. What I think happens here is that we give the compiler a classpath where that empty package is already compiled and the compiler does not see it because of the quoted rule. Maybe there is a compiler flag? We'll need to dig in.

But sbt console works with the empty/default package...

Sbt sees everything because it's all in the same compilation unit and that's probably why it's working. Worksheets are a separate entity, which causes a problem here.

@tgodzik tgodzik added improvement Not a bug or a feature, but something general we can improve upstream-fix-needed Waiting on a fix upstream and removed upstream-fix-needed Waiting on a fix upstream labels Jan 4, 2021
@bjornregnell
Copy link
Contributor Author

Aha. Thanks for the explanation! Seems tricky... Sbt has all compiled stuff in target/scala-2.13/classes/. So if you cd to that dir you can scala -cp . and then get access to all your code in the REPL session, also the default package.

As the top level stuff in metals is visible when you click on run over main, there should be some way to create a classpath (or a jar, or a virtual dir, or whatever) with the default package included in the compiled pile of code? (Or am I missing something)

@tgodzik
Copy link
Contributor

tgodzik commented Jan 4, 2021

The presentation compiler seems to see that empty package (hence the completions etc.), so maybe we could force it somehow too... Maybe some kind of a REPL mode?

@bjornregnell
Copy link
Contributor Author

Sounds promissing! 💡

Do you mean that the user should shift a setting in metals for worksheets to work that way, or do you mean that the worksheet is in this "REPL mode" from start?

@tgodzik
Copy link
Contributor

tgodzik commented Jan 4, 2021

or do you mean that the worksheet is in this "REPL mode" from start?

I think this one, but I am just guessing here. Nothing like this might exist.

@bjornregnell
Copy link
Contributor Author

Ok that would be great! And there is no other way, except special "REPL mode" of getting the bytecode of the default package on classpath like happens when clicking "run/debug"?

@tgodzik
Copy link
Contributor

tgodzik commented Jan 4, 2021

Honestly, I don't know yet.

@bjornregnell
Copy link
Contributor Author

Ok, thanks! I'd be happy to help out with testing new stuff and give input to how it could work from a user perspective if needed.

@bjornregnell
Copy link
Contributor Author

bjornregnell commented May 19, 2021

@tgodzik Any further info on the possibility of worksheets at top level seeing default package stuff esp in builds with no src/blah ceremony? (I'm planning changes in student workspaces and teaching material so this will have impact on which way I go)

@bjornregnell bjornregnell changed the title Workshet does not recognize stuff in the default package at top level Worksheet does not recognize stuff in the default package at top level May 19, 2021
@tgodzik
Copy link
Contributor

tgodzik commented May 20, 2021

Checked a couple of things again and my current conclusion is that we can't really do it without some kind of guessing or at least workarounds. There are two main problems:

  • the build tool is unable to tell us what target should the worksheet belong to as it isn't contained in any of the source directories. The logic sbt operates with is custom to sbt. Workaround in sbt would be to add Compile/ unmanagedSourceDirectories += (ThisBuild / baseDirectory).value. Edit: I would not recommend it, seems a lot of things might break then.
  • empty package can only be accessed in the same compilation unit. I am not sure how to fix that, I haven't looked into this yet. Workaround would be to always use package.

@tgodzik
Copy link
Contributor

tgodzik commented May 20, 2021

The second point seems to be a Java limitation as far as I can tell.

tgodzik added a commit to tgodzik/mdoc that referenced this issue May 20, 2021
Related to scalameta/metals#2363

This simplifies worksheets for users, since they can define separate classes in files and use in workspaces.
tgodzik added a commit to tgodzik/mdoc that referenced this issue May 20, 2021
Related to scalameta/metals#2363

This simplifies worksheets for users, since they can define separate classes in files and use in workspaces.
@tgodzik
Copy link
Contributor

tgodzik commented May 20, 2021

Or is it? Seems I am able to work around it in scalameta/mdoc#505

Though not sure if that would work for Scala 3.

As for the files in root directory I think it is not possible for us to work around sensibly aside from mimicking sbt logic. Not sure if that is a good solution.

Edit: I would probably just recommend having at least src dir and point to that in sbt/mill/gradle. Though, probably Mill is best suited here.

@bjornregnell
Copy link
Contributor Author

Thanks for looking into this! In the beginning of a beginner programming course I think it is best to use the bare minimum build.sbt and keeping it as simple as possible. Each new cryptic line is a hurdle for pure beginners that have never seen code before and drains their grit and feeling of being lost. That is why I'm so engaged in this. A beginner gets an important sense of control if a file is created from scratch by hand and the the beginner has written every keystroke be themselves.

If it is at all possible to use the sbt logic I think it would make the barrier to scala+metals significantly lower for pure beginners. But if it turns out to be impossible or too fragile or too much work I think, I will in my teaching probably stick with plain REPL in terminal on my lectures and in exercises for students instead of worksheets in vscode+metals. But I will investigate the workarounds you propose. Thanks again.

@bjornregnell
Copy link
Contributor Author

just tested: empty package does not work in scala3 using a file test.worksheet.scala that access compiled code in .scala fiels, even if the worksheet file is in src/main/scala

But if a package declaration is made it works as expected.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
improvement Not a bug or a feature, but something general we can improve worksheets Use for tickets related to worksheets
Projects
None yet
Development

No branches or pull requests

3 participants