Try using execute_if/1 to either run a test or skip it, and fix all…

… tests of supported versions.

Although this approach might result in the repetition of the source text,
I believe it will make the `assertion part` clearer and reduce our maintenance burden.

apps/remote_control/lib/lexical/remote_control/build/document/compilers/quoted.ex

@@ -66,8 +66,9 @@ defmodule Lexical.RemoteControl.Build.Document.Compilers.Quoted do
       {captured_messages, {:exception, exception, stack, quoted_ast}} ->
         error = Build.Error.error_to_diagnostic(document, exception, stack, quoted_ast)
         diagnostics = Build.Error.message_to_diagnostic(document, captured_messages)
+        refined = Build.Error.refine_diagnostics([error | diagnostics])
 
-        {:error, [error | diagnostics]}
+        {:error, refined}
 
       {"", {:ok, modules}} ->
         purge_removed_modules(old_modules, modules)

apps/remote_control/lib/lexical/remote_control/build/error.ex

@@ -1,13 +1,11 @@
 defmodule Lexical.RemoteControl.Build.Error do
   alias Lexical.Document
   alias Lexical.Plugin.V1.Diagnostic.Result
+  alias Lexical.RemoteControl.Build.Location
   alias Mix.Task.Compiler
 
   require Logger
 
-  import Lexical.RemoteControl.Build.ErrorSupport,
-    only: [context_to_position: 1, position: 1, position: 2]
-
   @elixir_source "Elixir"
 
   @doc """
@@ -27,40 +25,7 @@ defmodule Lexical.RemoteControl.Build.Error do
       |> normalize()
       |> format()
     end)
-    |> uniq()
-  end
-
-  defp uniq(diagnostics) do
-    exacts =
-      Enum.filter(diagnostics, fn diagnostic -> match?({_, _, _, _}, diagnostic.position) end)
-
-    extract_line = fn
-      %Result{position: {line, _column}} -> line
-      %Result{position: line} -> line
-    end
-
-    # Note: Sometimes error and warning appear on one line at the same time
-    # So we need to uniq by line and severity,
-    # and :error is always more important than :warning
-    extract_line_and_severity = &{extract_line.(&1), &1.severity}
-
-    filtered =
-      diagnostics
-      |> Enum.filter(fn diagnostic -> not match?({_, _, _, _}, diagnostic.position) end)
-      |> Enum.sort_by(extract_line_and_severity)
-      |> Enum.uniq_by(extract_line)
-      |> reject_zeroth_line()
-
-    exacts ++ filtered
-  end
-
-  defp reject_zeroth_line(diagnostics) do
-    # Since 1.15, Elixir has some nonsensical error on line 0,
-    # e.g.: Can't compile this file
-    # We can simply ignore it, as there is a more accurate one
-    Enum.reject(diagnostics, fn diagnostic ->
-      diagnostic.position == 0
-    end)
+    |> Location.uniq()
   end
 
   defp normalize(%Compiler.Diagnostic{} = diagnostic) do
@@ -116,7 +81,7 @@ defmodule Lexical.RemoteControl.Build.Error do
 
       position =
         if span = error_or_wanning[:span] do
-          position(pos, span)
+          Location.position(pos, span)
         else
           pos
         end
@@ -135,7 +100,7 @@ defmodule Lexical.RemoteControl.Build.Error do
 
     Result.new(
       path,
-      position(compile_error.line),
+      Location.position(compile_error.line),
       compile_error.description,
       :error,
       @elixir_source
@@ -150,7 +115,7 @@ defmodule Lexical.RemoteControl.Build.Error do
       ) do
     [{_module, _function, _arity, context} | _] = stack
     message = Exception.message(function_clause)
-    position = context_to_position(context)
+    position = Location.context_to_position(context)
     Result.new(source.uri, position, message, :error, @elixir_source)
   end
 
@@ -161,7 +126,7 @@ defmodule Lexical.RemoteControl.Build.Error do
         _quoted_ast
       ) do
     message = Exception.message(error)
-    position = position(1)
+    position = Location.position(1)
     Result.new(source.uri, position, message, :error, @elixir_source)
   end
 
@@ -214,7 +179,7 @@ defmodule Lexical.RemoteControl.Build.Error do
 
     position =
       if pipe_or_struct? or expanding? do
-        context_to_position(context)
+        Location.context_to_position(context)
       else
         stack_to_position(stack)
       end
@@ -295,13 +260,13 @@ defmodule Lexical.RemoteControl.Build.Error do
 
     cond do
       is_nil(context) ->
-        position(0)
+        Location.position(0)
 
       Keyword.has_key?(context, :line) and Keyword.has_key?(context, :column) ->
-        position(context[:line], context[:column])
+        Location.position(context[:line], context[:column])
 
       Keyword.has_key?(context, :line) ->
-        position(context[:line])
+        Location.position(context[:line])
 
       true ->
         nil
@@ -325,7 +290,7 @@ defmodule Lexical.RemoteControl.Build.Error do
 
   defp stack_to_position([{_, target, _, context} | _rest])
        when target in [:__FILE__, :__MODULE__] do
-    context_to_position(context)
+    Location.context_to_position(context)
   end
 
   defp stack_to_position([]) do

apps/remote_control/lib/lexical/remote_control/build/location.ex

@@ -0,0 +1,65 @@
+defmodule Lexical.RemoteControl.Build.Location do
+  alias Lexical.Plugin.V1.Diagnostic.Result
+
+  def context_to_position(context) do
+    cond do
+      Keyword.has_key?(context, :line) and Keyword.has_key?(context, :column) ->
+        position(context[:line], context[:column])
+
+      Keyword.has_key?(context, :line) ->
+        position(context[:line])
+
+      true ->
+        nil
+    end
+  end
+
+  def position(line) do
+    line
+  end
+
+  def position({line, column}, {end_line, end_column}) do
+    {line, column, end_line, end_column}
+  end
+
+  def position(line, column) do
+    {line, column}
+  end
+
+  def position(line, column, end_line, end_column) do
+    {line, column, end_line, end_column}
+  end
+
+  def uniq(diagnostics) do
+    exacts =
+      Enum.filter(diagnostics, fn diagnostic -> match?({_, _, _, _}, diagnostic.position) end)
+
+    extract_line = fn
+      %Result{position: {line, _column}} -> line
+      %Result{position: line} -> line
+    end
+
+    # Note: Sometimes error and warning appear on one line at the same time
+    # So we need to uniq by line and severity,
+    # and :error is always more important than :warning
+    extract_line_and_severity = &{extract_line.(&1), &1.severity}
+
+    filtered =
+      diagnostics
+      |> Enum.filter(fn diagnostic -> not match?({_, _, _, _}, diagnostic.position) end)
+      |> Enum.sort_by(extract_line_and_severity)
+      |> Enum.uniq_by(extract_line)
+      |> reject_zeroth_line()
+
+    exacts ++ filtered
+  end
+
+  defp reject_zeroth_line(diagnostics) do
+    # Since 1.15, Elixir has some nonsensical error on line 0,
+    # e.g.: Can't compile this file
+    # We can simply ignore it, as there is a more accurate one
+    Enum.reject(diagnostics, fn diagnostic ->
+      diagnostic.position == 0
+    end)
+  end
+end

apps/remote_control/lib/lexical/remote_control/build/parse_error.ex

@@ -2,9 +2,7 @@ defmodule Lexical.RemoteControl.Build.ParseError do
   alias Lexical.Document
   alias Lexical.Plugin.V1.Diagnostic.Result
 
-  import Lexical.RemoteControl.Build.ErrorSupport,
-    only: [context_to_position: 1, position: 2, position: 4]
-
+  alias Lexical.RemoteControl.Build.Location
   @elixir_source "Elixir"
 
   # Parse errors happen during Code.string_to_quoted and are raised as SyntaxErrors, and TokenMissingErrors.
@@ -20,7 +18,7 @@ defmodule Lexical.RemoteControl.Build.ParseError do
     detail_diagnostics = detail_diagnostics(source, detail)
     error = message_info_to_binary(message_info, token)
     error_diagnostics = to_diagnostics(source, context, error, token)
-    error_diagnostics ++ detail_diagnostics
+    Location.uniq(detail_diagnostics ++ error_diagnostics)
   end
 
   def to_diagnostics(%Document{} = source, context, message_info, token)
@@ -49,7 +47,9 @@ defmodule Lexical.RemoteControl.Build.ParseError do
       &build_hint_diagnostics/4
     ]
 
-    Enum.flat_map(parse_error_diagnostic_functions, & &1.(source, context, message_info, token))
+    parse_error_diagnostic_functions
+    |> Enum.flat_map(& &1.(source, context, message_info, token))
+    |> Location.uniq()
   end
 
   defp build_end_line_diagnostics_from_context(
@@ -66,7 +66,7 @@ defmodule Lexical.RemoteControl.Build.ParseError do
       end
 
     if context[:end_line] do
-      pos = position(context[:end_line], context[:end_column])
+      pos = Location.position(context[:end_line], context[:end_column])
       result = Result.new(source.uri, pos, message, :error, @elixir_source)
       [result]
     else
@@ -84,7 +84,9 @@ defmodule Lexical.RemoteControl.Build.ParseError do
         end_line_message <> token
       end
 
-    diagnostic = Result.new(source.uri, context_to_position(context), message, :error, "Elixir")
+    diagnostic =
+      Result.new(source.uri, Location.context_to_position(context), message, :error, "Elixir")
+
     [diagnostic]
   end
 
@@ -110,7 +112,7 @@ defmodule Lexical.RemoteControl.Build.ParseError do
     opening_delimiter_length = opening_delimiter |> Atom.to_string() |> String.length()
 
     pos =
-      position(
+      Location.position(
         context[:line],
         context[:column],
         context[:line],
@@ -123,7 +125,7 @@ defmodule Lexical.RemoteControl.Build.ParseError do
 
   defp build_syntax_error_diagnostic(%Document{} = source, context, message_info, token) do
     message = "#{message_info}#{token}"
-    pos = position(context[:line], context[:column])
+    pos = Location.position(context[:line], context[:column])
     result = Result.new(source.uri, pos, message, :error, @elixir_source)
     [result]
   end

apps/remote_control/test/lexical/remote_control/build/document/compilers/config_test.exs

@@ -93,7 +93,7 @@ defmodule Lexical.RemoteControl.Build.Document.Compilers.ConfigTest do
                |> compile()
 
       assert result.message =~ "missing terminator"
-      assert result.position == {1, 12}
+      assert result.position in [{1, 12}, {1, 1}]
       assert result.severity == :error
       assert result.source == "Elixir"
     end

apps/remote_control/test/lexical/remote_control/build/document/compilers/eex_test.exs

@@ -12,6 +12,7 @@ defmodule Lexical.RemoteControl.Build.Document.Compilers.EExTest do
   import Compilers.EEx, only: [recognizes?: 1]
   import Lexical.Test.Quiet
   import Lexical.Test.CodeSigil
+  import Lexical.Test.DiagnosticSupport
 
   def with_capture_server(_) do
     start_supervised!(CaptureServer)
@@ -85,6 +86,8 @@ defmodule Lexical.RemoteControl.Build.Document.Compilers.EExTest do
   describe "compile_quoted/2" do
     setup [:with_capture_server]
 
+    @feature_condition span_in_diagnostic?: false
+    @tag execute_if(@feature_condition)
     test "handles unused variables" do
       assert {:ok, [%Result{} = result]} =
                ~q[
@@ -99,6 +102,23 @@ defmodule Lexical.RemoteControl.Build.Document.Compilers.EExTest do
       assert result.source == "EEx"
       assert result.uri =~ "file:///file.eex"
     end
+
+    @feature_condition span_in_diagnostic?: true
+    @tag execute_if(@feature_condition)
+    test "handles unused variables when #{inspect(@feature_condition)}" do
+      assert {:ok, [%Result{} = result]} =
+               ~q[
+               <%= something = 6 %>
+               ]
+               |> document_with_content()
+               |> compile()
+
+      assert result.message == ~s[variable "something" is unused]
+      assert result.position == {1, 5, 1, 14}
+      assert result.severity == :warning
+      assert result.source == "EEx"
+      assert result.uri =~ "file:///file.eex"
+    end
   end
 
   describe "eval_quoted/2" do
@@ -115,7 +135,8 @@ defmodule Lexical.RemoteControl.Build.Document.Compilers.EExTest do
       assert result.uri =~ "file:///file.eex"
     end
 
-    @tag :with_diagnostics
+    @feature_condition span_in_diagnostic?: false
+    @tag execute_if(@feature_condition)
     test "handles undefinied variable" do
       document = document_with_content(~q[
         <%= thing %>
@@ -134,5 +155,21 @@ defmodule Lexical.RemoteControl.Build.Document.Compilers.EExTest do
       assert result.source == "EEx"
       assert result.uri =~ "file:///file.eex"
     end
+
+    @feature_condition span_in_diagnostic?: true
+    @tag execute_if(@feature_condition)
+    test "handles undefinied variable when #{inspect(@feature_condition)}" do
+      document = document_with_content(~q[
+        <%= thing %>
+      ])
+
+      assert {:error, [%Result{} = result]} = compile(document)
+
+      assert result.message == "undefined variable \"thing\""
+      assert result.position == {1, 5, 1, 10}
+      assert result.severity == :error
+      assert result.source == "EEx"
+      assert result.uri =~ "file:///file.eex"
+    end
   end
 end