Change requests from reviews

Author: pikinier20

dist/bin/scaladoc

@@ -127,6 +127,7 @@ eval "\"$JAVACMD\"" \
      ${JAVA_OPTS:-$default_java_opts} \
      "${java_args[@]}" \
      "${jvm_cp_args-}" \
+      -Dscala.usejavacp=true \
      "dotty.tools.scaladoc.Main"  \
      "${scala_args[@]}" \
      "${residual_args[@]}" \

project/Build.scala

@@ -1243,7 +1243,7 @@ object Build {
       libraryDependencies += ("org.scala-js" %%% "scalajs-dom" % "1.1.0").cross(CrossVersion.for3Use2_13)
     )
 
-  def generateDocumentation(targets: Seq[String], name: String, outDir: String, ref: String, params: Seq[String] = Nil, addBootclasspath: Boolean = false) =
+  def generateDocumentation(targets: Seq[String], name: String, outDir: String, ref: String, params: Seq[String] = Nil, usingScript: Boolean = true) =
     Def.taskDyn {
       val distLocation = (dist / pack).value
       val projectVersion = version.value
@@ -1255,13 +1255,6 @@ object Build {
       def srcManaged(v: String, s: String) = s"out/bootstrap/stdlib-bootstrapped/scala-$v/src_managed/main/$s-library-src"
       def scalaSrcLink(v: String, s: String) = s"-source-links:$s=github://scala/scala/v$v#src/library"
       def dottySrcLink(v: String, s: String) = s"-source-links:$s=github://lampepfl/dotty/$v#library/src"
-      def bootclasspath: Seq[String] = if(addBootclasspath) Seq(
-        "-bootclasspath",
-        Seq(
-            scalaLib,
-            dottyLib
-        ).mkString(System.getProperty("path.separator"))
-      ) else Nil
 
       val revision = Seq("-revision", ref, "-project-version", projectVersion)
       val cmd = Seq(
@@ -1272,9 +1265,22 @@ object Build {
         scalaSrcLink(stdLibVersion, srcManaged(dottyNonBootstrappedVersion, "scala")),
         dottySrcLink(referenceVersion, srcManaged(dottyNonBootstrappedVersion, "dotty")),
         s"-source-links:github://lampepfl/dotty/$referenceVersion",
-      ) ++ scalacOptionsDocSettings ++ revision ++ params ++ targets ++ bootclasspath
+      ) ++ scalacOptionsDocSettings ++ revision ++ params ++ targets
       import _root_.scala.sys.process._
-      Def.task((s"$distLocation/bin/scaladoc" +: cmd).!)
+      if (usingScript)
+        Def.task((s"$distLocation/bin/scaladoc" +: cmd).!)
+      else {
+        val escapedCmd = cmd.map(arg => if(arg.contains(" ")) s""""$arg"""" else arg)
+        Def.task {
+          try {
+            (Compile / run).toTask(escapedCmd.mkString(" ", " ", "")).value
+            0
+          } catch {
+            case _ : Throwable => 1
+          }
+        }
+      }
+
     }
 
   val SourceLinksIntegrationTest = config("sourceLinksIntegrationTest") extend Test
@@ -1321,8 +1327,7 @@ object Build {
       generateSelfDocumentation := Def.taskDyn {
         generateDocumentation(
           (Compile / classDirectory).value.getAbsolutePath :: Nil,
-          "scaladoc", "scaladoc/output/self", VersionUtil.gitHash,
-          addBootclasspath = true
+          "scaladoc", "scaladoc/output/self", VersionUtil.gitHash
         )
       }.value,
       generateScalaDocumentation := Def.inputTaskDyn {
@@ -1363,7 +1368,7 @@ object Build {
             s"-source-links:docs=github://lampepfl/dotty/master#docs",
             "-doc-root-content", docRootFile.toString,
             "-Ydocument-synthetic-types"
-          )
+          ), usingScript = false
         ))
       }.evaluated,
 
@@ -1372,8 +1377,8 @@ object Build {
           (Test / Build.testcasesOutputDir).value,
           "scaladoc testcases",
           "scaladoc/output/testcases",
-          "master",
-          addBootclasspath = true)
+          "master"
+        )
       }.value,
 
       Test / buildInfoKeys := Seq[BuildInfoKey](

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

@@ -76,7 +76,7 @@ case class DocContext(args: Scaladoc.Args, compilerContext: CompilerContext):
 
   lazy val snippetCompilerArgs = snippets.SnippetCompilerArgs.load(args.snippetCompiler, args.snippetCompilerDebug)(using compilerContext)
 
-  lazy val snippetChecker = snippets.SnippetChecker(args.classpath, args.bootclasspath, args.tastyFiles, compilerContext.settings.scalajs.value(using compilerContext))
+  lazy val snippetChecker = snippets.SnippetChecker(args)(using compilerContext)
 
   lazy val staticSiteContext = args.docsRoot.map(path => StaticSiteContext(
       File(path).getAbsoluteFile(),

scaladoc/src/dotty/tools/scaladoc/ScaladocSettings.scala

@@ -19,7 +19,7 @@ import dotty.tools.dotc.core.Contexts._
 class ScaladocSettings extends SettingGroup with AllScalaSettings:
   val unsupportedSettings = Seq(
     // Options that we like to support
-    extdirs, javabootclasspath, encoding, usejavacp,
+    extdirs, javabootclasspath, encoding,
     // Needed for plugin architecture
     plugin,disable,require, pluginsDir, pluginOptions,
     // we need support for sourcepath and sourceroot
@@ -100,7 +100,7 @@ class ScaladocSettings extends SettingGroup with AllScalaSettings:
     MultiStringSetting("-snippet-compiler", "snippet-compiler", snippets.SnippetCompilerArgs.usage)
 
   val snippetCompilerDebug: Setting[Boolean] =
-    BooleanSetting("-snippet-compiler-debug", snippets.SnippetCompilerArgs.debugUsage, false)
+    BooleanSetting("-Ysnippet-compiler-debug", snippets.SnippetCompilerArgs.debugUsage, false)
 
   def scaladocSpecificSettings: Set[Setting[_]] =
     Set(sourceLinks, syntax, revision, externalDocumentationMappings, socialLinks, skipById, skipByRegex, deprecatedSkipPackages, docRootContent, snippetCompiler, snippetCompilerDebug)

scaladoc/src/dotty/tools/scaladoc/snippets/SnippetChecker.scala

@@ -7,21 +7,28 @@ import java.io.File
 
 import dotty.tools.io.AbstractFile
 import dotty.tools.dotc.fromtasty.TastyFileUtil
+import dotty.tools.dotc.config.Settings._
+import dotty.tools.dotc.config.ScalaSettings
 
-class SnippetChecker(val classpath: String, val bootclasspath: String, val tastyFiles: Seq[File], isScalajs: Boolean):
+class SnippetChecker(val args: Scaladoc.Args)(using cctx: CompilerContext):
+
+// (val classpath: String, val bootclasspath: String, val tastyFiles: Seq[File], isScalajs: Boolean, useJavaCp: Boolean):
   private val sep = System.getProperty("path.separator")
-  private val cp = List(
-    tastyFiles
+
+  private val fullClasspath = List(
+    args.tastyFiles
       .map(_.getAbsolutePath())
       .map(AbstractFile.getFile(_))
       .flatMap(t => try { TastyFileUtil.getClassPath(t) } catch { case e: AssertionError => Seq() })
       .distinct.mkString(sep),
-    classpath,
-    bootclasspath
+    args.classpath
   ).mkString(sep)
 
+  private val snippetCompilerSettings: Seq[SnippetCompilerSetting[_]] = cctx.settings.userSetSettings(cctx.settingsState).filter(_ != cctx.settings.classpath).map( s =>
+    SnippetCompilerSetting(s, s.valueIn(cctx.settingsState))
+  ) :+ SnippetCompilerSetting(cctx.settings.classpath, fullClasspath)
 
-  private val compiler: SnippetCompiler = SnippetCompiler(classpath = cp, scalacOptions = if isScalajs then "-scalajs" else "")
+  private val compiler: SnippetCompiler = SnippetCompiler(snippetCompilerSettings = snippetCompilerSettings)
 
   // These constants were found empirically to make snippet compiler
   // report errors in the same position as main compiler.

scaladoc/src/dotty/tools/scaladoc/snippets/SnippetCompiler.scala

@@ -2,10 +2,9 @@ package dotty.tools.scaladoc
 package snippets
 
 import dotty.tools.io.{AbstractFile, VirtualDirectory}
-import dotty.tools.dotc.interactive.InteractiveDriver
-import dotty.tools.dotc.interactive.Interactive
-import dotty.tools.dotc.interactive.InteractiveCompiler
+import dotty.tools.dotc.Driver
 import dotty.tools.dotc.core.Contexts.Context
+import dotty.tools.dotc.core.Mode
 import dotty.tools.dotc.config.Settings.Setting._
 import dotty.tools.dotc.interfaces.SourcePosition
 import dotty.tools.dotc.ast.Trees.Tree
@@ -21,22 +20,27 @@ import dotty.tools.dotc.interfaces.Diagnostic._
 import scala.util.{ Try, Success, Failure }
 
 class SnippetCompiler(
-  classpath: String,
-  val scalacOptions: String = "",
+  val snippetCompilerSettings: Seq[SnippetCompilerSetting[_]],
   target: AbstractFile = new VirtualDirectory("(memory)")
 ):
+  object SnippetDriver extends Driver:
+    val currentCtx =
+      val rootCtx = initCtx.fresh.addMode(Mode.ReadPositions).addMode(Mode.Interactive)
+      rootCtx.setSetting(rootCtx.settings.YretainTrees, true)
+      rootCtx.setSetting(rootCtx.settings.YcookComments, true)
+      rootCtx.setSetting(rootCtx.settings.YreadComments, true)
+      rootCtx.setSetting(rootCtx.settings.color, "never")
+      rootCtx.setSetting(rootCtx.settings.XimportSuggestionTimeout, 0)
 
-  private def newDriver: InteractiveDriver = {
-    val defaultFlags =
-      List("-color:never", "-unchecked", "-deprecation", "-Ximport-suggestion-timeout", "0")
-    val options = scalacOptions.split("\\s+").toList
-    val settings =
-      options ::: defaultFlags ::: "-classpath" :: classpath :: Nil
-
-    new InteractiveDriver(settings)
-  }
-
-  private val driver = newDriver
+      val ctx = setup(Array(""), rootCtx) match
+        case Some((_, ctx)) =>
+          ctx
+        case None => rootCtx
+      val res = snippetCompilerSettings.foldLeft(ctx.fresh) { (ctx, setting) =>
+        ctx.setSetting(setting.setting, setting.value)
+      }
+      res.initialize()(using res)
+      res
 
   private val scala3Compiler = new Compiler
 
@@ -86,9 +90,9 @@ class SnippetCompiler(
     wrappedSnippet: WrappedSnippet,
     arg: SnippetCompilerArg
   ): SnippetCompilationResult = {
-    val context = driver.currentCtx.fresh
+    val context = SnippetDriver.currentCtx.fresh
       .setSetting(
-        driver.currentCtx.settings.outputDir,
+        SnippetDriver.currentCtx.settings.outputDir,
         target
       )
       .setReporter(new StoreReporter)

scaladoc/src/dotty/tools/scaladoc/snippets/SnippetCompilerSetting.scala

@@ -0,0 +1,8 @@
+package dotty.tools.scaladoc
+package snippets
+
+import dotty.tools.scaladoc.DocContext
+import dotty.tools.dotc.config.Settings._
+import dotty.tools.dotc.config.ScalaSettings
+
+case class SnippetCompilerSetting[T](setting: Setting[T], value: T)

scaladoc/src/dotty/tools/scaladoc/tasty/comments/markdown/SnippetRenderer.scala

@@ -18,7 +18,7 @@ object SnippetRenderer:
     case MessageLevel.Error => "snippet-error"
     case MessageLevel.Debug => "snippet-debug"
 
-  private def cutBetwenSymbols[A](
+  private def cutBetweenSymbols[A](
     startSymbol: String,
     endSymbol: String,
     snippetLines: Seq[SnippetLine]
@@ -33,7 +33,7 @@ object SnippetRenderer:
     } yield f(begin, mid, end)
 
   private def wrapHiddenSymbols(snippetLines: Seq[SnippetLine]): Seq[SnippetLine] =
-    val mRes = cutBetwenSymbols("//{", "//}", snippetLines) {
+    val mRes = cutBetweenSymbols("//{", "//}", snippetLines) {
       case (begin, mid, end) =>
         begin ++ mid.drop(1).dropRight(1).map(_.withClass("hideable")) ++ wrapHiddenSymbols(end)
     }
@@ -74,7 +74,7 @@ object SnippetRenderer:
     }
 
   private def wrapMultiLineComments(snippetLines: Seq[SnippetLine]): Seq[SnippetLine] =
-    val mRes = cutBetwenSymbols("/*", "*/", snippetLines) {
+    val mRes = cutBetweenSymbols("/*", "*/", snippetLines) {
       case (begin, mid, end) if mid.size == 1 =>
         val midRedacted = mid.map(wrapLineInBetween(Some("/*"), Some("*/"), _))
         begin ++ midRedacted ++ end

scaladoc/test/dotty/tools/scaladoc/BaseHtmlTest.scala

@@ -34,8 +34,7 @@ class BaseHtmlTest:
             tastyFiles = pcks.flatMap(tastyFiles(_)),
             output = dest.toFile,
             docsRoot = docsRoot,
-            projectVersion = Some(projectVersion),
-            bootclasspath = dotty.tools.dotc.util.ClasspathFromClassloader(classOf[Predef$].getClassLoader())
+            projectVersion = Some(projectVersion)
           )
         Scaladoc.run(args)(using testContext)
         op(using ProjectContext(dest))

scaladoc/test/dotty/tools/scaladoc/snippets/SnippetCompilerTest.scala

@@ -6,7 +6,9 @@ import org.junit.Assert._
 import dotty.tools.io.{AbstractFile, VirtualDirectory}
 
 class SnippetCompilerTest {
-  val compiler = SnippetCompiler(System.getProperty("java.class.path"))
+  val compiler = SnippetCompiler(
+    Seq(SnippetCompilerSetting(testContext.settings.usejavacp, true))
+  )
   def wrapFn: String => WrappedSnippet = (str: String) => WrappedSnippet(
     str,
     Some("test"),