scalameta / metals

Scala language server with rich IDE features 🚀
https://scalameta.org/metals/
Apache License 2.0
2.08k stars 330 forks source link

Folders with mixed Scala version (2.13 / 3.2) scripts consider the whole folder as Scala 2.13 project #4481

Open carlosedp opened 2 years ago

carlosedp commented 2 years ago

Describe the bug

I have a folder with multiple Scala utility scripts (.sc files). After opening VSCode in the folder and opening any of the scripts (2.13 or 3), Metals detects it and I click "Scala CLI".

image

I use //> using scala "2.13" / //> using scala "3" on each script but when opening VSCode / Metals, the whole folder (project) is considered a 2.13 project.

image

Expected behavior

Detect each file/script version based on it's own using directive.

Operating system

macOS

Editor/Extension

VS Code

Version of Metals

0.11.8+202-bd7c2e4d-SNAPSHOT

Extra context or search terms

No response

carlosedp commented 2 years ago

Samples:

❯ ll
.rwxr-xr-x 84 cdepaula  4 Oct 16:35 scala3.sc*
.rw-r--r-- 86 cdepaula  4 Oct 16:34 scala213.sc
❯ cat *
────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
File: scala213.sc
Size: 111 B
────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
#!/usr/bin/env -S scala-cli shebang

//> using scala "2.13"

println("I'm scala 2.13")

if (0 == 0) {"itszero"}
────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
File: scala3.sc
Size: 161 B
────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
#!/usr/bin/env -S scala-cli shebang

//> using scala "3.2"

println("I'm scala 3.2")

if 0 == 0 then "itszero"  // Shows `( expected but integer constant found`
────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────

image

tgodzik commented 2 years ago

Thanks for reporting! By default we try to include all the files within a directory together in order to allow you to import things between them.

There is a couple of options available to fix your current issue:

I think the second option would probably be best. Not sure if we could sensibly use the third one.

carlosedp commented 2 years ago

Thanks for the feedback Tomasz, my 2 cents:

always treat scripts as standalone (no importing from other files)

I think this is a good idea for scripts since they are usually self-contained and the ideal way is to explicitly import other scripts thru $file or //> using file .... If things are imported, would metals be able to reference the imported file?

try to check other files for the using directives, if they are in multiple files then just import the script there is apparently a scala target directive that can be helpful here and Metals would just use whichever Scala version you open first: //> using target.scala "2.13"

This might not solve the issue for multiple scripts directory...

carlosedp commented 2 years ago

Maybe an idea for folders with multiple scripts (no build tool files), would be having Metals handle one Build target for each script...

tanishiking commented 2 years ago

Maybe an idea for folders with multiple scripts (no build tool files), would be having Metals handle one Build target for each script...

Yeah, that's true. and @dos65 mentioned something about multiple build server/target stuff https://github.com/scalameta/metals/pull/4455#discussion_r983491205

SethTisue commented 1 year ago

I'm not sure what the right way forward here is, but I just want to say that I've hit this repeatedly as well. I'm using scala-cli to write lots of scripts these days, and they're usually self-contained and unrelated to each other. So I wish it weren't problematic to throw 2 or more such scripts in the same directory.

tgodzik commented 1 year ago

We could change the default for when we discover scripts in Metals. Since it's run alongside an existing workspace maybe it doesn't make sense to default to a directory? :thinking:

tgodzik commented 1 year ago

On the other hand if we change the default there will be no possibility to use multiple files.

ckipp01 commented 1 year ago

On the other hand if we change the default there will be no possibility to use multiple files.

Yea this would be a huge deal breaker for me and it would sort of kill the whole "single module workspace" for scala-cli.

SethTisue commented 1 year ago

if we change the default there will be no possibility to use multiple files

not even with //> using file ...? I guess I was hoping that shebang scripts could be considered self-contained by default, but with that as the escape hatch.

tgodzik commented 1 year ago

On the other hand if we change the default there will be no possibility to use multiple files.

Yea this would be a huge deal breaker for me and it would sort of kill the whole "single module workspace" for scala-cli.

Though, that would only happen if it's not your main tool in a workspace.

if we change the default there will be no possibility to use multiple files

not even with //> using file ...? I guess I was hoping that shebang scripts could be considered self-contained by default, but with that as the escape hatch.

Not sure, we would need to check that.

SethTisue commented 1 year ago

I guess I was hoping that shebang scripts could be considered self-contained by default, but with that as the escape hatch

(Note that it's unclear to me whether I'm suggesting a Metals change only, or suggesting a scala-cli change that Metals would then adapt to.)

tgodzik commented 1 year ago

This would be a Metals only change. Basically what metals does currently is when we see script.sc we invoke scala-cli on a parent dir with the setup-ide command, which then becomes a scala-cli directory.

Coming to think of it we could have two options scala-cli(file) and scala-cli(directory) to choose from. What that be a good idea?

ckipp01 commented 1 year ago

Coming to think of it we could have two options scala-cli(file) and scala-cli(directory) to choose from. What that be a good idea?

Ooo, that might be a good idea.

kubukoz commented 1 year ago

Should there be a third option (workspace)? Just in case someone has a bunch of .sc files in a workspace and they've already clarified what the sources are (by means of calling scala-cli <sources> which would generate the .bsp entry).

carlosedp commented 1 year ago

Another important thing would be nice to have some kind of pattern to identify the test file for certain script, like myscript.sc or myscript.scala and myscript.test.scala being part of the same "project".