Highlight code errors on formatting and evaluation (#948)

assets/js/cell/index.js

@@ -108,6 +108,13 @@ const Cell = {
           this.state.liveEditor.onChange((newSource) => {
             updateChangeIndicator();
           });
+
+          this.handleEvent(
+            `evaluation_finished:${this.props.cellId}`,
+            ({ code_error }) => {
+              this.state.liveEditor.setCodeErrorMarker(code_error);
+            }
+          );
         }
 
         // Setup markdown updates

assets/js/cell/live_editor.js

@@ -140,6 +140,34 @@ class LiveEditor {
     }
   }
 
+  /**
+   * Adds an underline marker for the given syntax error.
+   *
+   * To clear an existing marker `null` error is also supported.
+   */
+  setCodeErrorMarker(error) {
+    const owner = "elixir.error.syntax";
+
+    if (error) {
+      const line = this.editor.getModel().getLineContent(error.line);
+      const [, leadingWhitespace, trailingWhitespace] =
+        line.match(/^(\s*).*?(\s*)$/);
+
+      monaco.editor.setModelMarkers(this.editor.getModel(), owner, [
+        {
+          startLineNumber: error.line,
+          startColumn: leadingWhitespace.length + 1,
+          endLineNumber: error.line,
+          endColumn: line.length + 1 - trailingWhitespace.length,
+          message: error.description,
+          severity: monaco.MarkerSeverity.Error,
+        },
+      ]);
+    } else {
+      monaco.editor.setModelMarkers(this.editor.getModel(), owner, []);
+    }
+  }
+
   __mountEditor() {
     const settings = settingsStore.get();
 
@@ -347,37 +375,43 @@ class LiveEditor {
 
       return this.__asyncIntellisenseRequest("format", { code: content })
         .then((response) => {
-          /**
-           * We use a single edit replacing the whole editor content,
-           * but the editor itself optimises this into a list of edits
-           * that produce minimal diff using the Myers string difference.
-           *
-           * References:
-           *   * https://github.com/microsoft/vscode/blob/628b4d46357f2420f1dbfcea499f8ff59ee2c251/src/vs/editor/contrib/format/format.ts#L324
-           *   * https://github.com/microsoft/vscode/blob/628b4d46357f2420f1dbfcea499f8ff59ee2c251/src/vs/editor/common/services/editorSimpleWorker.ts#L489
-           *   * https://github.com/microsoft/vscode/blob/628b4d46357f2420f1dbfcea499f8ff59ee2c251/src/vs/base/common/diff/diff.ts#L227-L231
-           *
-           * Eventually the editor will received the optimised list of edits,
-           * which we then convert to Delta and send to the server.
-           * Consequently, the Delta carries only the minimal formatting diff.
-           *
-           * Also, if edits are applied to the editor, either by typing
-           * or receiving remote changes, the formatting is cancelled.
-           * In other words the formatting changes are actually applied
-           * only if the editor stays intact.
-           *
-           * References:
-           *   * https://github.com/microsoft/vscode/blob/628b4d46357f2420f1dbfcea499f8ff59ee2c251/src/vs/editor/contrib/format/format.ts#L313
-           *   * https://github.com/microsoft/vscode/blob/628b4d46357f2420f1dbfcea499f8ff59ee2c251/src/vs/editor/browser/core/editorState.ts#L137
-           *   * https://github.com/microsoft/vscode/blob/628b4d46357f2420f1dbfcea499f8ff59ee2c251/src/vs/editor/contrib/format/format.ts#L326
-           */
+          this.setCodeErrorMarker(response.code_error);
 
-          const replaceEdit = {
-            range: model.getFullModelRange(),
-            text: response.code,
-          };
+          if (response.code) {
+            /**
+             * We use a single edit replacing the whole editor content,
+             * but the editor itself optimises this into a list of edits
+             * that produce minimal diff using the Myers string difference.
+             *
+             * References:
+             *   * https://github.com/microsoft/vscode/blob/628b4d46357f2420f1dbfcea499f8ff59ee2c251/src/vs/editor/contrib/format/format.ts#L324
+             *   * https://github.com/microsoft/vscode/blob/628b4d46357f2420f1dbfcea499f8ff59ee2c251/src/vs/editor/common/services/editorSimpleWorker.ts#L489
+             *   * https://github.com/microsoft/vscode/blob/628b4d46357f2420f1dbfcea499f8ff59ee2c251/src/vs/base/common/diff/diff.ts#L227-L231
+             *
+             * Eventually the editor will received the optimised list of edits,
+             * which we then convert to Delta and send to the server.
+             * Consequently, the Delta carries only the minimal formatting diff.
+             *
+             * Also, if edits are applied to the editor, either by typing
+             * or receiving remote changes, the formatting is cancelled.
+             * In other words the formatting changes are actually applied
+             * only if the editor stays intact.
+             *
+             * References:
+             *   * https://github.com/microsoft/vscode/blob/628b4d46357f2420f1dbfcea499f8ff59ee2c251/src/vs/editor/contrib/format/format.ts#L313
+             *   * https://github.com/microsoft/vscode/blob/628b4d46357f2420f1dbfcea499f8ff59ee2c251/src/vs/editor/browser/core/editorState.ts#L137
+             *   * https://github.com/microsoft/vscode/blob/628b4d46357f2420f1dbfcea499f8ff59ee2c251/src/vs/editor/contrib/format/format.ts#L326
+             */
 
-          return [replaceEdit];
+            const replaceEdit = {
+              range: model.getFullModelRange(),
+              text: response.code,
+            };
+
+            return [replaceEdit];
+          } else {
+            return [];
+          }
         })
         .catch(() => null);
     };

lib/livebook/evaluator.ex

@@ -289,16 +289,16 @@ defmodule Livebook.Evaluator do
     context = put_in(context.env.file, file)
     start_time = System.monotonic_time()
 
-    {result_context, response} =
+    {result_context, response, code_error} =
       case eval(code, context.binding, context.env) do
         {:ok, result, binding, env} ->
           result_context = %{binding: binding, env: env, id: random_id()}
           response = {:ok, result}
-          {result_context, response}
+          {result_context, response, nil}
 
-        {:error, kind, error, stacktrace} ->
+        {:error, kind, error, stacktrace, code_error} ->
           response = {:error, kind, error, stacktrace}
-          {context, response}
+          {context, response, code_error}
       end
 
     evaluation_time_ms = get_execution_time_delta(start_time)
@@ -309,7 +309,13 @@ defmodule Livebook.Evaluator do
     Evaluator.IOProxy.clear_input_cache(state.io_proxy)
 
     output = state.formatter.format_response(response)
-    metadata = %{evaluation_time_ms: evaluation_time_ms, memory_usage: memory()}
+
+    metadata = %{
+      evaluation_time_ms: evaluation_time_ms,
+      memory_usage: memory(),
+      code_error: code_error
+    }
+
     send(send_to, {:evaluation_response, ref, output, metadata})
 
     :erlang.garbage_collect(self())
@@ -391,10 +397,23 @@ defmodule Livebook.Evaluator do
     catch
       kind, error ->
         stacktrace = prune_stacktrace(__STACKTRACE__)
-        {:error, kind, error, stacktrace}
+
+        code_error =
+          if code_error?(error) and (error.file == env.file and error.file != "nofile") do
+            %{line: error.line, description: error.description}
+          else
+            nil
+          end
+
+        {:error, kind, error, stacktrace, code_error}
     end
   end
 
+  defp code_error?(%SyntaxError{}), do: true
+  defp code_error?(%TokenMissingError{}), do: true
+  defp code_error?(%CompileError{}), do: true
+  defp code_error?(_error), do: false
+
   # Adapted from https://github.com/elixir-lang/elixir/blob/1c1654c88adfdbef38ff07fc30f6fbd34a542c07/lib/iex/lib/iex/evaluator.ex#L355-L372
 
   @elixir_internals [:elixir, :elixir_expand, :elixir_compiler, :elixir_module] ++

lib/livebook/intellisense.ex

@@ -51,16 +51,13 @@ defmodule Livebook.Intellisense do
   end
 
   def handle_request({:format, code}, _context) do
-    case format_code(code) do
-      {:ok, code} -> %{code: code}
-      :error -> nil
-    end
+    format_code(code)
   end
 
   @doc """
   Formats Elixir code.
   """
-  @spec format_code(String.t()) :: {:ok, String.t()} | :error
+  @spec format_code(String.t()) :: Runtime.format_response()
   def format_code(code) do
     try do
       formatted =
@@ -68,9 +65,11 @@ defmodule Livebook.Intellisense do
         |> Code.format_string!()
         |> IO.iodata_to_binary()
 
-      {:ok, formatted}
+      %{code: formatted, code_error: nil}
     rescue
-      _ -> :error
+      error ->
+        code_error = %{line: error.line, description: error.description}
+        %{code: nil, code_error: code_error}
     end
   end
 

lib/livebook/runtime.ex

@@ -112,9 +112,12 @@ defprotocol Livebook.Runtime do
   @type format_request :: {:format, code :: String.t()}
 
   @type format_response :: %{
-          code: String.t()
+          code: String.t() | nil,
+          code_error: code_error() | nil
         }
 
+  @type code_error :: %{line: pos_integer(), description: String.t()}
+
   @typedoc """
   The runtime memory usage for each type in bytes.
 
@@ -173,7 +176,7 @@ defprotocol Livebook.Runtime do
 
     * `{:evaluation_response, ref, output, metadata}` - final
       result of the evaluation. Recognised metadata entries
-      are: `evaluation_time_ms` and `memory_usage`
+      are: `code_error`, `evaluation_time_ms` and `memory_usage`
 
   The output may include input fields. The evaluation may then
   request the current value of a previously rendered input by

lib/livebook_web/live/session_live.ex

@@ -1212,9 +1212,11 @@ defmodule LivebookWeb.SessionLive do
   defp after_operation(
          socket,
          _prev_socket,
-         {:add_cell_evaluation_response, _client_pid, _id, _output, _metadata}
+         {:add_cell_evaluation_response, _client_pid, cell_id, _output, metadata}
        ) do
-    prune_outputs(socket)
+    socket
+    |> prune_outputs()
+    |> push_event("evaluation_finished:#{cell_id}", %{code_error: metadata.code_error})
   end
 
   defp after_operation(socket, _prev_socket, _operation), do: socket

test/livebook/evaluator_test.exs

@@ -11,7 +11,7 @@ defmodule Livebook.EvaluatorTest do
 
   defmacrop metadata do
     quote do
-      %{evaluation_time_ms: _, memory_usage: %{}}
+      %{evaluation_time_ms: _, memory_usage: %{}, code_error: _}
     end
   end
 
@@ -91,13 +91,54 @@ defmodule Livebook.EvaluatorTest do
       List.first(%{})
       """
 
-      Evaluator.evaluate_code(evaluator, self(), code, :code_1)
+      Evaluator.evaluate_code(evaluator, self(), code, :code_1, nil, file: "file.ex")
 
       assert_receive {:evaluation_response, :code_1,
                       {:error, :error, :function_clause, [{List, :first, _arity, _location}]},
                       metadata()}
     end
 
+    test "returns additional metadata when there is a syntax error", %{evaluator: evaluator} do
+      code = "1+"
+
+      Evaluator.evaluate_code(evaluator, self(), code, :code_1, nil, file: "file.ex")
+
+      assert_receive {:evaluation_response, :code_1, {:error, :error, %TokenMissingError{}, []},
+                      %{
+                        code_error: %{
+                          line: 1,
+                          description: "syntax error: expression is incomplete"
+                        }
+                      }}
+    end
+
+    test "returns additional metadata when there is a compilation error", %{evaluator: evaluator} do
+      code = "x"
+
+      Evaluator.evaluate_code(evaluator, self(), code, :code_1, nil, file: "file.ex")
+
+      assert_receive {:evaluation_response, :code_1, {:error, :error, %CompileError{}, []},
+                      %{
+                        code_error: %{
+                          line: 1,
+                          description: "undefined function x/0 (there is no such import)"
+                        }
+                      }}
+    end
+
+    test "ignores code errors when they happen in the actual evaluation", %{evaluator: evaluator} do
+      code = """
+      Code.eval_string("x")
+      """
+
+      Evaluator.evaluate_code(evaluator, self(), code, :code_1, nil, file: "file.ex")
+
+      expected_stacktrace = [{Code, :validated_eval_string, 3, [file: 'lib/code.ex', line: 404]}]
+
+      assert_receive {:evaluation_response, :code_1,
+                      {:error, :error, %CompileError{}, ^expected_stacktrace}, %{code_error: nil}}
+    end
+
     test "in case of an error returns only the relevant part of stacktrace",
          %{evaluator: evaluator} do
       code = """

test/livebook/intellisense_test.exs

@@ -30,6 +30,22 @@ defmodule Livebook.IntellisenseTest do
 
   alias Livebook.Intellisense
 
+  describe "format_code/1" do
+    test "formats valid code" do
+      assert %{code: "1 + 1", code_error: nil} = Intellisense.format_code("1+1")
+    end
+
+    test "returns a syntax error when invalid code is given" do
+      assert %{
+               code: nil,
+               code_error: %{
+                 line: 1,
+                 description: "syntax error: expression is incomplete"
+               }
+             } = Intellisense.format_code("1+")
+    end
+  end
+
   describe "get_completion_items/3" do
     test "completion when no hint given" do
       context = eval(do: nil)