Merge pull request #7494 from adpi2/sbt2-todo-refactorings

[2.x] Fix `SbtRefactorings` and run `SessionSettingsSpec`

buildfile/src/main/scala/sbt/internal/parser/SbtParser.scala

@@ -38,6 +38,7 @@ import dotty.tools.dotc.config.Printers
 private[sbt] object SbtParser:
   val END_OF_LINE_CHAR = '\n'
   val END_OF_LINE = String.valueOf(END_OF_LINE_CHAR)
+  val WRAPPER_POSITION_OFFSET = 25 // size of the wrapper object
   private[parser] val NOT_FOUND_INDEX = -1
   private[sbt] val FAKE_FILE = VirtualFileRef.of("fake")
   private[parser] val XML_ERROR = "';' expected but 'val' found."
@@ -263,16 +264,9 @@ private[sbt] case class SbtParser(path: VirtualFileRef, lines: Seq[String])
 
     def convertStatement(tree: Tree)(using Context): Option[(String, Tree, LineRange)] =
       if tree.span.exists then
-        val statement = String(tree.sourcePos.linesSlice).trim
-        val lines = tree.sourcePos.lines
-        val wrapperLineOffset = 0
-        Some(
-          (
-            statement,
-            tree,
-            LineRange(lines.start + wrapperLineOffset, lines.end + wrapperLineOffset)
-          )
-        )
+        val pos = tree.sourcePos
+        val statement = String(pos.source.content.slice(pos.start, pos.end)).trim
+        Some((statement, tree, LineRange(pos.lines.start, pos.lines.end)))
       else None
     val stmtTreeLineRange = statements.flatMap(convertStatement)
     val importsLineRange = importsToLineRanges(sourceFile, imports)
@@ -290,12 +284,10 @@ private[sbt] case class SbtParser(path: VirtualFileRef, lines: Seq[String])
   private def importsToLineRanges(
       sourceFile: SourceFile,
       imports: Seq[Tree]
-  )(using context: Context): Seq[(String, Int)] =
+  )(using Context): Seq[(String, Int)] =
     imports.map { tree =>
-      // not sure why I need to reconstruct the position myself
-      val pos = SourcePosition(sourceFile, tree.span)
-      val content = String(pos.linesSlice).trim()
-      val wrapperLineOffset = 0
-      (content, pos.line + wrapperLineOffset)
+      val pos = tree.sourcePos
+      val content = String(pos.source.content.slice(pos.start, pos.end)).trim
+      (content, tree.sourcePos.line)
     }
 end SbtParser

buildfile/src/main/scala/sbt/internal/parser/SbtRefactorings.scala

@@ -19,31 +19,29 @@ private[sbt] object SbtRefactorings:
 
   /** A session setting is simply a tuple of a Setting[_] and the strings which define it. */
   type SessionSetting = (Def.Setting[_], Seq[String])
-  type SbtConfigFile = (File, Seq[String])
   val emptyString = ""
   val reverseOrderingInt = Ordering[Int].reverse
 
   /**
    * Refactoring a `.sbt` file so that the new settings are used instead of any existing settings.
-   * @param configFile SbtConfigFile with the lines of an sbt file as a List[String] where each string is one line
+   * @param lines the lines of an sbt file as a List[String] where each string is one line
    * @param commands A List of settings (space separate) that should be inserted into the current file.
    *                 If the settings replaces a value, it will replace the original line in the .sbt file.
    *                 If in the `.sbt` file we have multiply value for one settings -
    *                 the first will be replaced and the other will be removed.
-   * @return a SbtConfigFile with new lines which represent the contents of the refactored .sbt file.
+   * @return the new lines which represent the content of the refactored .sbt file.
    */
   def applySessionSettings(
-      configFile: SbtConfigFile,
+      lines: Seq[String],
       commands: Seq[SessionSetting]
-  ): SbtConfigFile = {
-    val (file, lines) = configFile
+  ): Seq[String] = {
     val split = SbtParser(FAKE_FILE, lines)
     given ctx: Context = SbtParser.defaultGlobalForParser.compileCtx
     val recordedCommands = recordCommands(commands, split)
     val sortedRecordedCommands = recordedCommands.sortBy(_._1)(reverseOrderingInt)
 
     val newContent = replaceFromBottomToTop(lines.mkString(END_OF_LINE), sortedRecordedCommands)
-    (file, newContent.linesIterator.toList)
+    newContent.linesIterator.toList
   }
 
   private def replaceFromBottomToTop(
@@ -78,7 +76,8 @@ private[sbt] object SbtRefactorings:
         val replacement =
           if (acc.isEmpty) command.mkString(END_OF_LINE)
           else emptyString
-        (tree.sourcePos.start, st, replacement) +: acc
+        val pos = tree.sourcePos.start - SbtParser.WRAPPER_POSITION_OFFSET
+        (pos, st, replacement) +: acc
       } else {
         acc
       }
@@ -93,14 +92,9 @@ private[sbt] object SbtRefactorings:
     seq.toMap
   }
 
-  // todo: revisit
-  private def extractSettingName(tree: untpd.Tree): String =
-    tree.toString()
-    // tree.children match {
-    //   case h :: _ =>
-    //     extractSettingName(h)
-    //   case _ =>
-    //     tree.toString()
-    // }
+  private def extractSettingName(tree: untpd.Tree): String = tree match
+    case untpd.Ident(name)        => name.toString
+    case untpd.InfixOp(lhs, _, _) => extractSettingName(lhs)
+    case other                    => other.toString
 
 end SbtRefactorings

buildfile/src/test/scala/sbt/internal/parser/SessionSettingsSpec.scala

@@ -9,71 +9,64 @@ package sbt
 package internal
 package parser
 
-/*
-import java.io.{ File, FilenameFilter }
+import sbt.internal.inc.PlainVirtualFileConverter
+import sbt.internal.parser.SbtRefactorings.SessionSetting
 
+import java.io.File
+import java.io.FilenameFilter
+import java.nio.file.Files
+import java.nio.file.Path
+import java.nio.file.Paths
 import scala.io.Source
-import SessionSettings.SessionSetting
+import scala.jdk.CollectionConverters.*
 
 abstract class AbstractSessionSettingsSpec(folder: String) extends AbstractSpec {
-  protected val rootPath = getClass.getResource("/" + folder).getPath
-  println(s"Reading files from: $rootPath")
-  protected val rootDir = new File(rootPath)
+  private val rootDir = Paths.get(getClass.getResource("/" + folder).toURI)
+  println(s"Reading files from: $rootDir")
+  private val converter = PlainVirtualFileConverter.converter
 
-  test("SessionSettings should be identical for empty map") {
-    def unit(f: File) = Seq((Source.fromFile(f).getLines().toList, Seq()))
-    runTestOnFiles(unit)
+  testOnFiles("should be identical for empty map") { f =>
+    Seq((readLines(f), Seq()))
   }
 
-  test("it should replace statements") {
-    runTestOnFiles(replace)
-  }
-
-  private def runTestOnFiles(
-      expectedResultAndMap: File => Seq[(List[String], Seq[SessionSetting])]
-  ): Unit = {
-
-    val allFiles = rootDir
-      .listFiles(new FilenameFilter() {
-        def accept(dir: File, name: String) = name.endsWith(".sbt.txt")
-      })
-      .toList
-    allFiles foreach { file =>
-      val originalLines = Source.fromFile(file).getLines().toList
-      expectedResultAndMap(file) foreach { case (expectedResultList, commands) =>
-        val resultList = SbtRefactorings.applySessionSettings((file, originalLines), commands)
-        val expected = SbtParser(file, expectedResultList)
-        val result = SbtParser(file, resultList._2)
-        assert(result.settings == expected.settings)
-      }
-    }
-  }
-
-  protected def replace(f: File) = {
-    val dirs = rootDir
-      .listFiles(new FilenameFilter() {
-        def accept(dir: File, name: String) = {
-          val startsWith = f.getName + "_"
-          name.startsWith(startsWith)
-        }
-      })
-      .toSeq
+  testOnFiles("should replace statements") { f =>
+    val dirs = listFiles(rootDir)(_.startsWith(s"${f.getFileName}_"))
     dirs.flatMap { dir =>
-      val files = dir.listFiles(new FilenameFilter {
-        override def accept(dir: File, name: String) = name.endsWith(".set")
-      })
+      val files = listFiles(dir)(_.endsWith(".set"))
       files.map { file =>
-        val seq = Source.fromFile(file).getLines().toSeq
-        val result = Source.fromFile(file.getAbsolutePath + ".result").getLines().toList
+        val seq = readLines(file)
+        val result = readLines(Paths.get(s"$file.result"))
         val sessionSettings = seq.map(line => (null, Seq(line)))
         (result, sessionSettings)
       }
     }
   }
 
+  private def testOnFiles(name: String)(
+      expectedResultAndMap: Path => Seq[(Seq[String], Seq[SessionSetting])]
+  ): Unit = test(name) {
+
+    val allFiles = listFiles(rootDir)(_.endsWith(".sbt.txt"))
+
+    allFiles.foreach { file =>
+      val originalLines = readLines(file)
+      val virtualFile = converter.toVirtualFile(file)
+      expectedResultAndMap(file).foreach { case (expectedResultList, commands) =>
+        val resultList = SbtRefactorings.applySessionSettings(originalLines, commands)
+        val expected = SbtParser(virtualFile, expectedResultList)
+        val result = SbtParser(virtualFile, resultList)
+        assert(result.settings == expected.settings)
+      }
+    }
+  }
+
+  private def listFiles(dir: Path)(filter: String => Boolean): Seq[Path] =
+    Files.walk(dir).iterator.asScala.filter(f => filter(f.getFileName.toString)).toSeq
+
+  private def readLines(file: Path): Seq[String] =
+    Files.readAllLines(file).asScala.toList
 }
 
-class SessionSettingsSpec extends AbstractSessionSettingsSpec("session-settings")
+object SessionSettingsSpec extends AbstractSessionSettingsSpec("session-settings")
 
-class SessionSettingsQuickSpec extends AbstractSessionSettingsSpec("session-settings-quick")
- */
+object SessionSettingsQuickSpec extends AbstractSessionSettingsSpec("session-settings-quick")

internal/util-logging/src/main/scala/sbt/internal/util/codec/PositionFormats.scala

@@ -6,7 +6,8 @@
  */
 
 package sbt.internal.util.codec
-import _root_.sjsonnew.{ deserializationError, Builder, JsonFormat, Unbuilder }
+
+import sjsonnew.{ deserializationError, Builder, JsonFormat, Unbuilder }
 import xsbti.Position
 import java.util.Optional
 

main/src/main/scala/sbt/internal/SessionSettings.scala

@@ -107,8 +107,6 @@ object SessionSettings:
   // (Setting[_], Seq[String])
 
   type SessionMap = Map[ProjectRef, Seq[SessionSetting]]
-  type SbtConfigFile = sbt.internal.parser.SbtRefactorings.SbtConfigFile
-  // (File, Seq[String])
 
   /**
    * This will re-evaluate all Setting[_]'s on this session against the current build state and
@@ -246,7 +244,7 @@ object SessionSettings:
     }
     val newSettings = settings diff replace
     val oldContent = IO.readLines(writeTo)
-    val (_, exist) = SbtRefactorings.applySessionSettings((writeTo, oldContent), replace)
+    val exist = SbtRefactorings.applySessionSettings(oldContent, replace)
     val adjusted = if (newSettings.nonEmpty && needsTrailingBlank(exist)) exist :+ "" else exist
     val lines = adjusted ++ newSettings.flatMap(x => x._2 :+ "")
     IO.writeLines(writeTo, lines)

util-collection/src/test/scala/TupleMapExtensionTest.scala

@@ -1,13 +1,12 @@
 package sbt.internal.util
 
 import verify.BasicTestSuite
-import sbt.internal.util.TupleMapExtension.*
 
 object TupleMapExtensionTest extends BasicTestSuite:
   val tuple: Tuple.Map[(Int, String), Option] = ((Option(1), Option("foo")))
 
   test("tuple.mapN") {
     val f = (arg: (Int, String)) => arg._1.toString + "|" + arg._2
-    val actual = tuple.mapN[String](f)
+    val actual = TupleMapExtension.mapN[(Int, String), Option](tuple)(f)
     assert(actual == Option("1|foo"))
   }