Merge pull request #14698 from som-snytt/issue/14664

Clean up SourceFile creation and error handling

Author: smarter

compiler/src/dotty/tools/dotc/CompilationUnit.scala

@@ -99,7 +99,7 @@ object CompilationUnit {
   /** Make a compilation unit for top class `clsd` with the contents of the `unpickled` tree */
   def apply(clsd: ClassDenotation, unpickled: Tree, forceTrees: Boolean)(using Context): CompilationUnit =
     val file = clsd.symbol.associatedFile.nn
-    apply(new SourceFile(file, Array.empty[Char]), unpickled, forceTrees)
+    apply(SourceFile(file, Array.empty[Char]), unpickled, forceTrees)
 
   /** Make a compilation unit, given picked bytes and unpickled tree */
   def apply(source: SourceFile, unpickled: Tree, forceTrees: Boolean)(using Context): CompilationUnit = {

compiler/src/dotty/tools/dotc/Run.scala

@@ -307,12 +307,9 @@ class Run(comp: Compiler, ictx: Context) extends ImplicitRunInfo with Constraint
   def compileFromStrings(scalaSources: List[String], javaSources: List[String] = Nil): Unit = {
     def sourceFile(source: String, isJava: Boolean): SourceFile = {
       val uuid = java.util.UUID.randomUUID().toString
-      val ext = if (isJava) ".java" else ".scala"
-      val virtualFile = new VirtualFile(s"compileFromString-$uuid.$ext")
-      val writer = new BufferedWriter(new OutputStreamWriter(virtualFile.output, StandardCharsets.UTF_8.nn.name)) // buffering is still advised by javadoc
-      writer.write(source)
-      writer.close()
-      new SourceFile(virtualFile, Codec.UTF8)
+      val ext = if (isJava) "java" else "scala"
+      val name = s"compileFromString-$uuid.$ext"
+      SourceFile.virtual(name, source)
     }
     val sources =
       scalaSources.map(sourceFile(_, isJava = false)) ++

compiler/src/dotty/tools/dotc/interactive/InteractiveDriver.scala

@@ -297,14 +297,8 @@ class InteractiveDriver(val settings: List[String]) extends Driver {
     cleanupTree(tree)
   }
 
-  private def toSource(uri: URI, sourceCode: String): SourceFile = {
-    val path = Paths.get(uri)
-    val virtualFile = new VirtualFile(path.getFileName.toString, path.toString)
-    val writer = new BufferedWriter(new OutputStreamWriter(virtualFile.output, StandardCharsets.UTF_8.name))
-    writer.write(sourceCode)
-    writer.close()
-    new SourceFile(virtualFile, Codec.UTF8)
-  }
+  private def toSource(uri: URI, sourceCode: String): SourceFile =
+    SourceFile.virtual(Paths.get(uri).toString, sourceCode)
 
   /**
    * Initialize this driver and compiler.

compiler/src/dotty/tools/dotc/util/SourceFile.scala

@@ -13,9 +13,10 @@ import Chars._
 import scala.annotation.internal.sharable
 import scala.collection.mutable
 import scala.collection.mutable.ArrayBuffer
+import scala.util.chaining.given
 
-import java.io.IOException
 import java.nio.charset.StandardCharsets
+import java.nio.file.{FileSystemException, NoSuchFileException}
 import java.util.Optional
 import java.util.concurrent.atomic.AtomicInteger
 import java.util.regex.Pattern
@@ -71,15 +72,6 @@ class SourceFile(val file: AbstractFile, computeContent: => Array[Char]) extends
 
   def maybeIncomplete: Boolean = _maybeInComplete
 
-  def this(file: AbstractFile, codec: Codec) =
-    // It would be cleaner to check if the file exists instead of catching
-    // an exception, but it turns out that Files.exists is remarkably slow,
-    // at least on Java 8 (https://rules.sonarsource.com/java/tag/performance/RSPEC-3725),
-    // this is significant enough to show up in our benchmarks.
-    this(file,
-      try new String(file.toByteArray, codec.charSet).toCharArray
-      catch case _: java.nio.file.NoSuchFileException => Array[Char]())
-
   /** Tab increment; can be overridden */
   def tabInc: Int = 8
 
@@ -226,9 +218,8 @@ object SourceFile {
   implicit def fromContext(using Context): SourceFile = ctx.source
 
   def virtual(name: String, content: String, maybeIncomplete: Boolean = false) =
-    val src = new SourceFile(new VirtualFile(name, content.getBytes(StandardCharsets.UTF_8)), scala.io.Codec.UTF8)
-    src._maybeInComplete = maybeIncomplete
-    src
+    SourceFile(new VirtualFile(name, content.getBytes(StandardCharsets.UTF_8)), content.toCharArray)
+      .tap(_._maybeInComplete = maybeIncomplete)
 
   /** Returns the relative path of `source` within the `reference` path
    *
@@ -273,17 +264,20 @@ object SourceFile {
     ScriptSourceFile.hasScriptHeader(content)
 
   def apply(file: AbstractFile | Null, codec: Codec): SourceFile =
-    // see note above re: Files.exists is remarkably slow
+    // Files.exists is slow on Java 8 (https://rules.sonarsource.com/java/tag/performance/RSPEC-3725),
+    // so cope with failure; also deal with path prefix "Not a directory".
     val chars =
-    try
-      new String(file.toByteArray, codec.charSet).toCharArray
-    catch
-      case _: java.nio.file.NoSuchFileException => Array[Char]()
+      try new String(file.toByteArray, codec.charSet).toCharArray
+      catch
+        case _: NoSuchFileException => Array.empty[Char]
+        case fse: FileSystemException if fse.getMessage.endsWith("Not a directory") => Array.empty[Char]
 
     if isScript(file, chars) then
       ScriptSourceFile(file, chars)
     else
-      new SourceFile(file, chars)
+      SourceFile(file, chars)
+
+  def apply(file: AbstractFile | Null, computeContent: => Array[Char]): SourceFile = new SourceFile(file, computeContent)
 }
 
 @sharable object NoSource extends SourceFile(NoAbstractFile, Array[Char]()) {

compiler/src/dotty/tools/io/VirtualFile.scala

@@ -28,15 +28,15 @@ class VirtualFile(val name: String, override val path: String) extends AbstractF
   def this(name: String) = this(name, name)
 
   /**
-    * Initializes this instance with the specified name and an
-    * identical path.
+    * Initializes this instance with the specified path
+    * and a name taken from the last path element.
     *
-    * @param name the name of the virtual file to be created
+    * @param path the path of the virtual file to be created
     * @param content the initial contents of the virtual file
     * @return     the created virtual file
     */
-  def this(name: String, content: Array[Byte]) = {
-    this(name)
+  def this(path: String, content: Array[Byte]) = {
+    this(VirtualFile.nameOf(path), path)
     this.content = content
   }
 
@@ -104,3 +104,7 @@ class VirtualFile(val name: String, override val path: String) extends AbstractF
    */
   def lookupNameUnchecked(name: String, directory: Boolean): AbstractFile = unsupported()
 }
+object VirtualFile:
+  private def nameOf(path: String): String =
+    val i = path.lastIndexOf('/')
+    if i >= 0 && i < path.length - 1 then path.substring(i + 1) else path

compiler/test/dotty/tools/dotc/parsing/ParserTest.scala

@@ -21,7 +21,7 @@ class ParserTest extends DottyTest {
     parsedTrees.clear()
   }
 
-  def parse(file: PlainFile): Tree = parseSource(new SourceFile(file, Codec.UTF8))
+  def parse(file: PlainFile): Tree = parseSource(SourceFile(file, Codec.UTF8))
 
   private def parseSource(source: SourceFile): Tree = {
     //println("***** parsing " + source.file)

compiler/test/dotty/tools/dotc/parsing/ScannerTest.scala

@@ -19,7 +19,7 @@ class ScannerTest extends DottyTest {
 
   def scan(file: PlainFile): Unit = {
     //println("***** scanning " + file)
-    val source = new SourceFile(file, Codec.UTF8)
+    val source = SourceFile(file, Codec.UTF8)
     val scanner = new Scanner(source)
     var i = 0
     while (scanner.token != EOF) {

language-server/test/dotty/tools/languageserver/util/CodeRange.scala

@@ -3,7 +3,7 @@ package dotty.tools.languageserver.util
 import dotty.tools.languageserver.util.embedded.{CodeInRange, CodeMarker}
 import dotty.tools.languageserver.util.server.TestFile
 
-import org.eclipse.lsp4j._
+import org.eclipse.lsp4j.{Location, Range, SymbolKind}
 
 import PositionContext._
 

scaladoc/src/dotty/tools/scaladoc/DocContext.scala

@@ -42,8 +42,7 @@ def throwableToString(t: Throwable)(using CompilerContext): String =
 
 private def sourcePostionFor(f: File)(using CompilerContext) =
     val relPath = relativePath(f.toPath)
-    val virtualFile = new VirtualFile(relPath.toString, relPath.toString)
-    val sourceFile = new SourceFile(virtualFile, Codec.UTF8)
+    val sourceFile = SourceFile.virtual(relPath.toString, content = "")
     SourcePosition(sourceFile, Spans.NoSpan)
 
 // TODO (https://github.com/lampepfl/scala3doc/issues/238): provide proper error handling