From 87efc1b88c90d167f71a9e01fddf469022d31451 Mon Sep 17 00:00:00 2001 From: Scott Ming Date: Thu, 4 Jan 2024 19:06:13 +0800 Subject: [PATCH] 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. --- .../build/document/compilers/quoted.ex | 3 +- .../lib/lexical/remote_control/build/error.ex | 57 +---- .../remote_control/build/error_support.ex | 30 --- .../lexical/remote_control/build/location.ex | 65 +++++ .../remote_control/build/parse_error.ex | 20 +- .../build/document/compilers/config_test.exs | 2 +- .../build/document/compilers/eex_test.exs | 39 ++- .../remote_control/build/error_test.exs | 143 +++++++++-- .../remote_control/build/parse_error_test.exs | 226 +++++++++++++++--- .../lexical/remote_control/build_test.exs | 39 ++- .../lib/lexical/test/diagnostic_support.ex | 14 ++ 11 files changed, 487 insertions(+), 151 deletions(-) delete mode 100644 apps/remote_control/lib/lexical/remote_control/build/error_support.ex create mode 100644 apps/remote_control/lib/lexical/remote_control/build/location.ex create mode 100644 projects/lexical_test/lib/lexical/test/diagnostic_support.ex diff --git a/apps/remote_control/lib/lexical/remote_control/build/document/compilers/quoted.ex b/apps/remote_control/lib/lexical/remote_control/build/document/compilers/quoted.ex index 96654811b..50fac55be 100644 --- a/apps/remote_control/lib/lexical/remote_control/build/document/compilers/quoted.ex +++ b/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) diff --git a/apps/remote_control/lib/lexical/remote_control/build/error.ex b/apps/remote_control/lib/lexical/remote_control/build/error.ex index bc989cece..c7629561a 100644 --- a/apps/remote_control/lib/lexical/remote_control/build/error.ex +++ b/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 diff --git a/apps/remote_control/lib/lexical/remote_control/build/error_support.ex b/apps/remote_control/lib/lexical/remote_control/build/error_support.ex deleted file mode 100644 index c3dfe3498..000000000 --- a/apps/remote_control/lib/lexical/remote_control/build/error_support.ex +++ /dev/null @@ -1,30 +0,0 @@ -defmodule Lexical.RemoteControl.Build.ErrorSupport do - 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 -end diff --git a/apps/remote_control/lib/lexical/remote_control/build/location.ex b/apps/remote_control/lib/lexical/remote_control/build/location.ex new file mode 100644 index 000000000..43287996d --- /dev/null +++ b/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 diff --git a/apps/remote_control/lib/lexical/remote_control/build/parse_error.ex b/apps/remote_control/lib/lexical/remote_control/build/parse_error.ex index a0c74b1b3..d9f04f62f 100644 --- a/apps/remote_control/lib/lexical/remote_control/build/parse_error.ex +++ b/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 diff --git a/apps/remote_control/test/lexical/remote_control/build/document/compilers/config_test.exs b/apps/remote_control/test/lexical/remote_control/build/document/compilers/config_test.exs index 201a5bcd2..d4515078a 100644 --- a/apps/remote_control/test/lexical/remote_control/build/document/compilers/config_test.exs +++ b/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 diff --git a/apps/remote_control/test/lexical/remote_control/build/document/compilers/eex_test.exs b/apps/remote_control/test/lexical/remote_control/build/document/compilers/eex_test.exs index d6cc13ad3..0116a4c0a 100644 --- a/apps/remote_control/test/lexical/remote_control/build/document/compilers/eex_test.exs +++ b/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 diff --git a/apps/remote_control/test/lexical/remote_control/build/error_test.exs b/apps/remote_control/test/lexical/remote_control/build/error_test.exs index 470426264..3edac9f2d 100644 --- a/apps/remote_control/test/lexical/remote_control/build/error_test.exs +++ b/apps/remote_control/test/lexical/remote_control/build/error_test.exs @@ -5,6 +5,7 @@ defmodule Lexical.RemoteControl.Build.ErrorTest do alias Lexical.RemoteControl.ModuleMappings require Logger + import Lexical.Test.DiagnosticSupport use ExUnit.Case use Patch @@ -62,6 +63,8 @@ defmodule Lexical.RemoteControl.Build.ErrorTest do :ok end + @feature_condition span_in_diagnostic?: false + @tag execute_if(@feature_condition) test "handles undefined variable" do diagnostic = ~S[ @@ -74,11 +77,31 @@ defmodule Lexical.RemoteControl.Build.ErrorTest do |> compile() |> diagnostic() - assert diagnostic.message =~ ~s[undefined variable "a"] - assert diagnostic.position in [{4, 13}, {4, 13, 4, 14}] + assert diagnostic.message in [~s[undefined variable "a"], ~s[undefined function a/0]] + assert diagnostic.position in [4, {4, 13}] end - test "handles unsued warning" do + @feature_condition span_in_diagnostic?: true + @tag execute_if(@feature_condition) + test "handles undefined variable when #{inspect(@feature_condition)}" do + diagnostic = + ~S[ + defmodule Foo do + def bar do + a + end + end + ] + |> compile() + |> diagnostic() + + assert diagnostic.message == ~s[undefined variable "a"] + assert diagnostic.position == {4, 13, 4, 14} + end + + @feature_condition span_in_diagnostic?: false + @tag execute_if(@feature_condition) + test "handles unsued variable warning" do diagnostic = ~S[ defmodule Foo do @@ -91,7 +114,63 @@ defmodule Lexical.RemoteControl.Build.ErrorTest do |> diagnostic() assert diagnostic.message =~ ~s[variable "a" is unused] - assert diagnostic.position in [{4, 13}, {4, 13, 4, 14}] + assert diagnostic.position in [4, {4, 13}] + end + + @feature_condition span_in_diagnostic?: true + @tag execute_if(@feature_condition) + test "handles unsued variable warning when #{inspect(@feature_condition)}" do + diagnostic = + ~S[ + defmodule Foo do + def bar do + a = 1 + end + end + ] + |> compile() + |> diagnostic() + + assert diagnostic.message == ~s[variable "a" is unused] + assert diagnostic.position == {4, 13, 4, 14} + end + + @feature_condition span_in_diagnostic?: false + @tag execute_if(@feature_condition) + test "handles unused function warning" do + diagnostic = + ~S[ + defmodule UnusedDefp do + defp unused do + end + end + ] + |> compile() + |> diagnostic() + + assert diagnostic.uri + assert diagnostic.severity == :warning + assert diagnostic.position == 3 + assert diagnostic.message =~ ~S[function unused/0 is unused] + end + + @feature_condition span_in_diagnostic?: true + @tag execute_if(@feature_condition) + test "handles unused function warning when #{inspect(@feature_condition)}" do + diagnostic = + ~S[ + defmodule UnusedDefp do + defp unused do + end + end + ] + |> compile() + |> diagnostic() + + assert diagnostic.uri + assert diagnostic.severity == :warning + assert diagnostic.position == {3, 16} + assert diagnostic.message =~ ~S[function unused/0 is unused] end test "handles FunctionClauseError" do @@ -157,7 +236,27 @@ defmodule Lexical.RemoteControl.Build.ErrorTest do assert diagnostic.position in [4, {4, 13, 4, 18}] end + @feature_condition span_in_diagnostic?: false + @tag execute_if(@feature_condition) test "handles multiple UndefinedError in one line" do + diagnostic = + ~S/ + defmodule Foo do + def bar do + [print(:bar), a, b] + end + end + / + |> compile() + |> diagnostic() + + assert diagnostic.position == 4 + assert diagnostic.message in [~s[undefined function print/1], ~s[undefined function a/0]] + end + + @feature_condition span_in_diagnostic?: true + @tag execute_if(@feature_condition) + test "handles multiple UndefinedError in one line when #{inspect(@feature_condition)}" do diagnostics = ~S/ defmodule Foo do @@ -169,21 +268,15 @@ defmodule Lexical.RemoteControl.Build.ErrorTest do |> compile() |> diagnostics() - if Features.span_in_diagnostic?() do - [func_diagnotic, b, a] = diagnostics - assert a.message =~ ~s[undefined variable "a"] - assert a.position == {4, 27, 4, 28} + [func_diagnotic, b, a] = diagnostics + assert a.message == ~s[undefined variable "a"] + assert a.position == {4, 27, 4, 28} - assert b.message =~ ~s[undefined variable "b"] - assert b.position == {4, 30, 4, 31} + assert b.message == ~s[undefined variable "b"] + assert b.position == {4, 30, 4, 31} - assert func_diagnotic.message =~ ~s[undefined function print/1] - assert func_diagnotic.position == {4, 14, 4, 19} - else - [diagnostic] = diagnostics - assert diagnostic.message =~ ~s[undefined function print/1] - assert diagnostic.position == 4 - end + assert func_diagnotic.message == ~s[undefined function print/1] + assert func_diagnotic.position == {4, 14, 4, 19} end test "handles UndefinedError without moudle" do @@ -199,6 +292,8 @@ defmodule Lexical.RemoteControl.Build.ErrorTest do assert diagnostic.position == {3, 14} end + @feature_condition with_diagnostics?: false + @tag execute_if(@feature_condition) test "handles ArgumentError" do diagnostics = ~s[String.to_integer ""] @@ -207,6 +302,20 @@ defmodule Lexical.RemoteControl.Build.ErrorTest do [diagnostic | _] = diagnostics + assert diagnostic.message == + ~s[warning: \e[0mthe call to String.to_integer/1 will fail with ArgumentError\n /unknown.ex:1] + end + + @feature_condition with_diagnostics?: true + @tag execute_if(@feature_condition) + test "handles ArgumentError when #{inspect(@feature_condition)}" do + diagnostics = + ~s[String.to_integer ""] + |> compile() + |> diagnostics() + + [diagnostic | _] = diagnostics + assert diagnostic.message =~ ~s[errors were found at the given arguments:] end diff --git a/apps/remote_control/test/lexical/remote_control/build/parse_error_test.exs b/apps/remote_control/test/lexical/remote_control/build/parse_error_test.exs index 67e48147c..10825c2e9 100644 --- a/apps/remote_control/test/lexical/remote_control/build/parse_error_test.exs +++ b/apps/remote_control/test/lexical/remote_control/build/parse_error_test.exs @@ -5,6 +5,7 @@ defmodule Lexical.RemoteControl.Build.ParseErrorTest do alias Lexical.RemoteControl.Build.CaptureServer alias Lexical.RemoteControl.Dispatch + import Lexical.Test.DiagnosticSupport use ExUnit.Case, async: true @@ -28,29 +29,39 @@ defmodule Lexical.RemoteControl.Build.ParseErrorTest do end describe "handling parse errors" do + @feature_condition details_in_context?: false + @tag execute_if(@feature_condition) test "handles token missing errors" do assert diagnostics = ~s[%{foo: 3] |> compile() |> diagnostics() - if Features.details_in_context?() do - [end_diagnostic, start_diagnostic] = diagnostics + [diagnostic] = diagnostics + assert diagnostic.message =~ ~s[missing terminator: }] + assert diagnostic.position == {1, 9} + end - assert start_diagnostic.message == - ~s[The "{" here is missing terminator "}"] + @feature_condition details_in_context?: true + @tag execute_if(@feature_condition) + test "handles token missing errors when #{inspect(@feature_condition)}" do + assert diagnostics = + ~s[%{foo: 3] + |> compile() + |> diagnostics() + + [start_diagnostic, end_diagnostic] = diagnostics - assert start_diagnostic.position == {1, 2, 1, 3} + assert start_diagnostic.message == + ~s[The "{" here is missing terminator "}"] - assert end_diagnostic.message == ~s[missing terminator: }] - assert end_diagnostic.position == {1, 9} - else - [diagnostic] = diagnostics - assert diagnostic.message =~ ~s[missing terminator: }] - assert diagnostic.position == {1, 9} - end + assert start_diagnostic.position == {1, 2, 1, 3} + assert end_diagnostic.message == ~s[missing terminator: }] + assert end_diagnostic.position == {1, 9} end + @feature_condition details_in_context?: false + @tag execute_if(@feature_condition) test "returns both the error and the detail when provided" do errors = ~S[ @@ -72,34 +83,90 @@ defmodule Lexical.RemoteControl.Build.ParseErrorTest do |> compile() |> diagnostics() - assert [error, detail] = errors + assert [start_diagnostic, end_diagnostic] = errors + + assert end_diagnostic.message =~ "unexpected reserved word: end" + assert end_diagnostic.position == {15, 9} + + assert String.downcase(start_diagnostic.message) =~ + ~S[the "(" here is missing terminator ")"] + + assert start_diagnostic.position == 4 + end + + @feature_condition details_in_context?: true + @tag execute_if(@feature_condition) + test "returns both the error and the detail when provided and #{inspect(@feature_condition)}" do + errors = + ~S[ + def handle_info(file_diagnostics(uri: uri, diagnostics: diagnostics), %State{} = state) do + state = State.clear(state, uri) + state = Enum.reduce(diagnostics, state, fn diagnostic, state -> + case State.add(diagnostic, state, uri) do + {:ok, new_state} -> + new_state + {:error, reason} -> + Logger.error("Could not add diagnostic #{inspect(diagnostic)} because #{inspect(error)}") + state + end + end + + publish_diagnostics(state) + end + ] + |> compile() + |> diagnostics() + + assert [start_diagnostic, end_diagnostic] = errors - assert error.message =~ "unexpected reserved word: end" - assert error.position == {15, 9} + assert start_diagnostic.message =~ "unexpected reserved word: end" + assert start_diagnostic.position == {15, 9} - assert String.downcase(detail.message) =~ ~S[the "(" here is missing terminator ")"] - assert detail.position in [4, {4, 28, 4, 29}] + assert String.downcase(end_diagnostic.message) =~ ~S[the "(" here is missing terminator ")"] + assert end_diagnostic.position == {4, 28, 4, 29} end - test "return the more precise one when there are multiple diagnostics on the same line" do - [end_diagnostic, start_diagnostic] = + @feature_condition details_in_context?: false, with_diagnostics?: false + @tag execute_if(@feature_condition) + test "returns multiple diagnostics on the same line" do + [end_diagnostic] = ~S{Keywor.get([], fn x -> )} |> compile() |> diagnostics() - assert end_diagnostic.message in [ - ~S[unexpected token: )], - ~S[unexpected token: ), expected: "end"] - ] + assert end_diagnostic.message =~ ~s[The \"fn\" here is missing terminator \"end\"] + assert end_diagnostic.position == 1 + end + + @feature_condition details_in_context?: false, with_diagnostics?: true + @tag execute_if(@feature_condition) + test "returns multiple diagnostics on the same line when #{inspect(@feature_condition)}" do + [end_diagnostic] = + ~S{Keywor.get([], fn x -> )} + |> compile() + |> diagnostics() + assert end_diagnostic.message =~ ~S[unexpected token: )] assert end_diagnostic.position == {1, 24} + end - assert String.downcase(start_diagnostic.message) =~ - ~S[the "fn" here is missing terminator "end"] + @feature_condition details_in_context?: true + @tag execute_if(@feature_condition) + test "returns multiple diagnostics on the same line when #{inspect(@feature_condition)}" do + [start_diagnostic, end_diagnostic] = + ~S{Keywor.get([], fn x -> )} + |> compile() + |> diagnostics() + + assert end_diagnostic.message == ~S[unexpected token: ), expected "end"] + assert end_diagnostic.position == {1, 24} - assert start_diagnostic.position in [1, {1, 16, 1, 18}] + assert start_diagnostic.message == ~S[The "fn" here is missing terminator "end"] + assert start_diagnostic.position == {1, 16, 1, 18} end + @feature_condition details_in_context?: false + @tag execute_if(@feature_condition) test "returns two diagnostics when missing end at the real end" do errors = ~S[ @@ -110,7 +177,7 @@ defmodule Lexical.RemoteControl.Build.ParseErrorTest do |> compile() |> diagnostics() - assert [end_diagnostic, start_diagnostic] = errors + assert [start_diagnostic, end_diagnostic] = errors assert %Diagnostic.Result{} = end_diagnostic assert end_diagnostic.message =~ "missing terminator: end" @@ -118,15 +185,40 @@ defmodule Lexical.RemoteControl.Build.ParseErrorTest do assert %Diagnostic.Result{} = start_diagnostic assert start_diagnostic.message == ~S[The "do" here is missing terminator "end"] - assert start_diagnostic.position in [2, {2, 23, 2, 25}] + assert start_diagnostic.position == 2 end - test "returns the token in the message when there is a token" do + @feature_condition details_in_context?: true + @tag execute_if(@feature_condition) + test "returns two diagnostics when missing end at the real end and #{inspect(@feature_condition)}" do + errors = + ~S[ + defmodule Foo do + def bar do + :ok + end] + |> compile() + |> diagnostics() + + assert [start_diagnostic, end_diagnostic] = errors + + assert %Diagnostic.Result{} = end_diagnostic + assert end_diagnostic.message == "missing terminator: end" + assert end_diagnostic.position == {5, 12} + + assert %Diagnostic.Result{} = start_diagnostic + assert start_diagnostic.message == ~S[The "do" here is missing terminator "end"] + assert start_diagnostic.position == {2, 23, 2, 25} + end + + test "returns the token in the message when encountering the `syntax error`" do diagnostic = ~S[1 + * 3] |> compile() |> diagnostic() assert diagnostic.message == "syntax error before: '*'" assert diagnostic.position == {1, 5} end + @feature_condition details_in_context?: false + @tag execute_if(@feature_condition) test "returns the approximate correct location when there is a hint." do diagnostics = ~S[ defmodule Foo do @@ -138,14 +230,39 @@ defmodule Lexical.RemoteControl.Build.ParseErrorTest do end end] |> compile() |> diagnostics() - [end_message, start_message, hint_message] = diagnostics + [start_diagnostic, hint_diagnostic, end_diagnostic] = diagnostics + assert start_diagnostic.message == ~S[The "do" here is missing terminator "end"] + assert start_diagnostic.position == 2 + assert end_diagnostic.message == ~S[missing terminator: end (for "do" starting at line 2)] + assert end_diagnostic.position == {9, 12} + + assert hint_diagnostic.message == + ~S[HINT: it looks like the "do" here does not have a matching "end"] + + assert hint_diagnostic.position == 3 + end + + @feature_condition details_in_context?: true + @tag execute_if(@feature_condition) + test "returns the approximate correct location when there is a hint and #{inspect(@feature_condition)}" do + diagnostics = ~S[ + defmodule Foo do + def bar_missing_end do + :ok + + def bar do + :ok + end + end] |> compile() |> diagnostics() + + [start_diagnostic, end_diagnostic, hint_message] = diagnostics # TODO: I think we should remove the `hint` - # assert end_message.message == ~S[missing terminator: end (for "do" starting at line 2)] - assert end_message.position == {9, 12} + # assert end_diagnostic.message == ~S[missing terminator: end (for "do" starting at line 2)] + assert end_diagnostic.position == {9, 12} - assert start_message.message == ~S[The "do" here is missing terminator "end"] - assert start_message.position in [2, {2, 23, 2, 25}] + assert start_diagnostic.message == ~S[The "do" here is missing terminator "end"] + assert start_diagnostic.position == {2, 23, 2, 25} assert hint_message.message == ~S[HINT: it looks like the "do" here does not have a matching "end"] @@ -153,6 +270,8 @@ defmodule Lexical.RemoteControl.Build.ParseErrorTest do assert hint_message.position == 3 end + @feature_condition details_in_context?: false + @tag execute_if(@feature_condition) test "returns the last approximate correct location when there are multiple missing" do diagnostics = ~S[ defmodule Foo do @@ -166,13 +285,42 @@ defmodule Lexical.RemoteControl.Build.ParseErrorTest do end end] |> compile() |> diagnostics() - [end_message, start_message, hint_message] = diagnostics + [start_diagnostic, hint_diagnostic, end_diagnostic] = diagnostics + + # assert end_diagnostic.message == ~S[missing terminator: end (for "do" starting at line 3)] + assert end_diagnostic.position == {11, 12} + + assert start_diagnostic.message == ~S[The "do" here is missing terminator "end"] + assert start_diagnostic.position == 3 + + assert hint_diagnostic.message == + ~S[HINT: it looks like the "do" here does not have a matching "end"] + + assert hint_diagnostic.position == 6 + end + + @feature_condition details_in_context?: true + @tag execute_if(@feature_condition) + test "returns the last approximate correct location when there are multiple missing and #{inspect(@feature_condition)}" do + diagnostics = ~S[ + defmodule Foo do + def bar_missing_end do + :ok + + def bar_missing_end2 do + + def bar do + :ok + end + end] |> compile() |> diagnostics() + + [start_diagnostic, end_diagnostic, hint_message] = diagnostics - # assert end_message.message == ~S[missing terminator: end (for "do" starting at line 3)] - assert end_message.position == {11, 12} + # assert end_diagnostic.message == ~S[missing terminator: end (for "do" starting at line 3)] + assert end_diagnostic.position == {11, 12} - assert start_message.message == ~S[The "do" here is missing terminator "end"] - assert start_message.position in [3, {3, 31, 3, 33}] + assert start_diagnostic.message == ~S[The "do" here is missing terminator "end"] + assert start_diagnostic.position == {3, 31, 3, 33} assert hint_message.message == ~S[HINT: it looks like the "do" here does not have a matching "end"] diff --git a/apps/remote_control/test/lexical/remote_control/build_test.exs b/apps/remote_control/test/lexical/remote_control/build_test.exs index 8d1bd0279..1525f36f2 100644 --- a/apps/remote_control/test/lexical/remote_control/build_test.exs +++ b/apps/remote_control/test/lexical/remote_control/build_test.exs @@ -10,6 +10,7 @@ defmodule Lexical.BuildTest do import Messages import Lexical.Test.Fixtures + import Lexical.Test.DiagnosticSupport use ExUnit.Case use Patch @@ -124,6 +125,8 @@ defmodule Lexical.BuildTest do describe "compilng a project with parse errors" do setup :with_parse_errors_project + @feature_condition span_in_diagnostic?: false + @tag execute_if(@feature_condition) test "stuff", %{project: project} do Build.schedule_compile(project, true) @@ -133,6 +136,21 @@ defmodule Lexical.BuildTest do assert diagnostic.uri assert diagnostic.message =~ "SyntaxError" end + + @feature_condition span_in_diagnostic?: true + @tag execute_if(@feature_condition) + test "stuff when #{inspect(@feature_condition)}", %{project: project} do + Build.schedule_compile(project, true) + + assert_receive project_compiled(status: :error) + assert_receive project_diagnostics(diagnostics: [%Diagnostic.Result{} = diagnostic]) + + assert diagnostic.uri + assert diagnostic.position == {4, 24} + + assert diagnostic.message =~ + "** (MismatchedDelimiterError) mismatched delimiter found on lib/parse_errors.ex:15" + end end describe "when compiling a project that has warnings" do @@ -180,6 +198,8 @@ defmodule Lexical.BuildTest do assert diagnostic.position == {4, 15} end + @feature_condition details_in_context?: false + @tag execute_if(@feature_condition) test "handles missing token errors", %{project: project} do source = ~S[%{foo: 3] compile_document(project, source) @@ -247,6 +267,8 @@ defmodule Lexical.BuildTest do assert diagnostic.position == {3, 12} end + @feature_condition span_in_diagnostic?: false + @tag execute_if(@feature_condition) test "reports unused variables", %{project: project} do source = ~S[ defmodule WithWarnings do @@ -272,6 +294,8 @@ defmodule Lexical.BuildTest do end end + @feature_condition span_in_diagnostic?: false + @tag execute_if(@feature_condition) test "reports missing parens", %{project: project} do source = ~S[ defmodule WithWarnings do @@ -307,6 +331,8 @@ defmodule Lexical.BuildTest do end end + @feature_condition span_in_diagnostic?: false + @tag execute_if(@feature_condition) test "reports unused defp functions", %{project: project} do source = ~S[ defmodule UnusedDefp do @@ -326,6 +352,8 @@ defmodule Lexical.BuildTest do assert diagnostic.details == nil end + @feature_condition span_in_diagnostic?: false + @tag execute_if(@feature_condition) test "handles undefined usages", %{project: project} do source = ~S[ defmodule WithUndefinedFunction do @@ -346,6 +374,8 @@ defmodule Lexical.BuildTest do assert diagnostic.details == nil end + @feature_condition span_in_diagnostic?: false + @tag execute_if(@feature_condition) test "reports multiple errors", %{project: project} do source = ~S[ defmodule WithFiveErrors do @@ -360,13 +390,8 @@ defmodule Lexical.BuildTest do assert_receive file_compiled(status: :error) - if Features.with_diagnostics?() do - assert_receive file_diagnostics(diagnostics: [_, _, _] = diagnostics) - assert length(diagnostics) == 3 - else - assert_receive file_diagnostics(diagnostics: [_, _, _, _, _] = diagnostics) - assert length(diagnostics) == 5 - end + assert_receive file_diagnostics(diagnostics: [_, _, _] = diagnostics) + assert length(diagnostics) == 3 end test "adding a new module notifies the listener", %{project: project} do diff --git a/projects/lexical_test/lib/lexical/test/diagnostic_support.ex b/projects/lexical_test/lib/lexical/test/diagnostic_support.ex new file mode 100644 index 000000000..8ce5a7ea1 --- /dev/null +++ b/projects/lexical_test/lib/lexical/test/diagnostic_support.ex @@ -0,0 +1,14 @@ +defmodule Lexical.Test.DiagnosticSupport do + def execute_if(feature_condition) do + matched? = + Enum.all?(feature_condition, fn {feature_fn, value} -> + apply(Features, feature_fn, []) == value + end) + + if matched? do + :ok + else + :skip + end + end +end