NIT Misc scaladocs & refactors (#3106)

* NIT: add scaladoc for PersistentDiagnosticLogger

* Remove ExecutorService from BspClient, since it was used for extracting line offset from generated sources, with the current implementation we don't read the files so no need for more threads.
Original code's purpose can be seen in the commit that introduces it: 4297b07.

* Nit: Add scaladoc for GeneratedSource

* Nit: Simplify code in HasGeneratedSources

* Apply scalafix

* Bring back the needed line that updates the copy of the diagnostic

Author: MaciejG604

modules/build/src/main/scala/scala/build/GeneratedSource.scala

@@ -2,6 +2,20 @@ package scala.build
 
 import scala.build.internal.WrapperParams
 
+/** Represents a source that's not originally in the user's workspace, yet it's a part of the
+  * project. It can either be synthetically generated by Scala CLI, e.g. BuildInfo or just modified,
+  * e.g. script wrappers
+  *
+  * @param generated
+  *   path to the file created by Scala CLI
+  * @param reportingPath
+  *   the origin of the source:
+  *   - Left(String): there's no path that corresponds to the source it may be a snippet or a gist
+  *     etc.
+  *   - Right(os.Path): this source has been generated based on a file at this path
+  * @param wrapperParamsOpt
+  *   if the generated source is a script wrapper then the params are present here
+  */
 final case class GeneratedSource(
   generated: os.Path,
   reportingPath: Either[String, os.Path],

modules/build/src/main/scala/scala/build/PersistentDiagnosticLogger.scala

@@ -9,6 +9,7 @@ import scala.build.errors.{BuildException, Diagnostic}
 import scala.build.internals.FeatureType
 import scala.scalanative.build as sn
 
+/** Used to collect and send diagnostics to the build client when operating as a BSP */
 class PersistentDiagnosticLogger(parent: Logger) extends Logger {
   private val diagBuilder = List.newBuilder[Diagnostic]
 

modules/build/src/main/scala/scala/build/bsp/BspClient.scala

@@ -18,7 +18,6 @@ import scala.build.{BloopBuildClient, GeneratedSource, Logger}
 import scala.jdk.CollectionConverters.*
 
 class BspClient(
-  readFilesEs: ExecutorService,
   @volatile var logger: Logger,
   var forwardToOpt: Option[b.BuildClient] = None
 ) extends b.BuildClient with BuildClientForwardStubs with BloopBuildClient
@@ -27,76 +26,68 @@ class BspClient(
   private def updatedPublishDiagnosticsParams(
     params: b.PublishDiagnosticsParams,
     genSource: GeneratedSource
-  ): Either[() => b.PublishDiagnosticsParams, b.PublishDiagnosticsParams] = {
+  ): b.PublishDiagnosticsParams = {
     val updatedUri = genSource.reportingPath.fold(
       _ => params.getTextDocument.getUri,
       _.toNIO.toUri.toASCIIString
     )
     val updatedDiagnostics =
       if (genSource.wrapperParamsOpt.isEmpty)
-        Right(params.getDiagnostics)
+        params.getDiagnostics
       else
-        Left { () =>
-          val updateLine = scalaLine => scalaLineToScLine(scalaLine, genSource.wrapperParamsOpt)
-          params.getDiagnostics.asScala.toSeq
-            .map { diag =>
-              val updatedDiagOpt = for {
-                startLine <- updateLine(diag.getRange.getStart.getLine)
-                endLine   <- updateLine(diag.getRange.getEnd.getLine)
-              } yield {
-                val diag0 = diag.duplicate()
-                diag0.getRange.getStart.setLine(startLine)
-                diag0.getRange.getEnd.setLine(endLine)
-
-                val scalaDiagnostic = new Gson().fromJson[b.ScalaDiagnostic](
-                  diag0.getData().asInstanceOf[JsonElement],
-                  classOf[b.ScalaDiagnostic]
-                )
-
-                scalaDiagnostic.getActions().asScala.foreach { action =>
-                  for {
-                    change    <- action.getEdit().getChanges().asScala
-                    startLine <- updateLine(change.getRange.getStart.getLine)
-                    endLine   <- updateLine(change.getRange.getEnd.getLine)
-                  } yield {
-                    change.getRange().getStart.setLine(startLine)
-                    change.getRange().getEnd.setLine(endLine)
-                  }
+        val updateLine = scalaLine => scalaLineToScLine(scalaLine, genSource.wrapperParamsOpt)
+        params.getDiagnostics.asScala.toSeq
+          .map { diag =>
+            val updatedDiagOpt = for {
+              startLine <- updateLine(diag.getRange.getStart.getLine)
+              endLine   <- updateLine(diag.getRange.getEnd.getLine)
+            } yield {
+              val diag0 = diag.duplicate()
+              diag0.getRange.getStart.setLine(startLine)
+              diag0.getRange.getEnd.setLine(endLine)
+
+              val scalaDiagnostic = new Gson().fromJson[b.ScalaDiagnostic](
+                diag0.getData().asInstanceOf[JsonElement],
+                classOf[b.ScalaDiagnostic]
+              )
+
+              scalaDiagnostic.getActions().asScala.foreach { action =>
+                for {
+                  change    <- action.getEdit().getChanges().asScala
+                  startLine <- updateLine(change.getRange.getStart.getLine)
+                  endLine   <- updateLine(change.getRange.getEnd.getLine)
+                } yield {
+                  change.getRange().getStart.setLine(startLine)
+                  change.getRange().getEnd.setLine(endLine)
                 }
+              }
 
-                diag0.setData(scalaDiagnostic)
+              diag0.setData(scalaDiagnostic)
 
-                if (
-                  diag0.getMessage.contains(
-                    "cannot be a main method since it cannot be accessed statically"
-                  )
+              if (
+                diag0.getMessage.contains(
+                  "cannot be a main method since it cannot be accessed statically"
+                )
+              )
+                diag0.setMessage(
+                  WarningMessages.mainAnnotationNotSupported( /* annotationIgnored */ false)
                 )
-                  diag0.setMessage(
-                    WarningMessages.mainAnnotationNotSupported( /* annotationIgnored */ false)
-                  )
 
-                diag0
-              }
-              updatedDiagOpt.getOrElse(diag)
+              diag0
             }
-            .asJava
-        }
-    def updatedParamsFor(
-      updatedDiagnostics: java.util.List[b.Diagnostic]
-    ): b.PublishDiagnosticsParams = {
-      val updatedTextDoc = new b.TextDocumentIdentifier(updatedUri)
-      val updatedParams = new b.PublishDiagnosticsParams(
-        updatedTextDoc,
-        params.getBuildTarget,
-        updatedDiagnostics,
-        params.getReset
-      )
-      updatedParams.setOriginId(params.getOriginId)
-      updatedParams
-    }
-    updatedDiagnostics
-      .left.map(f => () => updatedParamsFor(f()))
-      .map(updatedParamsFor(_))
+            updatedDiagOpt.getOrElse(diag)
+          }
+          .asJava
+
+    val updatedTextDoc = new b.TextDocumentIdentifier(updatedUri)
+    val updatedParams = new b.PublishDiagnosticsParams(
+      updatedTextDoc,
+      params.getBuildTarget,
+      updatedDiagnostics,
+      params.getReset
+    )
+    updatedParams.setOriginId(params.getOriginId)
+    updatedParams
   }
 
   override def onBuildPublishDiagnostics(params: b.PublishDiagnosticsParams): Unit = {
@@ -116,24 +107,11 @@ class BspClient(
         }
     }
 
-    def call(updatedParams0: b.PublishDiagnosticsParams): Unit =
-      super.onBuildPublishDiagnostics(updatedParams0)
     updatedParamsOpt match {
       case None =>
-        call(params)
-      case Some(Right(updatedParams0)) =>
-        call(updatedParams0)
-      case Some(Left(updateParamsFunc)) =>
-        val runnable =
-          new Runnable {
-            def run(): Unit =
-              try call(updateParamsFunc())
-              catch {
-                case t: Throwable =>
-                  logger.debug(s"Caught $t while publishing updated diagnostics")
-              }
-          }
-        readFilesEs.submit(runnable)
+        super.onBuildPublishDiagnostics(params)
+      case Some(updatedParams) =>
+        super.onBuildPublishDiagnostics(updatedParams)
     }
   }
 

modules/build/src/main/scala/scala/build/bsp/BspImpl.scala

@@ -29,7 +29,7 @@ import scala.concurrent.{ExecutionContext, Future, Promise}
 import scala.jdk.CollectionConverters.*
 import scala.util.{Failure, Success}
 
-/** The implementation for [[Bsp]].
+/** The implementation for [[Bsp]] command.
   *
   * @param argsToInputs
   *   a function transforming terminal args to [[Inputs]]
@@ -53,6 +53,12 @@ final class BspImpl(
 
   import BspImpl.{PreBuildData, PreBuildProject, buildTargetIdToEvent, responseError}
 
+  private val shownGlobalMessages =
+    new java.util.concurrent.ConcurrentHashMap[String, Unit]()
+  private var actualLocalClient: BspClient                     = _
+  private var localClient: b.BuildClient with BloopBuildClient = _
+  private val bloopSession                                     = new BloopSession.Reference
+
   /** Sends the buildTarget/didChange BSP notification to the BSP client, indicating that the build
     * targets defined in the current session have changed.
     *
@@ -257,9 +263,6 @@ final class BspImpl(
           client.resetBuildExceptionDiagnostics(targetId)
     }
 
-  private val shownGlobalMessages =
-    new java.util.concurrent.ConcurrentHashMap[String, Unit]()
-
   private def showGlobalWarningOnce(msg: String): Unit =
     shownGlobalMessages.computeIfAbsent(
       msg,
@@ -378,9 +381,6 @@ final class BspImpl(
     }
   }
 
-  private var actualLocalClient: BspClient                     = _
-  private var localClient: b.BuildClient with BloopBuildClient = _
-
   /** Returns a reference to the [[BspClient]], respecting the given verbosity
     * @param verbosity
     *   verbosity to be passed to the resulting [[BspImpl.LoggingBspClient]]
@@ -450,33 +450,30 @@ final class BspImpl(
     bloopSession0
   }
 
-  private val bloopSession = new BloopSession.Reference
-
   /** The logic for the actual running of the `bsp` command, initializing the BSP connection.
     * @param initialInputs
     *   the initial input sources passed upon initializing the BSP connection (which are subject to
     *   change on subsequent workspace/reload requests)
     */
-  def run(initialInputs: Inputs, initialBspOptions: BspReloadableOptions): Future[Unit] = {
+  override def run(initialInputs: Inputs, initialBspOptions: BspReloadableOptions): Future[Unit] = {
     val logger    = initialBspOptions.logger
     val verbosity = initialBspOptions.verbosity
 
-    actualLocalClient = new BspClient(
-      threads.buildThreads.bloop.jsonrpc, // meh
-      logger
-    )
+    actualLocalClient = new BspClient(logger)
     localClient = getLocalClient(verbosity)
 
     val currentBloopSession = newBloopSession(initialInputs, initialBspOptions)
     bloopSession.update(null, currentBloopSession, "BSP server already initialized")
 
-    val actualLocalServer
-      : b.BuildServer with b.ScalaBuildServer with b.JavaBuildServer with b.JvmBuildServer
-        with ScalaScriptBuildServer with HasGeneratedSources =
-      new BuildServerProxy(
-        () => bloopSession.get().bspServer,
-        () => onReload()
-      )
+    val actualLocalServer: b.BuildServer
+      with b.ScalaBuildServer
+      with b.JavaBuildServer
+      with b.JvmBuildServer
+      with ScalaScriptBuildServer
+      with HasGeneratedSources = new BuildServerProxy(
+      () => bloopSession.get().bspServer,
+      () => onReload()
+    )
 
     val localServer: b.BuildServer with b.ScalaBuildServer with b.JavaBuildServer
       with b.JvmBuildServer with ScalaScriptBuildServer =
@@ -540,9 +537,8 @@ final class BspImpl(
     Future.firstCompletedOf(futures)(es)
   }
 
-  /** Shuts down the current Bloop session
-    */
-  def shutdown(): Unit =
+  /** Shuts down the current Bloop session */
+  override def shutdown(): Unit =
     for (currentBloopSession <- bloopSession.getAndNullify())
       currentBloopSession.dispose()
 

modules/build/src/main/scala/scala/build/bsp/HasGeneratedSources.scala

@@ -24,8 +24,9 @@ object HasGeneratedSources {
     lazy val uriMap: Map[String, GeneratedSource] =
       sources
         .flatMap { g =>
-          g.reportingPath.toOption.toSeq.map { _ =>
-            g.generated.toNIO.toUri.toASCIIString -> g
+          g.reportingPath match {
+            case Left(_)  => Nil
+            case Right(_) => Seq(g.generated.toNIO.toUri.toASCIIString -> g)
           }
         }
         .toMap