Java support status placeholder #3398

Arthurm1 · 2021-12-23T15:20:29Z

ckipp01 · 2021-12-23T15:28:19Z

So, more of just a question, but in the TODO section, how many of these things do we actually want? Is the goal truly to cover them all? It's incredible that we now support java targets, but I'm also wondering at what point does Metals, the Scala language server venture too far into the Java land support? Realistically we'll never reach parity with something like IntelliJ for cross-JVM languages, but I also don't really think that's a goal. Adding all the features on this list is no small task, and also increases the scope of Metals. Is that what we want? I guess I just never assumed that something like formatting on type for Java sources is something that Metals would support or want to support. Again, I don't mean this to take away from the fantastic achievement of the Java support we now have. #2520 is truly fantastic work! I'd just love to get a clearer idea of what the ultimate goal is with our Scala Java interop and Java support.

Arthurm1 · 2021-12-23T15:49:18Z

I'm pretty happy with the current set of functionality. It makes working with combined Scala/Java codebases so much easier.

I think run/debug main and code folding should be simple to add and help ease of use and I will probably look to implement.

As for the others, I thought I'd fix whatever problems came up after release and then wait to see if anyone has a real need for anything else. I doubt I'll be looking to add any of it anytime soon. (I don't actually need or want format-on-type)

In terms of increasing the scope of Metals - I shift between thinking that a larger scope and hence larger user base is a good thing for supplying ideas for helpful features and for finding bugs but possibly a bad thing considering a lot of Java requirements probably won't overlap very much with Scala. And of course, all this java stuff has to be supported by scala devs 😢

ckipp01 · 2021-12-23T15:57:12Z

😆 of course, a larger user base is always great, but I think it's important to talk about where we'd like to end up in terms of interop.

I'm maybe an outlier in that if I talk with people that work in heavy Java / Scala projects, I recommend IntelliJ with zero hesitation. We also have a decent size user base that uses Metals because it's lighter, faster, and more agile due to its limited scope. I'd hate to lose that.

And of course, all this java stuff has to be supported by scala devs 😢

Indeed.

tgodzik · 2021-12-23T16:19:01Z

A lot of the features that depend on semanticdb should be simple to add and should use exactly the same logic. The hardest part would be things requiring PC, which I don't believe is in our reach. I would also not support any refactorings and focus on the Scala ones. Of course if anyone is willing to contribute anything I would gladly accept those, but I don't think the should ever be in our main scope.

One thing that would be awesome is if we were able to start java language server in the background and make Metals as a intermediary, which means a lot of things could come for free. But this is a whole other rabbit hole, or rather a dragon's cave of difficulties.

tgodzik · 2021-12-23T16:19:48Z

Btw. I think we could do hover using semanticdb, which would be low effort, but could help out people investigating their codebases.

tgodzik · 2021-12-23T19:37:11Z

I took a look at fixing renames and there seems to be an issue with file watcher for java semanticdb files and also semanticdb for Java files starts at 0 instead of 1 like in Scala.

My attempt is here: https://github.com/scalameta/metals/compare/main...tgodzik:fix-java-renames?expand=1

Another issue we noticed is that we should not show java in the doctor for Scala only files (or figure out a smarter way of doing it all)

Arthurm1 · 2021-12-23T19:52:03Z

For the Doctor - I guess we could check if there are no java files in the project - then not show the java info. I'm not sure there's anything from the BSP to indicate this as it's valid to have empty javacoptions.

Not sure it's possible to even specify in the build tools that the project is only Scala. With Gradle you can't have a project that supports only Scala (java support is implicit). Not sure if that's the same as SBT, Mill, Bazel. I guess they all use Zinc under the hood so it'll quite happily compile a java file?

ckipp01 · 2021-12-23T20:04:20Z

One thing I keep seeing for example when in the Java files in Metals is this in the logs:

2021.12.23 21:03:39 INFO  no build target found for /Users/ckipp/Documents/scala-workspace/scalameta-org/metals/.metals/.tmp/Main-2978370456315699633.scala. Using presentation compiler with project's scala-library version: 2.13.7
Dec 23, 2021 9:03:41 PM scala.meta.internal.pc.CompilerAccess retryWithCleanCompiler
INFO: compiler crashed due to an error in the Scala compiler, retrying with new compiler instance.
Dec 23, 2021 9:03:41 PM scala.meta.internal.pc.CompilerAccess handleError
SEVERE: object scala.runtime in compiler mirror not found.
scala.reflect.internal.MissingRequirementError: object scala.runtime in compiler mirror not found.
        at scala.reflect.internal.MissingRequirementError$.signal(MissingRequirementError.scala:24)
        at scala.reflect.internal.MissingRequirementError$.notFound(MissingRequirementError.scala:25)
        at scala.reflect.internal.Mirrors$RootsBase.$anonfun$getModuleOrClass$5(Mirrors.scala:61)
        at scala.reflect.internal.Mirrors$RootsBase.getPackage(Mirrors.scala:61)
        at scala.reflect.internal.Definitions$DefinitionsClass.RuntimePackage$lzycompute(Definitions.scala:198)
        at scala.reflect.internal.Definitions$DefinitionsClass.RuntimePackage(Definitions.scala:198)
        at scala.reflect.internal.Definitions$DefinitionsClass.RuntimePackageClass$lzycompute(Definitions.scala:199)
        at scala.reflect.internal.Definitions$DefinitionsClass.RuntimePackageClass(Definitions.scala:199)
        at scala.reflect.internal.Definitions$DefinitionsClass.AnnotationDefaultAttr$lzycompute(Definitions.scala:1251)
        at scala.reflect.internal.Definitions$DefinitionsClass.AnnotationDefaultAttr(Definitions.scala:1250)
        at scala.reflect.internal.Definitions$DefinitionsClass.syntheticCoreClasses$lzycompute(Definitions.scala:1408)
        at scala.reflect.internal.Definitions$DefinitionsClass.syntheticCoreClasses(Definitions.scala:1407)
        at scala.reflect.internal.Definitions$DefinitionsClass.symbolsNotPresentInBytecode$lzycompute(Definitions.scala:1450)
        at scala.reflect.internal.Definitions$DefinitionsClass.symbolsNotPresentInBytecode(Definitions.scala:1450)
        at scala.reflect.internal.Definitions$DefinitionsClass.init(Definitions.scala:1506)
        at scala.tools.nsc.Global$Run.<init>(Global.scala:1213)
        at scala.tools.nsc.interactive.Global$TyperRun.<init>(Global.scala:1323)
        at scala.tools.nsc.interactive.Global.newTyperRun(Global.scala:1346)
        at scala.tools.nsc.interactive.Global.<init>(Global.scala:294)
        at scala.meta.internal.pc.MetalsGlobal.<init>(MetalsGlobal.scala:37)

I don't fully get where the Main-2978370456315699633.scala is coming from, but it happens every time I navigate to a java source.

Arthurm1 · 2021-12-23T20:32:28Z

I think /.tmp/mainXXX.scala gets created when ScalaFix warming up and this gets triggered when indexing is complete. I don't think it's java relevant

metals/metals/src/main/scala/scala/meta/internal/metals/ScalafixProvider.scala

Line 56 in 11c7c27

.resolve(s"Main${Random.nextLong()}.scala")

I've never seen the mirror issue. You get this when navigating from something like scala.meta.internal.pc.ScalaPresentationCompiler.scala to scala.meta.pc.PresentationCompiler.java?

kpodsiad · 2021-12-23T20:41:22Z

@ckipp01 @Arthurm1 I can confirm that .metals/.tmp/mainXXX.scala is not related to the java support. Previously it'll be no build target: using presentation compiler with only scala-library ... but I changed it in #3388 to include path.

We should filter this out, it can be very misleading.

ckipp01 · 2021-12-23T21:00:02Z

Ahhh sorry for the noise!! I never noticed it before 😬 and happened to when paying more attention to the logs now when trying every out.

Arthurm1 · 2021-12-23T21:08:26Z

No worries - I fully expect some issue(s) as it's a big PR and I've only run it on Windows+WSL which I doubt is that common. Plus I may have been a bit lax on writing tests 😟

ckipp01 · 2021-12-26T11:12:14Z

Another issue we noticed is that we should not show java in the doctor for Scala only files (or figure out a smarter way of doing it all)

After chatting with @Arthurm1 on Discord about this a bit, here's what I'd propose. What if instead of showing an entire new target for this we just do something like this:

### foo
  - scala version: 2.13.7
  - diagnostics: ✅ 
  - goto definition: ✅ 
  - completions: ✅ 
  - find references: ✅ 
  - java support: ✅

And java support is calculated based on diagnostics + goto definition + find references in the java target all being green. I find this less intrusive and can potentially avoid confusion for newcomers seeing two different foos, one for Scala and one for Java. Especially seeing that right now no matter what they are getting a warning about completions in the java target not working.

dos65 · 2021-12-30T19:57:31Z

Additional issue. If you open a scala-cli project as it was described in #3414 Metals warns about Code navigation will not work for 2 build targets in this workspace due to mis-configuration. Select 'More information' to learn how to fix this problem...

But this one incorrect as there is no java sources in project.

ckipp01 · 2022-01-01T20:48:34Z

One thing I just hit on in a project which isn't a huge issue, but still should probably be addressed, I have a Tree type in my project and when trying to complete Tree I got the following as a completion option:

com.sourcegraph.semanticdb_javac.Semanticdb.Tree

I guess I wouldn't expect this to be offered as a completion item. How come this is, and is there a way we can ensure this isn't offered unless a user is explicitly using this?

tgodzik · 2022-01-03T11:31:30Z

That's because the java plugin ends up on the classpath :/ We could exclude it from completions, but this means if anyone wants to use it the would need to modify the exclude paths 🤔

ckipp01 · 2022-01-03T11:37:15Z

That's because the java plugin ends up on the classpath :/ We could exclude it from completions, but this means if anyone wants to use it the would need to modify the exclude paths 🤔

🤔 hmm excluding isn't ideal either, that could probably cause a lot of confusion for someone wanting to actually use that dep and then they get no completions unless it's removed. I'm not sure then.

Arthurm1 · 2022-01-03T15:44:10Z

Could we ask Sourcegraph to release an additional jar that's entirely shaded? And Metals/Bloop could use that one?

dos65 · 2022-01-03T16:15:59Z

@tgodzik

That's because the java plugin ends up on the classpath :/ We could exclude it from completions, but this means if anyone wants to use it the would need to modify the exclude paths thinking

I think it might be solved on bloop side. I don't see sourcegraph jars in bloop json files so it seems that the returned only over bsp request

Arthurm1 · 2022-01-03T16:22:39Z

The jar gets added to the classpath dynamically here based on the contents of the bloop.settings.json file

kpodsiad · 2022-01-04T09:52:11Z

I wanted to test java support in Metals. I opened a Gradle project with only java sources and Metals won't start since there is no .scala file.
Should we bother to support such cases? @tgodzik had an idea that we can add a command Enable Metals. It might be a good idea, it doesn't conflict with java lsp and we don't have to create additional conditions when to enable Metals.

dos65 · 2022-01-04T10:01:13Z

@kpodsiad

Should we bother to support such cases?

I can't imagine the case when somebody wants to use Metals on a full-java project.

ckipp01 · 2022-01-04T10:02:45Z

@kpodsiad

Should we bother to support such cases?

I can't imagine the case when somebody wants to use Metals on a full-java project.

Agreed, imo this isn't an area we want to venture in.

tgodzik · 2022-01-04T14:07:41Z

I did a quick PoC of showing hover using semanticdb here, but I am not 100% sure if we want to go this way.

What we would need to do still:

use GlobalClassTable for dependency symbols
find symbols in workspace for anything outside the current file
add modifiers

Main drawback of this would be that it would be much less efficient and non-interactive.

We should probably move everything that we don't plan to fix currently into separate feature requests and discuss the topics there.

Arthurm1 · 2022-01-04T14:19:26Z

@kpodsiad I think it would be nice to have a command to start Metals. I find it strange that you can navigate to the Metals tab on VSCode and can't use Metals until opening a Scala file. It would be nice to have a "Start Metals Anyway" button underneath the "New Scala Project" button.

tgodzik · 2022-03-11T15:08:02Z

Closing this one, I mentioned what still needs to be done in scalameta/metals-feature-requests#5

kpodsiad mentioned this issue Dec 26, 2021

Proposed workaround for #3379. #3404

Closed

ckipp01 added java mtags java support Java support related tickets and removed java mtags labels Dec 29, 2021

tgodzik mentioned this issue Dec 31, 2021

Show more information in the doctor #3426

Merged

tgodzik closed this as completed Mar 11, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Java support status placeholder #3398

Java support status placeholder #3398

Arthurm1 commented Dec 23, 2021 •

edited by tgodzik

Loading

ckipp01 commented Dec 23, 2021

Arthurm1 commented Dec 23, 2021

ckipp01 commented Dec 23, 2021

tgodzik commented Dec 23, 2021

tgodzik commented Dec 23, 2021

tgodzik commented Dec 23, 2021

Arthurm1 commented Dec 23, 2021

ckipp01 commented Dec 23, 2021 •

edited

Loading

Arthurm1 commented Dec 23, 2021

kpodsiad commented Dec 23, 2021

ckipp01 commented Dec 23, 2021

Arthurm1 commented Dec 23, 2021

ckipp01 commented Dec 26, 2021

dos65 commented Dec 30, 2021

ckipp01 commented Jan 1, 2022

tgodzik commented Jan 3, 2022

ckipp01 commented Jan 3, 2022

Arthurm1 commented Jan 3, 2022

dos65 commented Jan 3, 2022

Arthurm1 commented Jan 3, 2022

kpodsiad commented Jan 4, 2022

dos65 commented Jan 4, 2022

ckipp01 commented Jan 4, 2022

tgodzik commented Jan 4, 2022

Arthurm1 commented Jan 4, 2022

tgodzik commented Mar 11, 2022

Java support status placeholder #3398

Java support status placeholder #3398

Comments

Arthurm1 commented Dec 23, 2021 • edited by tgodzik Loading

ckipp01 commented Dec 23, 2021

Arthurm1 commented Dec 23, 2021

ckipp01 commented Dec 23, 2021

tgodzik commented Dec 23, 2021

tgodzik commented Dec 23, 2021

tgodzik commented Dec 23, 2021

Arthurm1 commented Dec 23, 2021

ckipp01 commented Dec 23, 2021 • edited Loading

Arthurm1 commented Dec 23, 2021

kpodsiad commented Dec 23, 2021

ckipp01 commented Dec 23, 2021

Arthurm1 commented Dec 23, 2021

ckipp01 commented Dec 26, 2021

dos65 commented Dec 30, 2021

ckipp01 commented Jan 1, 2022

tgodzik commented Jan 3, 2022

ckipp01 commented Jan 3, 2022

Arthurm1 commented Jan 3, 2022

dos65 commented Jan 3, 2022

Arthurm1 commented Jan 3, 2022

kpodsiad commented Jan 4, 2022

dos65 commented Jan 4, 2022

ckipp01 commented Jan 4, 2022

tgodzik commented Jan 4, 2022

Arthurm1 commented Jan 4, 2022

tgodzik commented Mar 11, 2022

Arthurm1 commented Dec 23, 2021 •

edited by tgodzik

Loading

ckipp01 commented Dec 23, 2021 •

edited

Loading