Added docstrigs and comments. Also cleaned unused imports. Changd the printStream connected to the compilation logger sent to the mill evaluator from Bsp from System.out (might interfere with the lsp communication ) to the outputstream of the evaluator's logger.

10 files changed, 266 insertions, 88 deletions

diff --git a/contrib/bsp/src/mill/contrib/MainMillBuildServer.scala b/contrib/bsp/src/mill/contrib/MainMillBuildServer.scala
index af5483a4..aa4cf211 100644
--- a/contrib/bsp/src/mill/contrib/MainMillBuildServer.scala
+++ b/contrib/bsp/src/mill/contrib/MainMillBuildServer.scala
@@ -3,14 +3,12 @@ package mill.contrib
 import play.api.libs.json._
 import java.nio.file.FileAlreadyExistsException
 import java.util.concurrent.Executors
-
 import upickle.default._
 import ch.epfl.scala.bsp4j._
 import mill._
 import mill.define.{Command, Discover, ExternalModule}
 import mill.eval.Evaluator
 import org.eclipse.lsp4j.jsonrpc.Launcher
-
 import scala.collection.JavaConverters._
 
 
@@ -23,6 +21,7 @@ object BSP extends ExternalModule {
   val bspVersion = "2.0.0"
   val languages = List("scala", "java")
 
+  // computes the path to the java executable
   def whichJava: String = {
     if (scala.sys.props.contains("JAVA_HOME")) scala.sys.props("JAVA_HOME") else "java"
   }
diff --git a/contrib/bsp/src/mill/contrib/bsp/BspLoggedReporter.scala b/contrib/bsp/src/mill/contrib/bsp/BspLoggedReporter.scala
index 06c45b79..a90b02a5 100644
--- a/contrib/bsp/src/mill/contrib/bsp/BspLoggedReporter.scala
+++ b/contrib/bsp/src/mill/contrib/bsp/BspLoggedReporter.scala
@@ -2,19 +2,35 @@ package mill.contrib.bsp
 
 import java.io.File
 import java.util.concurrent.atomic.AtomicInteger
-import java.util.concurrent.{ConcurrentHashMap, ConcurrentMap}
-
+import java.util.concurrent.ConcurrentHashMap
 import ch.epfl.scala.bsp4j._
 import ch.epfl.scala.{bsp4j => bsp}
 import sbt.internal.inc.ManagedLoggedReporter
 import sbt.internal.util.ManagedLogger
 import xsbti.{Problem, Severity}
-
 import scala.collection.JavaConverters._
 import scala.collection.concurrent
 import scala.compat.java8.OptionConverters._
-import scala.io.Source
 
+
+/**
+  * Specialized reporter that sends compilation diagnostics
+  * for each problem it logs, either as information, warning or
+  * error as well as task finish notifications of type `compile-report`.
+  *
+  * @param client the client to send diagnostics to
+  * @param targetId the target id of the target whose compilation
+  *                 the diagnostics are related to
+  * @param taskId   a unique id of the compilation task of the target
+  *                 specified by `targetId`
+  * @param compilationOriginId optional origin id the client assigned to
+  *                            the compilation request. Needs to be sent
+  *                            back as part of the published diagnostics
+  *                            as well as compile report
+  * @param maxErrors The maximum number of errors to be logged during the
+  *                  compilation of targetId
+  * @param logger    The logger that will log the messages for each Problem.
+  */
 class BspLoggedReporter(client: bsp.BuildClient,
                         targetId: BuildTargetIdentifier,
                         taskId: TaskId,
@@ -60,10 +76,14 @@ class BspLoggedReporter(client: bsp.BuildClient,
     client.onBuildTaskFinish(taskFinishParams)
   }
 
+  // Obtains the parameters for sending diagnostics about the given Problem ( as well as
+  // about all previous problems generated for the same text file ) related to the specified
+  // targetId, incorporating the given optional originId ( generated by the client for the
+  // compile request )
   //TODO: document that if the problem is a general information without a text document
   // associated to it, then the document field of the diagnostic is set to the uri of the target
-  def getDiagnostics(problem: Problem, targetId: bsp.BuildTargetIdentifier, originId: Option[String]):
-                                                                                bsp.PublishDiagnosticsParams = {
+  private[this] def getDiagnostics(problem: Problem, targetId: bsp.BuildTargetIdentifier, originId: Option[String]):
+                                                                                    bsp.PublishDiagnosticsParams = {
       val diagnostic = getSingleDiagnostic(problem)
       val sourceFile = problem.position().sourceFile().asScala
       val textDocument = new TextDocumentIdentifier(
@@ -81,10 +101,14 @@ class BspLoggedReporter(client: bsp.BuildClient,
       params
     }
 
+  // Compute the compilation status code
   private[this] def getStatusCode: StatusCode = {
     if (errors.get > 0) StatusCode.ERROR else StatusCode.OK
   }
 
+  // Update the published diagnostics for the fiven text file by
+  // adding the recently computed diagnostic to the list of
+  // all previous diagnostics generated for the same file.
   private[this] def appendDiagnostics(textDocument: TextDocumentIdentifier,
                                       currentDiagnostic: Diagnostic): List[Diagnostic] = {
     diagnosticMap.putIfAbsent(textDocument, new bsp.PublishDiagnosticsParams(
@@ -94,14 +118,15 @@ class BspLoggedReporter(client: bsp.BuildClient,
     diagnosticMap(textDocument).getDiagnostics.asScala.toList ++ List(currentDiagnostic)
   }
 
+  // Computes the diagnostic related to the given Problem
   private[this] def getSingleDiagnostic(problem: Problem): Diagnostic ={
 
     val start = new bsp.Position(
       problem.position.startLine.asScala.getOrElse(problem.position.line.asScala.getOrElse(0)),
       problem.position.startOffset.asScala.getOrElse(problem.position.offset.asScala.getOrElse(0)))
     val end = new bsp.Position(
-      problem.position.endLine.asScala.getOrElse(problem.position.line.asScala.getOrElse(0)),
-      problem.position.endOffset.asScala.getOrElse(problem.position.offset.asScala.getOrElse(0)))
+      problem.position.endLine.asScala.getOrElse(problem.position.line.asScala.getOrElse(start.getLine)),
+      problem.position.endOffset.asScala.getOrElse(problem.position.offset.asScala.getOrElse(start.getCharacter)))
     val diagnostic = new bsp.Diagnostic(new bsp.Range(start, end), problem.message)
     diagnostic.setCode(problem.position.lineContent)
     diagnostic.setSource("compiler from mill")
diff --git a/contrib/bsp/src/mill/contrib/bsp/BspTestReporter.scala b/contrib/bsp/src/mill/contrib/bsp/BspTestReporter.scala
index 69471675..51a2f2ba 100644
--- a/contrib/bsp/src/mill/contrib/bsp/BspTestReporter.scala
+++ b/contrib/bsp/src/mill/contrib/bsp/BspTestReporter.scala
@@ -1,10 +1,23 @@
 package mill.contrib.bsp
 
 import ch.epfl.scala.bsp4j._
-import mill.api.{BspContext, TestReporter}
+import mill.api.BspContext
 import sbt.testing._
 
 
+/**
+  * Context class for BSP, specialized for sending `task-start` and
+  * `task-finish` notifications for every test being ran.
+  * @param client The client to send notifications to
+  * @param targetId The targetId of the BSP target for which
+  *                 the test request is being processed
+  * @param taskId   The unique taskId associated with the
+  *                 test task that will trigger this reporter
+  *                 to log testing events.
+  * @param arguments compilation arguments as part of the BSP context,
+  *                  in case special arguments need to be passed to
+  *                  the compiler before running the test task.
+  */
 class BspTestReporter(
                        client: BuildClient,
                        targetId: BuildTargetIdentifier,
@@ -76,7 +89,9 @@ class BspTestReporter(
     client.onBuildTaskFinish(taskFinishParams)
   }
 
-  def getDisplayName(e: Event): String = {
+  // Compute the display name of the test / test suite
+  // to which the given event relates
+  private[this] def getDisplayName(e: Event): String = {
     e.selector() match{
       case s: NestedSuiteSelector => s.suiteId()
       case s: NestedTestSelector => s.suiteId() + "." + s.testName()
@@ -86,6 +101,8 @@ class BspTestReporter(
     }
   }
 
+  // Compute the test report data structure that will go into
+  // the task finish notification after all tests are ran.
   def getTestReport: TestReport = {
     val report = new TestReport(targetId, passed, failed, ignored, cancelled, skipped)
     report.setTime(totalTime)
diff --git a/contrib/bsp/src/mill/contrib/bsp/MillBspLogger.scala b/contrib/bsp/src/mill/contrib/bsp/MillBspLogger.scala
index 195d9aab..a17c4a40 100644
--- a/contrib/bsp/src/mill/contrib/bsp/MillBspLogger.scala
+++ b/contrib/bsp/src/mill/contrib/bsp/MillBspLogger.scala
@@ -4,6 +4,19 @@ import ch.epfl.scala.bsp4j._
 import mill.api.Logger
 import mill.util.ProxyLogger
 
+
+/**
+  * BSP-specialized logger class which sends `task-progress`
+  * notifications ( upon the invocation of the `ticker` method ) and
+  * `show-message` notifications ( for each error or information
+  * being logged ).
+  * @param client the client to send notifications to, also the
+  *               client that initiated a request which triggered
+  *               a mill task evaluation
+  * @param taskId unique ID of the task being evaluated
+  * @param logger the logger to which the messages received by this
+  *               MillBspLogger are being redirected
+  */
 class MillBspLogger(client: BuildClient, taskId: Int, logger: Logger) extends ProxyLogger(logger) {
 
   override def ticker(s: String): Unit = {
@@ -17,6 +30,7 @@ class MillBspLogger(client: BuildClient, taskId: Int, logger: Logger) extends Pr
       params.setProgress(progress(0).toLong)
       params.setTotal(progress(1).toLong)
       client.onBuildTaskProgress(params)
+      super.ticker(s)
     } catch {
       case e: Exception =>
     }
diff --git a/contrib/bsp/src/mill/contrib/bsp/MillBuildServer.scala b/contrib/bsp/src/mill/contrib/bsp/MillBuildServer.scala
index f34571e4..77e3ff38 100644
--- a/contrib/bsp/src/mill/contrib/bsp/MillBuildServer.scala
+++ b/contrib/bsp/src/mill/contrib/bsp/MillBuildServer.scala
@@ -1,21 +1,16 @@
 package mill.contrib.bsp
 
-import java.io.File
-
 import sbt.testing._
-import java.util.Collections
 import java.util.concurrent.CompletableFuture
-
 import mill.scalalib.Lib.discoverTests
 import ch.epfl.scala.bsp4j._
 import mill.{scalalib, _}
-import mill.api.{BspContext, Loose, Result, Strict}
+import mill.api.{BspContext, Result, Strict}
 import mill.contrib.bsp.ModuleUtils._
 import mill.eval.Evaluator
 import mill.scalalib._
 import mill.scalalib.api.CompilationResult
 import sbt.internal.inc._
-
 import scala.collection.JavaConverters._
 import mill.modules.Jvm
 import mill.util.Ctx
@@ -24,8 +19,6 @@ import mill.main.MainModule
 import sbt.internal.util.{ConsoleOut, MainAppender, ManagedLogger}
 import sbt.util.LogExchange
 
-import scala.io.Source
-
 
 class MillBuildServer(evaluator: Evaluator,
                       _bspVersion: String,
@@ -98,21 +91,6 @@ class MillBuildServer(evaluator: Evaluator,
         "")
   }
 
-  private[this] def getSourceFiles(sources: Seq[os.Path]): Iterable[os.Path] = {
-    var files = Seq.empty[os.Path]
-
-    for (source <- sources) {
-      if (os.exists(source)) (if (os.isDir(source)) os.walk(source) else Seq(source))
-        .foreach(path => if (os.isFile(path) && List("scala", "java").contains(path.ext) &&
-          !path.last.startsWith(".")) {
-          files ++= Seq(path)
-        })
-    }
-
-    files
-  }
-
-
   override def buildTargetSources(sourcesParams: SourcesParams): CompletableFuture[SourcesResult] = {
 
     def computeSourcesResult: SourcesResult = {
@@ -212,25 +190,10 @@ class MillBuildServer(evaluator: Evaluator,
     handleExceptions[String, ResourcesResult]((in) => getResources, "")
   }
 
-  def getOriginId(params: CompileParams): Option[String] = {
-    try {
-      Option(params.getOriginId)
-    } catch {
-      case e: Exception => Option.empty[String]
-    }
-  }
-
-  def getArguments(params: CompileParams) : Option[Seq[String]] = {
-    try {
-      Option(params.getArguments.asScala)
-    } catch {
-      case e: Exception => Option.empty[Seq[String]]
-    }
-  }
-
-  def getCompilationLogger: ManagedLogger = {
+  // construct the ManagedLogger that will go into the compilation problems reporter
+  private[this] def getCompilationLogger: ManagedLogger = {
       val consoleAppender = MainAppender.defaultScreen(ConsoleOut.printStreamOut(
-        System.out
+        millEvaluator.log.outputStream
       ))
       val l = LogExchange.logger("Hello")
       LogExchange.unbindLoggerAppenders("Hello")
@@ -238,7 +201,9 @@ class MillBuildServer(evaluator: Evaluator,
       l
   }
 
-  def getBspLoggedReporterPool(params: Parameters, taskStartMessage: String => String,
+  // define the function that spawns compilation reporter for each module based on the
+  // module's hash code TODO: find something more reliable than the hash code
+  private[this] def getBspLoggedReporterPool(params: Parameters, taskStartMessage: String => String,
                                              taskStartDataKind: String, taskStartData: BuildTargetIdentifier => Object):
                                                                 Int => Option[ManagedLoggedReporter] = {
     (int: Int) =>
@@ -280,7 +245,7 @@ class MillBuildServer(evaluator: Evaluator,
       )
       val compileResult = new CompileResult(getStatusCode(result))
       compileResult.setOriginId(compileParams.getOriginId)
-      compileResult //TODO: See what form IntelliJ expects data about products of compilation in order to set data field
+      compileResult //TODO: See in what form IntelliJ expects data about products of compilation in order to set data field
     }
     handleExceptions[String, CompileResult]((in) => getCompileResult, "")
   }
@@ -304,6 +269,7 @@ class MillBuildServer(evaluator: Evaluator,
     handleExceptions[String, RunResult]((in) => getRunResult, "")
   }
 
+  // Get the execution status code given the results from Evaluator.evaluate
   private[this] def getStatusCode(results: Evaluator.Results): StatusCode = {
 
     if (results.failing.keyCount > 0) {
@@ -468,7 +434,8 @@ class MillBuildServer(evaluator: Evaluator,
     handleExceptions[String, ScalaMainClassesResult]((in) => getScalaMainClasses, "")
   }
 
-  private[this] def getTestFrameworks(module: TestModule) (implicit ctx: Ctx.Home): Seq[String] = {
+  // Detect and return the test classes contained in the given TestModule
+  private[this] def getTestClasses(module: TestModule) (implicit ctx: Ctx.Home): Seq[String] = {
     val runClasspath = getTaskResult(millEvaluator, module.runClasspath)
     val frameworks = getTaskResult(millEvaluator, module.testFrameworks)
     val compilationResult = getTaskResult(millEvaluator, module.compile)
@@ -496,7 +463,7 @@ class MillBuildServer(evaluator: Evaluator,
       for (targetId <- scalaTestClassesParams.getTargets.asScala) {
         targetIdToModule(targetId) match {
           case module: TestModule =>
-                    items ++= List(new ScalaTestClassesItem(targetId, getTestFrameworks(module).toList.asJava))
+                    items ++= List(new ScalaTestClassesItem(targetId, getTestClasses(module).toList.asJava))
           case module: JavaModule => //TODO: maybe send a notification that this target has no test classes
         }
       }
@@ -505,12 +472,14 @@ class MillBuildServer(evaluator: Evaluator,
     handleExceptions[Ctx.Home, ScalaTestClassesResult]((c) => getScalaTestClasses(c), ctx)
   }
 
+  // Given the mapping from modules to targetIds, construct the mapping from targetIds to modules
   private[this] def targetToModule(moduleToTargetId: Predef.Map[JavaModule, BuildTargetIdentifier]):
                                                       Predef.Map[BuildTargetIdentifier, JavaModule] = {
       moduleToTargetId.keys.map(mod => (moduleToTargetId(mod), mod)).toMap
 
   }
 
+  // Resolve all the mill modules contained in the project
   private[this] def getMillModules(ev: Evaluator): Seq[JavaModule] = {
     ev.rootModule.millInternal.segmentsToModules.values.
       collect {
@@ -518,6 +487,8 @@ class MillBuildServer(evaluator: Evaluator,
       }.toSeq ++ Seq(rootModule)
   }
 
+  // Recompute the modules in the project in case any changes to the build took place
+  // and update all the mappings that depend on this info
   private[this] def recomputeTargets(): Unit = {
     rootModule = ModuleUtils.getRootJavaModule(millEvaluator.rootModule)
     millModules = getMillModules(millEvaluator)
@@ -526,6 +497,11 @@ class MillBuildServer(evaluator: Evaluator,
     moduleToTarget = ModuleUtils.millModulesToBspTargets(millModules, rootModule, evaluator, List("scala", "java"))
   }
 
+  // Given a function that take input of type T and return output of type V,
+  // apply the function on the given inputs and return a completable future of
+  // the result. If the execution of the function raises an Exception, complete
+  // the future exceptionally. Also complete exceptionally if the server was not
+  // yet initialized.
   private[this] def handleExceptions[T, V](serverMethod: T => V, input: T): CompletableFuture[V] = {
     val future = new CompletableFuture[V]()
     initialized match {
diff --git a/contrib/bsp/src/mill/contrib/bsp/ModuleUtils.scala b/contrib/bsp/src/mill/contrib/bsp/ModuleUtils.scala
index c5f203b5..85a808a8 100644
--- a/contrib/bsp/src/mill/contrib/bsp/ModuleUtils.scala
+++ b/contrib/bsp/src/mill/contrib/bsp/ModuleUtils.scala
@@ -1,38 +1,87 @@
 package mill.contrib.bsp
 
-import java.net.URI
-import java.util.Collections
-
 import scala.collection.JavaConverters._
 import ch.epfl.scala.bsp4j._
 import mill.T
 import mill.api.Result.Success
 import mill.api.{Loose, Strict}
-import mill.define.{BaseModule, Ctx, Discover, Module, Segment, Segments, Sources, Target, Task}
+import mill.define.{BaseModule, Ctx, Segment, Segments, Target, Task}
 import mill.eval._
 import mill.eval.Evaluator
 import mill.scalajslib.ScalaJSModule
 import mill.scalalib.api.Util
 import mill.scalanativelib._
-import mill.scalalib.{CrossModuleBase, GenIdea, GenIdeaImpl, JavaModule, ScalaModule, TestModule}
-import mill.util.DummyLogger
+import mill.scalalib.{JavaModule, ScalaModule, TestModule}
 import os.Path
 
-
+/**
+  * Utilities for translating the mill build into
+  * BSP information like BuildTargets and BuildTargetIdentifiers
+  */
 object ModuleUtils {
 
-    def millModulesToBspTargets(modules: Seq[JavaModule],
+  /**
+    * Compute mapping between all the JavaModules contained in the
+    * working directory ( has to be a mill-based project ) and
+    * BSP BuildTargets ( mill modules correspond one-to-one to
+    * bsp build targets ).
+    * @param modules All JavaModules contained in the working
+    *                directory of the mill project
+    * @param rootModule The root module ( corresponding to the root
+    *                   of the mill project )
+    * @param evaluator The mill evaluator that can resolve information
+    *                  about the mill project
+    * @param supportedLanguages the languages supported by the modules
+    *                           of the mill project
+    * @return JavaModule -> BuildTarget mapping
+    */
+  def millModulesToBspTargets(modules: Seq[JavaModule],
                                 rootModule: JavaModule,
                                 evaluator: Evaluator,
                                 supportedLanguages: List[String]): Predef.Map[JavaModule, BuildTarget] = {
 
-      val moduleIdMap = getModuleTargetIdMap(modules, evaluator)
+    val moduleIdMap = getModuleTargetIdMap(modules, evaluator)
 
-      (for ( module <- modules )
-        yield (module, getTarget(rootModule, module, evaluator, moduleIdMap))).toMap
+    (for ( module <- modules )
+      yield (module, getTarget(rootModule, module, evaluator, moduleIdMap))).toMap
 
-    }
+  }
 
+  /**
+    * Compute the BuildTarget associated with the given module,
+    * may or may not be identical to the root of the working
+    * directory ( rootModule )
+    *
+    * @param rootModule  mill JavaModule for the project root
+    * @param module      mill JavaModule to compute the BuildTarget
+    *                    for
+    * @param evaluator   mill Evaluator
+    * @param moduleIdMap mapping from each mill JavaModule
+    *                    contained in the working directory and
+    *                    a BuildTargetIdentifier associated
+    *                    with it.
+    * @return build target for `module`
+    */
+  def getTarget( rootModule: JavaModule,
+                 module: JavaModule,
+                 evaluator: Evaluator,
+                 moduleIdMap: Map[JavaModule, BuildTargetIdentifier]
+               ): BuildTarget = {
+    if (module == rootModule)
+      getRootTarget(module, evaluator, moduleIdMap)
+    else
+      getRegularTarget(module, evaluator, moduleIdMap)
+  }
+
+  /**
+    * Given the BaseModule corresponding to the root
+    * of the working directory, compute a JavaModule that
+    * has the same millSourcePath. Set generated sources
+    * accoridng to the location of the compilation
+    * products
+    * @param rootBaseModule module for the root
+    * @return root JavaModule
+    */
   def getRootJavaModule(rootBaseModule: BaseModule): JavaModule = {
     implicit val ctx: Ctx = rootBaseModule.millOuterCtx
     new JavaModule {
@@ -42,12 +91,24 @@ object ModuleUtils {
 
       def out = T.sources{millSourcePath / "out"}
       def target = T.sources{millSourcePath / "target"}
-      //override def sources: Sources = T.sources{millSourcePath}
       override def generatedSources: Target[Seq[PathRef]] = T.sources{
          out() ++ target()}
     }
   }
 
+  /**
+    * Compute the BuildTarget associated with the root
+    * directory of the mill project being built
+    * @param rootModule the root JavaModule extracted from
+    *                   the build file by a mill evalautor
+    * @param evaluator  mill evaluator that can resolve
+    *                   build information
+    * @param moduleIdMap mapping from each mill JavaModule
+    *                    contained in the working directory and
+    *                    a BuildTargetIdentifier associated
+    *                    with it.
+    * @return root BuildTarget
+    */
   def getRootTarget(
                      rootModule: JavaModule,
                      evaluator: Evaluator,
@@ -70,6 +131,19 @@ object ModuleUtils {
     rootTarget
   }
 
+  /**
+    * Compute the BuildTarget associated with the given mill
+    * JavaModule, which is any module present in the working
+    * directory, but it's not the root module itself.
+    *
+    * @param module      any in-project mill module
+    * @param evaluator   mill evaluator
+    * @param moduleIdMap mapping from each mill JavaModule
+    *                    contained in the working directory and
+    *                    a BuildTargetIdentifier associated
+    *                    with it.
+    * @return inner BuildTarget
+    */
   def getRegularTarget(
                  module: JavaModule,
                  evaluator: Evaluator,
@@ -99,28 +173,19 @@ object ModuleUtils {
     buildTarget
   }
 
-  def getTarget( rootModule: JavaModule,
-                 module: JavaModule,
-                 evaluator: Evaluator,
-                 moduleIdMap: Map[JavaModule, BuildTargetIdentifier]
-               ): BuildTarget = {
-    if (module == rootModule)
-      getRootTarget(module, evaluator, moduleIdMap)
-    else
-      getRegularTarget(module, evaluator, moduleIdMap)
-  }
-
-  def getModuleCapabilities(module: JavaModule, evaluator: Evaluator): BuildTargetCapabilities = {
+  // obtain the capabilities of the given module ( ex: canCompile, canRun, canTest )
+  private[this] def getModuleCapabilities(module: JavaModule, evaluator: Evaluator): BuildTargetCapabilities = {
     val canTest = module match {
-      case module: TestModule => true
+      case _: TestModule => true
       case default => false
     }
 
     new BuildTargetCapabilities(true, canTest, true)
   }
 
+  // Compute the ScalaBuildTarget from information about the given JavaModule.
   //TODO: Fix the data field for JavaModule when the bsp specification is updated
-  def computeBuildTargetData(module: JavaModule, evaluator: Evaluator): ScalaBuildTarget = {
+  private[this] def computeBuildTargetData(module: JavaModule, evaluator: Evaluator): ScalaBuildTarget = {
     module match {
       case m: ScalaModule =>
         val scalaVersion = evaluateInformativeTask(evaluator, m.scalaVersion, "")
@@ -144,10 +209,25 @@ object ModuleUtils {
     }
   }
 
+  /**
+    * Evaluate the given task using the given mill evaluator and return
+    * its result of type Result
+    * @param evaluator mill evalautor
+    * @param task task to evaluate
+    * @tparam T
+    */
   def getTaskResult[T](evaluator: Evaluator, task: Task[T]): Result[Any] = {
     evaluator.evaluate(Strict.Agg(task)).results(task)
   }
 
+  /**
+    * Evaluate the given task using the given mill evaluator and return
+    * its result of type T, or the default value of the evaluation failed.
+    * @param evaluator mill evalautor
+    * @param task task to evaluate
+    * @param defaultValue default value to return in case of failure
+    * @tparam T
+    */
   def evaluateInformativeTask[T](evaluator: Evaluator, task: Task[T], defaultValue: T): T = {
       val evaluated = evaluator.evaluate(Strict.Agg(task)).results(task)
       evaluated match {
@@ -156,7 +236,9 @@ object ModuleUtils {
       }
   }
 
-  def computeScalaLangDependencies(module: ScalaModule, evaluator: Evaluator): Loose.Agg[PathRef] = {
+  // Compute all relevant scala dependencies of `module`, like scala-library, scala-compiler,
+  // and scala-reflect
+  private[this] def computeScalaLangDependencies(module: ScalaModule, evaluator: Evaluator): Loose.Agg[PathRef] = {
     evaluateInformativeTask(evaluator, module.resolveDeps(module.scalaLibraryIvyDeps), Loose.Agg.empty[PathRef]) ++
       evaluateInformativeTask(evaluator, module.scalacPluginClasspath, Loose.Agg.empty[PathRef]) ++
       evaluateInformativeTask(evaluator, module.resolveDeps(module.ivyDeps), Loose.Agg.empty[PathRef]).
@@ -165,7 +247,8 @@ object ModuleUtils {
           pathRef.path.toNIO.toAbsolutePath.toUri.toString.contains("scala-library"))
   }
 
-  def getScalaTargetPlatform(module: ScalaModule): ScalaPlatform = {
+  // Obtain the scala platform for `module`
+  private[this] def getScalaTargetPlatform(module: ScalaModule): ScalaPlatform = {
     module match {
       case m: ScalaNativeModule => ScalaPlatform.NATIVE
       case m: ScalaJSModule => ScalaPlatform.JS
@@ -173,6 +256,13 @@ object ModuleUtils {
     }
   }
 
+  /**
+    * Compute mapping between a mill JavaModule and the BuildTargetIdentifier
+    * associated with its corresponding bsp BuildTarget.
+    * @param modules mill modules inside the project ( including root )
+    * @param evaluator mill evalautor to resolve build information
+    * @return JavaModule -> BuildTargetIdentifier mapping
+    */
   def getModuleTargetIdMap(modules: Seq[JavaModule], evaluator:Evaluator): Predef.Map[JavaModule, BuildTargetIdentifier] = {
 
     (for ( module <- modules )
diff --git a/contrib/bsp/src/mill/contrib/bsp/TaskParameters.scala b/contrib/bsp/src/mill/contrib/bsp/TaskParameters.scala
index b020b498..29a07f17 100644
--- a/contrib/bsp/src/mill/contrib/bsp/TaskParameters.scala
+++ b/contrib/bsp/src/mill/contrib/bsp/TaskParameters.scala
@@ -1,10 +1,15 @@
 package mill.contrib.bsp
-import java.util
 
 import scala.collection.JavaConverters._
-import scala.compat.java8.OptionConverters._
 import ch.epfl.scala.bsp4j.{BuildTargetIdentifier, CompileParams, RunParams, TestParams}
 
+
+/**
+  * Common trait to represent BSP request parameters that
+  * have a specific form: include one or more targetIds,
+  * arguments for the execution of the task, and an optional
+  * origin id generated by the client.
+  */
 trait Parameters {
   def getTargets: List[BuildTargetIdentifier]
 
@@ -83,14 +88,33 @@ case class TParams(testParams: TestParams) extends Parameters {
 }
 
 object TaskParameters {
+
+  /**
+    * Convert parameters specific to the compile request
+    * to the common trait Parameters.
+    * @param compileParams compile request parameters
+    * @return general task parameters containing compilation info
+    */
   def fromCompileParams(compileParams: CompileParams): Parameters = {
     CParams(compileParams)
   }
 
+  /**
+    * Convert parameters specific to the run request
+    * to the common trait Parameters.
+    * @param runParams run request parameters
+    * @return general task parameters containing running info
+    */
   def fromRunParams(runParams: RunParams): Parameters = {
     RParams(runParams)
   }
 
+  /**
+    * Convert parameters specific to the test request
+    * to the common trait Parameters.
+    * @param testParams compile request parameters
+    * @return general task parameters containing testing info
+    */
   def fromTestParams(testParams: TestParams): Parameters = {
     TParams(testParams)
   }
diff --git a/main/api/src/mill/api/BspCompileArguments.scala b/main/api/src/mill/api/BspCompileArguments.scala
index 1af45a61..73586cc8 100644
--- a/main/api/src/mill/api/BspCompileArguments.scala
+++ b/main/api/src/mill/api/BspCompileArguments.scala
@@ -1,8 +1,17 @@
 package mill.api
 
+/**
+  * Data structure to represent Bsp client-specified
+  * compilation arguments
+  */
 class BspCompileArguments {
   var arguments: Seq[String] = Seq.empty[String]
 
+  /**
+    * Return the compilation arguments specified by the
+    * Bsp client, which may or may not be found in the
+    * compiler options of any module from the build file.
+    */
   def args: Seq[String] = {
     arguments
   }
diff --git a/main/api/src/mill/api/BspContext.scala b/main/api/src/mill/api/BspContext.scala
index c93fbca1..1281518d 100644
--- a/main/api/src/mill/api/BspContext.scala
+++ b/main/api/src/mill/api/BspContext.scala
@@ -2,10 +2,24 @@ package mill.api
 
 import sbt.testing.Event
 
+/**
+  * Bsp Context with functionality for retrieving compile
+  * arguments provided by a Bsp client, as well as for logging
+  * the start and finish of a task triggered by the request of
+  * a Bsp client. Can be integrated into mill's Ctx to inject
+  * Bsp functionality into tasks like compile/run/test.
+  */
 trait BspContext extends BspCompileArguments with TestReporter
 
+/**
+  * Dummy Bsp Context that does nothing
+  * upon starting or finishing a task, and
+  * contains no client-specified compilation
+  * arguments
+  */
 object DummyBspContext extends BspContext {
   override def args = Seq.empty[String]
+
   override def logStart(event:  Event): Unit = {
 
   }
diff --git a/main/api/src/mill/api/TestReporter.scala b/main/api/src/mill/api/TestReporter.scala
index b3d0e432..97dec761 100644
--- a/main/api/src/mill/api/TestReporter.scala
+++ b/main/api/src/mill/api/TestReporter.scala
@@ -2,6 +2,12 @@ package mill.api
 
 import sbt.testing._
 
+/**
+  * Test reporter class that can be
+  * injected into the test task and
+  * report information upon the start
+  * and the finish of testing events
+  */
 trait TestReporter {
   def logStart(event: Event): Unit
 
@@ -10,6 +16,10 @@ trait TestReporter {
 
 }
 
+/**
+  * Dummy Test Reporter that doesn't report
+  * anything for any testing event.
+  */
 object  DummyReporter extends TestReporter {
   override def logStart(event:  Event): Unit = {