Attempt to compile things from BSP even when import hooks failed

com-lihaoyi · Jul 23, 2020 · 27d92eb · 27d92eb
1 parent 87426f7
commit 27d92eb
Show file tree

Hide file tree

Showing 4 changed files with 65 additions and 42 deletions.
diff --git a/amm/interp/src/main/scala/ammonite/interp/script/AmmoniteBuildServer.scala b/amm/interp/src/main/scala/ammonite/interp/script/AmmoniteBuildServer.scala
@@ -385,20 +385,14 @@ class AmmoniteBuildServer(
     result
   }
 
-  private def compileScript(script: Script, target: BuildTargetIdentifier): Boolean =
-    if (script.processorDiagnostics.isEmpty) {
-      val (_, res) = compiler.compile(
-        script,
-        proc,
-        doCompile = compileScript(_, _)
-      )
-
-      res.isRight
-    } else {
-      for (client <- clientOpt)
-        sendDiagnostics(client, script, target, Nil)
-      false
-    }
+  private def compileScript(script: Script, target: BuildTargetIdentifier): Boolean = {
+    val (_, res) = compiler.compile(
+      script,
+      proc,
+      doCompile = compileScript(_, _)
+    )
+    script.processorDiagnostics.isEmpty && res.isRight
+  }
 
   def buildTargetCompile(params: CompileParams): CompletableFuture[CompileResult] =
     on(compileEc) {

diff --git a/amm/interp/src/main/scala/ammonite/interp/script/ScriptProcessor.scala b/amm/interp/src/main/scala/ammonite/interp/script/ScriptProcessor.scala
@@ -42,7 +42,7 @@ final case class ScriptProcessor(
       Seq(Diagnostic("ERROR", startPos, endPos, s"Expected $expected"))
     }
 
-    def hookFor(tree: ImportTree): Either[Diagnostic, (ImportTree, (Seq[String], ImportHook))] = {
+    def hookFor(tree: ImportTree): Either[Diagnostic, (Seq[String], ImportHook)] = {
       val hookOpt = importHooks.find { case (k, v) => tree.strippedPrefix.startsWith(k) }
       hookOpt.toRight {
         Diagnostic(
@@ -51,7 +51,7 @@ final case class ScriptProcessor(
           offsetToPos(tree.end),
           s"Invalid import hook '${tree.strippedPrefix.mkString(".")}'"
         )
-      }.map((tree, _))
+      }
     }
 
     def hookResults(
@@ -75,33 +75,32 @@ final case class ScriptProcessor(
       }
     }
 
-    val res = for {
-      elems <- splittedScript
-      withImportHooks <- elems
-        .traverse {
-          case (startIdx, leadingSpaces, statements) =>
-            val (statements0, importTrees) =
-              Parsers.parseImportHooksWithIndices(codeSource, statements)
-            importTrees.traverse(hookFor).map((startIdx, leadingSpaces, statements0, _))
-        }
-        .left.map(_.flatten)
-      r <-  withImportHooks
-        .traverse {
-          case (startIdx, leadingSpaces, statements0, imports) =>
-            imports
-              .traverse {
-                case (tree, (hookPrefix, hook)) =>
-                  hookResults(hookPrefix, hook, tree)
-              }
-              .map(l => Script.Block(startIdx, leadingSpaces, statements0, l.flatten))
-        }
-        .left.map(_.flatten)
-    } yield r
-
-    res match {
-      case Right(blocks) => Script(code, codeSource, blocks, Nil)
-      case Left(diagnostics) => Script(code, codeSource, Nil, diagnostics)
+    val diagnostics = new mutable.ListBuffer[Diagnostic]
+
+    for (l <- splittedScript.left)
+      diagnostics ++= l
+
+    val blocks = for {
+      elems <- splittedScript.right.toSeq
+      (startIdx, leadingSpaces, statements) <- elems
+    } yield {
+      val (statements0, importTrees) = Parsers.parseImportHooksWithIndices(codeSource, statements)
+      val importResults =
+        for {
+          tree <- importTrees
+          (hookPrefix, hook) <- hookFor(tree) match {
+            case Left(diag) => diagnostics += diag; Nil
+            case Right(imports0) => Seq(imports0)
+          }
+          res <- hookResults(hookPrefix, hook, tree) match {
+            case Left(diag) => diagnostics += diag; Nil
+            case Right(res) => res
+          }
+        } yield res
+      Script.Block(startIdx, leadingSpaces, statements0, importResults)
     }
+
+    Script(code, codeSource, blocks, diagnostics.toVector)
   }
 
   def load(path: os.Path, codeSource: CodeSource): Script = {

diff --git a/amm/src/test/resources/bsp/error/error-and-invalid-import-file.sc b/amm/src/test/resources/bsp/error/error-and-invalid-import-file.sc
@@ -0,0 +1,3 @@
+import $file.nope.nope.nope
+
+zz
diff --git a/amm/src/test/scala/ammonite/interp/script/AmmoniteBuildServerTests.scala b/amm/src/test/scala/ammonite/interp/script/AmmoniteBuildServerTests.scala
@@ -422,6 +422,33 @@ object AmmoniteBuildServerTests extends TestSuite {
           _ = assert(diagnostics == expectedDiagnostics)
         } yield ()
       }
+
+      "import file and compilation" - {
+        val runner = new BspScriptRunner(wd / "error" / "error-and-invalid-import-file.sc")
+
+        val expectedDiagnostics = List(
+          new BDiagnostic(
+            new Range(new BPosition(0, 7), new BPosition(0, 27)),
+            "Cannot resolve $file import: ./error/nope/nope/nope.sc"
+          ),
+          new BDiagnostic(
+            new Range(new BPosition(2, 0), new BPosition(2, 2)),
+            "not found: value zz"
+          )
+        )
+        expectedDiagnostics.foreach(_.setSeverity(DiagnosticSeverity.ERROR))
+
+        for {
+          _ <- runner.init()
+          _ <- runner.compile(StatusCode.ERROR)
+
+          diagnostics = runner.diagnostics()
+          _ = diagnostics.foreach { diag =>
+            diag.setMessage(diag.getMessage.replace(wd.toString, "."))
+          }
+          _ = assert(diagnostics == expectedDiagnostics)
+        } yield ()
+      }
     }
 
     "multi" - {