Skip to content

Commit

Permalink
bugfix: Auto imports in worksheets in Scala 3
Browse files Browse the repository at this point in the history
  • Loading branch information
kasiaMarek committed Feb 29, 2024
1 parent 929714c commit 55df3f3
Show file tree
Hide file tree
Showing 3 changed files with 108 additions and 12 deletions.
40 changes: 28 additions & 12 deletions presentation-compiler/src/main/dotty/tools/pc/AutoImports.scala
Original file line number Diff line number Diff line change
Expand Up @@ -302,11 +302,24 @@ object AutoImports:
}.headOption
case _ => None

def skipUsingDirectivesOffset =
def firstMemberDefinitionStart(tree: Tree)(using Context): Option[Int] =
tree match
case PackageDef(_, stats) =>
stats.flatMap {
case s: PackageDef => firstMemberDefinitionStart(s)
case stat if stat.span.exists => Some(stat.span.start)
case _ => None
}.headOption
case _ => None


def skipUsingDirectivesOffset(
firstObjectPos: Int = firstMemberDefinitionStart(tree).getOrElse(0)
): Int =
val firstObjectLine = pos.source.offsetToLine(firstObjectPos)
comments
.takeWhile(comment =>
!comment.isDocComment && comment.span.end < firstObjectBody(tree)
.fold(0)(_.span.start)
!comment.isDocComment && pos.source.offsetToLine(comment.span.end) + 1 < firstObjectLine
)
.lastOption
.fold(0)(_.span.end + 1)
Expand All @@ -318,7 +331,7 @@ object AutoImports:
val (lineNumber, padTop) = lastImportStatement match
case Some(stm) => (stm.endPos.line + 1, false)
case None if pkg.pid.symbol.isEmptyPackage =>
(pos.source.offsetToLine(skipUsingDirectivesOffset), false)
(pos.source.offsetToLine(skipUsingDirectivesOffset()), false)
case None =>
val pos = pkg.pid.endPos
val line =
Expand All @@ -330,7 +343,7 @@ object AutoImports:
new AutoImportPosition(offset, text, padTop)
}

def forScript(isAmmonite: Boolean): Option[AutoImportPosition] =
def forScript(path: String): Option[AutoImportPosition] =
firstObjectBody(tree).map { tmpl =>
val lastImportStatement =
tmpl.body.takeWhile(_.isInstanceOf[Import]).lastOption
Expand All @@ -340,10 +353,11 @@ object AutoImports:
offset
case None =>
val scriptOffset =
if isAmmonite then
ScriptFirstImportPosition.ammoniteScStartOffset(text, comments)
else
ScriptFirstImportPosition.scalaCliScStartOffset(text, comments)
if path.isAmmoniteGeneratedFile
then ScriptFirstImportPosition.ammoniteScStartOffset(text, comments)
else if path.isScalaCLIGeneratedFile
then ScriptFirstImportPosition.scalaCliScStartOffset(text, comments)
else Some(skipUsingDirectivesOffset(tmpl.span.start))

scriptOffset.getOrElse {
val tmplPoint = tmpl.self.srcPos.span.point
Expand All @@ -359,14 +373,16 @@ object AutoImports:

def fileStart =
AutoImportPosition(
skipUsingDirectivesOffset,
skipUsingDirectivesOffset(),
0,
padTop = false
)

val scriptPos =
if path.isAmmoniteGeneratedFile then forScript(isAmmonite = true)
else if path.isScalaCLIGeneratedFile then forScript(isAmmonite = false)
if path.isAmmoniteGeneratedFile ||
path.isScalaCLIGeneratedFile ||
path.isWorksheet
then forScript(path)
else None

scriptPos
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,18 @@ trait BaseAutoImportsSuite extends BaseCodeActionSuite:
selection
)

def checkWorksheetEdit(
original: String,
expected: String,
selection: Int = 0
): Unit =
checkEditSelection(
"example.worksheet.sc",
original,
expected,
selection
)

def checkEditSelection(
filename: String,
original: String,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -342,6 +342,69 @@ class AutoImportsSuite extends BaseAutoImportsSuite:
)
)

@Test def `worksheet-import` =
checkWorksheetEdit(
worksheetPcWrapper(
"""|//> using scala 3.3.0
|
|// Some comment
|
|// Object comment
|object A {
| val p: <<Path>> = ???
|}
|""".stripMargin
),
worksheetPcWrapper(
"""|//> using scala 3.3.0
|
|// Some comment
|import java.nio.file.Path
|
|// Object comment
|object A {
| val p: Path = ???
|}
|""".stripMargin
)
)

@Test def `object-import` =
checkEdit(
"""|object A {
| //some comment
| val p: <<Path>> = ???
|}
|""".stripMargin,
"""|import java.nio.file.Path
|object A {
| //some comment
| val p: Path = ???
|}
|""".stripMargin,
)

@Test def `toplevels-import` =
checkEdit(
"""|//some comment
|
|val p: <<Path>> = ???
|
|//some other comment
|
|val v = 1
|""".stripMargin,
"""|//some comment
|import java.nio.file.Path
|
|val p: Path = ???
|
|//some other comment
|
|val v = 1
|""".stripMargin,
)

private def ammoniteWrapper(code: String): String =
// Vaguely looks like a scala file that Ammonite generates
// from a sc file.
Expand All @@ -359,6 +422,11 @@ class AutoImportsSuite extends BaseAutoImportsSuite:
|}
|""".stripMargin

private def worksheetPcWrapper(code: String): String =
s"""|object worksheet{
|$code
|}""".stripMargin

// https://dotty.epfl.ch/docs/internals/syntax.html#soft-keywords
@Test
def `soft-keyword-check-test` =
Expand Down

0 comments on commit 55df3f3

Please sign in to comment.