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

Support project view in TVP panel for Scala 3 #2859

Closed
ckipp01 opened this issue Jun 5, 2021 · 4 comments · Fixed by #5622
Closed

Support project view in TVP panel for Scala 3 #2859

ckipp01 opened this issue Jun 5, 2021 · 4 comments · Fixed by #5622
Labels
Scala 3 Generic ticket relating to Scala 3 TVP Tickets related to the Tree View Protocol
Milestone

Comments

@ckipp01
Copy link
Member

ckipp01 commented Jun 5, 2021

I am not sure this is a bug or the feature isn't support for scala 3 yet.
I created a new sbt project using scala3.g8 template. Then I created some classes and objects in the Main.scala file.
The project explorer only detected the package, nothing else were shown unlike the Libraries section,

image

Originally posted by @Fubuchi in #2855

@ckipp01 ckipp01 changed the title Projects not appearing in TVP panel. Projects not appearing in TVP panel in Scala 3 Jun 5, 2021
@ckipp01
Copy link
Member Author

ckipp01 commented Jun 5, 2021

To add a little more context, I'd expect this to work in Scala 3 like it does in Scala 2. However, even after some more classes I've been unable to get TVP to show local stuff in the tree view.

Steps to recreate:

  1. create a new scala3 project using the metals new project functionality.
  2. Go to Main.scala and then open the TVP panel.
  3. See that nothing is there under example.
  4. To test further, add another object Foo with a couple members
  5. Also see they don't appear in the TVP panel.

@ckipp01 ckipp01 added the Scala 3 Generic ticket relating to Scala 3 label Jun 5, 2021
@dos65
Copy link
Member

dos65 commented Jun 5, 2021

The reason is that TVP uses ScalaSig that works only for scala2. For Scala3 we need to start reading tasty.

@ckipp01 ckipp01 changed the title Projects not appearing in TVP panel in Scala 3 Support project view in TVP panel for Scala 3 Jun 5, 2021
@ches
Copy link

ches commented Nov 18, 2021

The reason is that TVP uses ScalaSig that works only for scala2. For Scala3 we need to start reading tasty.

So this applies for the Libraries section where dependencies are built with Scala 3, I assume. If I'm on the right track then ClasspathSymbols.scala is the place to start.

For Scala 3 files local to a project, I imagine Signature and SymbolInformation support will open the way for this?

Could this extraction from JARs, abstracting over ScalaSig or TASTy, be in a SemanticDB utility along the lines of scalameta/scalameta#1566?

@dos65
Copy link
Member

dos65 commented Nov 18, 2021

@ches The only that is currently required is to add support of reading Tasty files at the place where TVP uses ScalaSig.

Everything related to Semanticdb already works well and shouldn't be connected with this thing.

@ckipp01 ckipp01 added the TVP Tickets related to the Tree View Protocol label Dec 29, 2021
tgodzik added a commit to tgodzik/metals that referenced this issue Sep 6, 2023
Previously, we would read the classpath to create the metals view, which worked well for Scala 2, but needed a reimplementation for Scala 3. So now, instead of doing this reimplementation, which would probably need to use tasty and might be quite complex, I decided to base it on the same approach we use for inexing.

This makes the feature more reliable since, we now used the same well tested functionaties as things like go to definition.

Unfortunately, we might index the particular files the second time, because the basic indexing caches are reversed and finding all symbols in a file from those maps was much slower than just doing it the second time. We later cache that information the same way we did in the previous tree view implementation. So we won't use more memory and running the benchmarks show that the new approach is similarily fast:

```
[info] Benchmark                  Mode  Cnt    Score    Error  Units
[info] ClasspathSymbolsBench.run    ss   10  542.090 ± 61.953  ms/op
```

as compared to the previous:
```
[info] Benchmark                  Mode  Cnt    Score    Error  Units
[info] ClasspathSymbolsBench.run    ss   10  477.283 ± 93.933  ms/op
```

Also it seems we index much more information, since mtags index all symbols, which would also explain the slight difference in timings as I think the difference was 1300 symbols previously and 17000 after this change (number of symbols detected in the benchmarks)

Fixes scalameta#2859
tgodzik added a commit to tgodzik/metals that referenced this issue Sep 11, 2023
Previously, we would read the classpath to create the metals view, which worked well for Scala 2, but needed a reimplementation for Scala 3. So now, instead of doing this reimplementation, which would probably need to use tasty and might be quite complex, I decided to base it on the same approach we use for inexing.

This makes the feature more reliable since, we now used the same well tested functionaties as things like go to definition.

Unfortunately, we might index the particular files the second time, because the basic indexing caches are reversed and finding all symbols in a file from those maps was much slower than just doing it the second time. We later cache that information the same way we did in the previous tree view implementation. So we won't use more memory and running the benchmarks show that the new approach is similarily fast:

```
[info] Benchmark                  Mode  Cnt    Score    Error  Units
[info] ClasspathSymbolsBench.run    ss   10  542.090 ± 61.953  ms/op
```

as compared to the previous:
```
[info] Benchmark                  Mode  Cnt    Score    Error  Units
[info] ClasspathSymbolsBench.run    ss   10  477.283 ± 93.933  ms/op
```

Also it seems we index much more information, since mtags index all symbols, which would also explain the slight difference in timings as I think the difference was 1300 symbols previously and 17000 after this change (number of symbols detected in the benchmarks)

Fixes scalameta#2859
tgodzik added a commit to tgodzik/metals that referenced this issue Sep 12, 2023
Previously, we would read the classpath to create the metals view, which worked well for Scala 2, but needed a reimplementation for Scala 3. So now, instead of doing this reimplementation, which would probably need to use tasty and might be quite complex, I decided to base it on the same approach we use for inexing.

This makes the feature more reliable since, we now used the same well tested functionaties as things like go to definition.

Unfortunately, we might index the particular files the second time, because the basic indexing caches are reversed and finding all symbols in a file from those maps was much slower than just doing it the second time. We later cache that information the same way we did in the previous tree view implementation. So we won't use more memory and running the benchmarks show that the new approach is similarily fast:

```
[info] Benchmark                  Mode  Cnt    Score    Error  Units
[info] ClasspathSymbolsBench.run    ss   10  542.090 ± 61.953  ms/op
```

as compared to the previous:
```
[info] Benchmark                  Mode  Cnt    Score    Error  Units
[info] ClasspathSymbolsBench.run    ss   10  477.283 ± 93.933  ms/op
```

Also it seems we index much more information, since mtags index all symbols, which would also explain the slight difference in timings as I think the difference was 1300 symbols previously and 17000 after this change (number of symbols detected in the benchmarks)

Fixes scalameta#2859
tgodzik added a commit to tgodzik/metals that referenced this issue Sep 13, 2023
Previously, we would read the classpath to create the metals view, which worked well for Scala 2, but needed a reimplementation for Scala 3. So now, instead of doing this reimplementation, which would probably need to use tasty and might be quite complex, I decided to base it on the same approach we use for inexing.

This makes the feature more reliable since, we now used the same well tested functionaties as things like go to definition.

Unfortunately, we might index the particular files the second time, because the basic indexing caches are reversed and finding all symbols in a file from those maps was much slower than just doing it the second time. We later cache that information the same way we did in the previous tree view implementation. So we won't use more memory and running the benchmarks show that the new approach is similarily fast:

```
[info] Benchmark                  Mode  Cnt    Score    Error  Units
[info] ClasspathSymbolsBench.run    ss   10  542.090 ± 61.953  ms/op
```

as compared to the previous:
```
[info] Benchmark                  Mode  Cnt    Score    Error  Units
[info] ClasspathSymbolsBench.run    ss   10  477.283 ± 93.933  ms/op
```

Also it seems we index much more information, since mtags index all symbols, which would also explain the slight difference in timings as I think the difference was 1300 symbols previously and 17000 after this change (number of symbols detected in the benchmarks)

Fixes scalameta#2859
tgodzik added a commit to tgodzik/metals that referenced this issue Oct 24, 2023
Previously, we would read the classpath to create the metals view, which worked well for Scala 2, but needed a reimplementation for Scala 3. So now, instead of doing this reimplementation, which would probably need to use tasty and might be quite complex, I decided to base it on the same approach we use for inexing.

This makes the feature more reliable since, we now used the same well tested functionaties as things like go to definition.

Unfortunately, we might index the particular files the second time, because the basic indexing caches are reversed and finding all symbols in a file from those maps was much slower than just doing it the second time. We later cache that information the same way we did in the previous tree view implementation. So we won't use more memory and running the benchmarks show that the new approach is similarily fast:

```
[info] Benchmark                  Mode  Cnt    Score    Error  Units
[info] ClasspathSymbolsBench.run    ss   10  542.090 ± 61.953  ms/op
```

as compared to the previous:
```
[info] Benchmark                  Mode  Cnt    Score    Error  Units
[info] ClasspathSymbolsBench.run    ss   10  477.283 ± 93.933  ms/op
```

Also it seems we index much more information, since mtags index all symbols, which would also explain the slight difference in timings as I think the difference was 1300 symbols previously and 17000 after this change (number of symbols detected in the benchmarks)

Fixes scalameta#2859
tgodzik added a commit to tgodzik/metals that referenced this issue Oct 26, 2023
Previously, we would read the classpath to create the metals view, which worked well for Scala 2, but needed a reimplementation for Scala 3. So now, instead of doing this reimplementation, which would probably need to use tasty and might be quite complex, I decided to base it on the same approach we use for inexing.

This makes the feature more reliable since, we now used the same well tested functionaties as things like go to definition.

Unfortunately, we might index the particular files the second time, because the basic indexing caches are reversed and finding all symbols in a file from those maps was much slower than just doing it the second time. We later cache that information the same way we did in the previous tree view implementation. So we won't use more memory and running the benchmarks show that the new approach is similarily fast:

```
[info] Benchmark                  Mode  Cnt    Score    Error  Units
[info] ClasspathSymbolsBench.run    ss   10  542.090 ± 61.953  ms/op
```

as compared to the previous:
```
[info] Benchmark                  Mode  Cnt    Score    Error  Units
[info] ClasspathSymbolsBench.run    ss   10  477.283 ± 93.933  ms/op
```

Also it seems we index much more information, since mtags index all symbols, which would also explain the slight difference in timings as I think the difference was 1300 symbols previously and 17000 after this change (number of symbols detected in the benchmarks)

Fixes scalameta#2859
tgodzik added a commit that referenced this issue Oct 27, 2023
Previously, we would read the classpath to create the metals view, which worked well for Scala 2, but needed a reimplementation for Scala 3. So now, instead of doing this reimplementation, which would probably need to use tasty and might be quite complex, I decided to base it on the same approach we use for inexing.

This makes the feature more reliable since, we now used the same well tested functionaties as things like go to definition.

Unfortunately, we might index the particular files the second time, because the basic indexing caches are reversed and finding all symbols in a file from those maps was much slower than just doing it the second time. We later cache that information the same way we did in the previous tree view implementation. So we won't use more memory and running the benchmarks show that the new approach is similarily fast:

```
[info] Benchmark                  Mode  Cnt    Score    Error  Units
[info] ClasspathSymbolsBench.run    ss   10  542.090 ± 61.953  ms/op
```

as compared to the previous:
```
[info] Benchmark                  Mode  Cnt    Score    Error  Units
[info] ClasspathSymbolsBench.run    ss   10  477.283 ± 93.933  ms/op
```

Also it seems we index much more information, since mtags index all symbols, which would also explain the slight difference in timings as I think the difference was 1300 symbols previously and 17000 after this change (number of symbols detected in the benchmarks)

Fixes #2859
tgodzik added a commit that referenced this issue Nov 8, 2023
Previously, we would read the classpath to create the metals view, which worked well for Scala 2, but needed a reimplementation for Scala 3. So now, instead of doing this reimplementation, which would probably need to use tasty and might be quite complex, I decided to base it on the same approach we use for inexing.

This makes the feature more reliable since, we now used the same well tested functionaties as things like go to definition.

Unfortunately, we might index the particular files the second time, because the basic indexing caches are reversed and finding all symbols in a file from those maps was much slower than just doing it the second time. We later cache that information the same way we did in the previous tree view implementation. So we won't use more memory and running the benchmarks show that the new approach is similarily fast:

```
[info] Benchmark                  Mode  Cnt    Score    Error  Units
[info] ClasspathSymbolsBench.run    ss   10  542.090 ± 61.953  ms/op
```

as compared to the previous:
```
[info] Benchmark                  Mode  Cnt    Score    Error  Units
[info] ClasspathSymbolsBench.run    ss   10  477.283 ± 93.933  ms/op
```

Also it seems we index much more information, since mtags index all symbols, which would also explain the slight difference in timings as I think the difference was 1300 symbols previously and 17000 after this change (number of symbols detected in the benchmarks)

Fixes #2859
@tgodzik tgodzik added this to the Metals v1.1.1 milestone Dec 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Scala 3 Generic ticket relating to Scala 3 TVP Tickets related to the Tree View Protocol
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants