Outline compiler #6114

kasiaMarek · 2024-02-13T12:54:42Z

connected to: #917

tgodzik

Thanks for working on this!

Let's also add a user setting to enable the new logic.

tgodzik · 2024-03-10T15:32:51Z

mtags-interfaces/src/main/java/scala/meta/pc/CompilerFiles.java

@@ -0,0 +1,6 @@
+package scala.meta.pc;
+
+public interface CompilerFiles {


Let's add some docs why it's needed.

The alternative is to not add new method, but send changedFiles for every file if the project doesn't compile

I moved the files to outline logic outside of pc. I think it increases readability of that logic.

mtags/src/main/scala-2.12/scala/meta/internal/pc/Compat.scala

tgodzik · 2024-03-10T15:37:14Z

mtags/src/main/scala-2/scala/meta/internal/pc/MetalsGlobal.scala

+  val richCompilationCache: TrieMap[String, RichCompilationUnit] =
+    TrieMap.empty[String, RichCompilationUnit]
+
+  // for those paths units were


We should add a bit more to the comment here

kasiaMarek · 2024-03-19T10:31:29Z

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

+
+  def successfulCompilation(): Unit = {
+    wasSuccessfullyCompiled.set(true)
+    changedDocuments.clear()


Note: that this doesn't account for possible changes before compilation end is reported, it may not 100% reflect the state of files that was during compilation.

mtags-interfaces/src/main/java/scala/meta/pc/OutlineFiles.java

kasiaMarek · 2024-03-19T10:36:30Z

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

+          .asJava
+      Some(
+        OutlineFiles(
+          allFiles,


We may want to cap the number of files for initial compilation, so it won't take too long.

We can do a high number like 300 or something

kasiaMarek · 2024-03-20T13:20:19Z

Failing Scala3 NIGHTLY test is a test-only problem, so it shouldn't block this PR.

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

mtags/src/main/scala-2.12/scala/meta/internal/pc/Compat.scala

mtags/src/main/scala-2.13.8/meta/internal/pc/Compat.scala

tests/unit/src/test/scala/tests/CompilersLspSuite.scala

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

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

tgodzik · 2024-04-17T16:13:49Z

mtags/src/main/scala-2/scala/meta/internal/pc/MetalsGlobal.scala

+      else false
+    }
+
+    // TODO the same in autoimports TEST


Do we check auto imports? Is the TODO still relevant?

I added tests, this seems to be already handled

…ers`

tgodzik

Some minor comments, but I also tested it on Metals codebase this workflow seems to break:

Add non compiling method to the MD5 object in mtagsShared
Change the name of MD5 to MD55
Try to auto import MD55 in another package.
We get completions for MD5 aside from MD55 and something like MD55.<local>

Could you take a look if you can reproduce it?

tgodzik · 2024-04-22T14:52:17Z

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

@@ -101,6 +104,10 @@ class Compilers(

  import compilerConfiguration._

+  val plugins = new CompilerPlugins()


Suggested change

val plugins = new CompilerPlugins()

I don't think it's used here now.

tgodzik · 2024-04-22T14:53:42Z

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

-              s"no build target found for $path. Using presentation compiler with project's scala-library version: ${scalaVersion}"
-            )
-          Some(fallbackCompiler)
+        case None => Some(fallbackCompiler)


Why remove this?

tgodzik · 2024-04-22T14:55:35Z

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

+    } provider.didChange(path)
+  }
+
+  def getOutlineFiles(id: String): Optional[JOutlineFiles] =


What is this id? Should we use BuildTargetIdentifier here? Or at least add an alias.

It is BuildTargetIdentifier, I'm constructing it here.

def getOutlineFiles(id: String): Optional[JOutlineFiles] = getOutlineFiles(buildTargetId(id)) def getOutlineFiles( buildTargetId: Option[BuildTargetIdentifier] )

It's just that it comes as a string from pc (can't be BuildTargetIdentifier, because pc interface doesn't depend on bsp4j) and it's less boilerplate to map it here instead of each place in Compilers.

kasiaMarek force-pushed the outline-compiler branch from dd965c4 to e5982ab Compare February 13, 2024 14:18

kasiaMarek requested a review from tgodzik February 27, 2024 14:44

tgodzik requested changes Mar 10, 2024

View reviewed changes

kasiaMarek force-pushed the outline-compiler branch from 34ac473 to f3967a7 Compare March 19, 2024 10:12

kasiaMarek commented Mar 19, 2024

View reviewed changes

mtags-interfaces/src/main/java/scala/meta/pc/OutlineFiles.java Show resolved Hide resolved

kasiaMarek commented Mar 19, 2024

View reviewed changes

kasiaMarek force-pushed the outline-compiler branch 2 times, most recently from 6675382 to d1060a8 Compare March 20, 2024 08:03

kasiaMarek requested a review from tgodzik March 26, 2024 16:15

tgodzik reviewed Apr 16, 2024

View reviewed changes

kasiaMarek force-pushed the outline-compiler branch from 8c58a4c to 5cd84e8 Compare April 17, 2024 12:29

tgodzik reviewed Apr 17, 2024

View reviewed changes

kasiaMarek force-pushed the outline-compiler branch from 5cd84e8 to 56d4ce5 Compare April 19, 2024 09:33

kasiaMarek marked this pull request as ready for review April 19, 2024 11:39

kasiaMarek requested a review from tgodzik April 19, 2024 11:52

tgodzik and others added 13 commits April 23, 2024 11:27

feature: Experiment with outline compielr

50e41c5

more outline compiler progress

aea58cc

add new files for outline

5bd7cf5

adjust for all Scala 2 versions

8cbc943

don't outline before first compilation is done

4b057cb

outline twice for first compile

4b98e67

test fixes

cefe63b

restrict when presentation compiler is restarted

40181f6

test: add new test

a414023

move logic outline files logic out of pc

cfa3f76

move outline from Compat + move semanticdbTextDocument to `Compil…

5c6f4d2

…ers`

add buildTargetId to pc

8095f31

cap numer of files to outline

2c1ab8d

kasiaMarek added 4 commits April 23, 2024 11:27

add tests

aac5df7

format

fb3a720

reverse moving outline from Compat

8c60944

formatting fix

1499749

kasiaMarek force-pushed the outline-compiler branch from dd3c21e to 1499749 Compare April 23, 2024 09:29

tgodzik reviewed Apr 25, 2024

View reviewed changes

review fixes

b8e8fc4

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Outline compiler #6114

Outline compiler #6114

kasiaMarek commented Feb 13, 2024 •

edited

tgodzik left a comment

tgodzik Mar 10, 2024

kasiaMarek Mar 19, 2024

tgodzik Mar 10, 2024

kasiaMarek Mar 19, 2024

kasiaMarek Mar 19, 2024

tgodzik Apr 17, 2024

kasiaMarek commented Mar 20, 2024

tgodzik Apr 17, 2024

kasiaMarek Apr 23, 2024

tgodzik left a comment

tgodzik Apr 22, 2024

tgodzik Apr 22, 2024

tgodzik Apr 22, 2024

kasiaMarek Apr 26, 2024

		@@ -0,0 +1,6 @@
		package scala.meta.pc;

		public interface CompilerFiles {

		@@ -101,6 +104,10 @@ class Compilers(

		import compilerConfiguration._

		val plugins = new CompilerPlugins()

Outline compiler #6114

Are you sure you want to change the base?

Outline compiler #6114

Conversation

kasiaMarek commented Feb 13, 2024 • edited

tgodzik left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kasiaMarek commented Mar 20, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tgodzik left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kasiaMarek commented Feb 13, 2024 •

edited