Basic support for new `.mill` and `.mill.scala` source files #6752

lolgab · 2024-09-03T14:59:38Z

Mill is migrating from the .sc suffix to .mill to distinguish its files from ammonite and scala-cli script files.
.sc will be still supported in a migration period

PR which introduced the new suffix: com-lihaoyi/mill#3426

Mill is migrating from the `.sc` suffix to `.mill` to distinguish its files from ammonite and scala-cli script files. `.sc` will be still supported in a migration period

tests/cross/src/test/scala/tests/pc/CompletionMillIvySuite.scala

tgodzik · 2024-09-04T16:09:23Z

We might also need to update the Scala syntax extension and Metals vs code extension

lihaoyi · 2024-10-04T12:15:09Z

@lolgab are you going to continue with this? If not I might pick it up and try to get it over the finish line

lolgab · 2024-10-04T12:33:31Z

@lihaoyi You can complete it. Sorry for holding it so long. I was wanting for the extensions discussion to stabilize, but then I forgot about it.

lihaoyi · 2024-10-04T15:23:41Z

@lolgab no worries, I decided to put a bounty on it (com-lihaoyi/mill#3664), in case you or anyone else wants to give it a try. If nobody does I'll take a crack at it myself

lolgab · 2024-10-04T16:05:48Z

@lihaoyi Ok, let me try to finish it.
@tgodzik opened a new PR in metals-vscode: scalameta/metals-vscode#1543

lolgab · 2024-10-04T16:14:19Z

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

@@ -46,7 +46,7 @@ class SemanticdbTextDocumentProvider(

    val explicitDialect = if (filePath.isSbt) {
      Some(dialects.Sbt1)
-    } else if (filePath.isScalaScript) {
+    } else if (filePath.isMill || filePath.isScalaScript) {


Even though we try to consider .mill not scala scripts, they are actually Scala 2.13 files with top level terms, so I believe this should stay.

lolgab · 2024-10-07T20:53:53Z

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

+        // hack for mill build sources until a proper BSP wrapped sources mapping gets into
+        // the official protocol


@tgodzik I managed to recreate MappedSources without BSP. The values seem to be set correctly, but still metals doesn't work correctly...
I think I need some help.

I will take a look!

tgodzik

Do we want to add the source mapping in this PR? I checked and it seems to work worse with it currently even when skipping checking the generated bit.

Maybe it's ok to merge without it for now. Things will work at the current level for Mill, we can improve slowly later.

tgodzik · 2024-10-10T17:09:11Z

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

+          generatedSourcesDirectory <- sources
+            .find(item =>
+              item
+                .getGenerated() && item.getKind() == b.SourceItemKind.DIRECTORY


Looks like it's not actually marked as generated.

Hmm.... with Mill BSP it's actually generated correctly.

tgodzik · 2024-10-10T17:22:17Z

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

+            .iterator()
+            .asScala
+            .collect {
+              case t if t.getName().endsWith("mill-build/") =>


Suggested change

case t if t.getName().endsWith("mill-build/") =>

case t if t.getName().endsWith("mill-build-") =>

weirdly it's this way for Bloop. With Mill BSP it works, but it seems BSP breaks on newest mill :sigh:

This code is not production-ready yet. I was hacking it to make source mapping work, but I will leave it to another PR as you suggested.

tgodzik · 2024-10-10T17:44:38Z

Looks like after the changes the generated code typechecks but sometimes doesn't return completions, not sure why though. Might be worth creating a completion test and investigating, but as I said might not be neccessary for this PR.

tgodzik · 2024-10-10T17:46:37Z

And btw. my experiments are here https://github.com/scalameta/metals/compare/main...tgodzik:metals:support-mill-suffix?expand=1

Things look to work most of the time (only the completions seem to be wrong) and those are pretty big hacks

tgodzik · 2024-10-11T08:07:01Z

metals/src/main/scala/scala/meta/internal/metals/codeactions/MillifyDependencyCodeAction.scala

@@ -24,7 +24,7 @@ class MillifyDependencyCodeAction(buffers: Buffers) extends CodeAction {
    val range = params.getRange()

    val tokenized =
-      if (path.isScalaScript && range.getStart == range.getEnd)
+      if ((path.isScalaScript || path.isMill) && range.getStart == range.getEnd)


I don't think this will work currently with Mill, it's for //> using dep org.something %% name % version to change it.

We could add that option to Mill build files, but it would require a bit of work (not much I think)

tgodzik · 2024-10-11T08:07:43Z

metals/src/main/scala/scala/meta/internal/parsing/Trees.scala

@@ -174,7 +174,7 @@ final class Trees(
      val skipFistShebang =
        if (text.startsWith("#!")) text.replaceFirst("#!", "//") else text
      val input = Input.VirtualFile(path.toURI.toString(), skipFistShebang)
-      if (path.isAmmoniteScript || path.isMill) {
+      if (path.isAmmoniteScript) {


Was this actually wrong? Do mill files have multiple stages like ammonite (don't remember why this was added actually) This actually caused multi-stage tests for ammonite fail, they might be wrong though.

Basic support for new .mill source files

fa05c28

Mill is migrating from the `.sc` suffix to `.mill` to distinguish its files from ammonite and scala-cli script files. `.sc` will be still supported in a migration period

tgodzik requested changes Sep 4, 2024

View reviewed changes

tests/cross/src/test/scala/tests/pc/CompletionMillIvySuite.scala Show resolved Hide resolved

alexarchambault mentioned this pull request Sep 9, 2024

Claim *.mill files as Scala files scala/vscode-scala-syntax#269

Merged

lihaoyi mentioned this pull request Oct 4, 2024

Support for .mill and .mill.scala files in VSCode/Metals (1000USD Bounty) com-lihaoyi/mill#3664

Open

2 tasks

lolgab added 2 commits October 4, 2024 17:53

Add .mill and .mill.scala in more places

7a3af84

Merge remote-tracking branch 'upstream/main' into support-mill-suffix

16b42bf

lolgab mentioned this pull request Oct 4, 2024

Add support for build.mill and build.mill.scala scalameta/metals-vscode#1543

Open

lolgab added 3 commits October 4, 2024 18:08

Add .mill.scala in isMillRelatedPath

69db9a9

Add .mill.scala to isMillRelatedPath

e79206b

Revert change to consider .mill files Scala scripts

2ea4e79

lolgab commented Oct 4, 2024

View reviewed changes

Add build.mill.scala to def millProject in BuildTools.scala

812a43c

lolgab changed the title ~~Basic support for new .mill source files~~ Basic support for new .mill and .mill.scala source files Oct 4, 2024

lolgab added 5 commits October 5, 2024 17:34

Fix suffix and update isScalaOrJavaFilename and isScalaFilename

0cc0a64

Make build.mill tests not use top level definitions for now

c344aa1

New places where we should consider mill files

28df84f

Call addMappedSource for mill build files

56d5c0f

Merge remote-tracking branch 'upstream/main' into support-mill-suffix

cebd2a8

lolgab commented Oct 7, 2024

View reviewed changes

Better script wrapping

da3aa45

tgodzik reviewed Oct 10, 2024

View reviewed changes

Remove hack for source mapping

a6b4f26

Stop considering all mill files Ammonite scripts

6160121

tgodzik requested changes Oct 11, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Basic support for new `.mill` and `.mill.scala` source files #6752

Basic support for new `.mill` and `.mill.scala` source files #6752

lolgab commented Sep 3, 2024 •

edited

Loading

tgodzik commented Sep 4, 2024

lihaoyi commented Oct 4, 2024

lolgab commented Oct 4, 2024

lihaoyi commented Oct 4, 2024

lolgab commented Oct 4, 2024

lolgab Oct 4, 2024

lolgab Oct 7, 2024

tgodzik Oct 8, 2024

tgodzik left a comment

tgodzik Oct 10, 2024

tgodzik Oct 10, 2024

tgodzik Oct 10, 2024

lolgab Oct 10, 2024

tgodzik commented Oct 10, 2024

tgodzik commented Oct 10, 2024

tgodzik Oct 11, 2024

tgodzik Oct 11, 2024

		// hack for mill build sources until a proper BSP wrapped sources mapping gets into
		// the official protocol

	case t if t.getName().endsWith("mill-build/") =>
	case t if t.getName().endsWith("mill-build-") =>

Basic support for new .mill and .mill.scala source files #6752

Are you sure you want to change the base?

Basic support for new .mill and .mill.scala source files #6752

Conversation

lolgab commented Sep 3, 2024 • edited Loading

tgodzik commented Sep 4, 2024

lihaoyi commented Oct 4, 2024

lolgab commented Oct 4, 2024

lihaoyi commented Oct 4, 2024

lolgab commented Oct 4, 2024

Choose a reason for hiding this comment

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

tgodzik commented Oct 10, 2024

tgodzik commented Oct 10, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Basic support for new `.mill` and `.mill.scala` source files #6752

Basic support for new `.mill` and `.mill.scala` source files #6752

lolgab commented Sep 3, 2024 •

edited

Loading