Skip to content

Commit

Permalink
Support Elixir 1.15 (#261)
Browse files Browse the repository at this point in the history
* Support as-you-type compiling on Elixir 1.15

Starting with Elixir 1.15, we don't need to capture io for compiling individual files or quoted anymore,
because with_diagnostics returns all compile-time diagnostics.

So I have implemented two ways to compile and process the compiled results.

You can see from the tests that the location of the undefined diagnostic is more precise,
and I've also enhanced the unused and undefined messages to make them more concise and clear.
  • Loading branch information
scottming authored Jul 21, 2023
1 parent 8b084d1 commit 6afbc75
Show file tree
Hide file tree
Showing 16 changed files with 380 additions and 193 deletions.
2 changes: 1 addition & 1 deletion .credo.exs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
name: "default",
files: %{
included: ["lib/", "src/", "web/", "apps/"],
excluded: ["apps/remote_control/test/fixtures/**/*.ex", "apps/common/lib/elixir/lib/future/**/*.ex"]
excluded: ["apps/remote_control/test/fixtures/**/*.ex", "apps/common/lib/future/**/*.ex"]
},
plugins: [],
requires: [],
Expand Down
2 changes: 2 additions & 0 deletions .github/workflows/elixir.yml
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,8 @@ jobs:
# and running the workflow steps.
matrix:
include:
- elixir: '1.15.3-otp-25'
otp: '25.3'
- elixir: '1.14.5-otp-25'
otp: '25.3'
- elixir: '1.13.4-otp-25'
Expand Down
4 changes: 0 additions & 4 deletions .tool-versions

This file was deleted.

9 changes: 9 additions & 0 deletions apps/common/lib/elixir/features.ex
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
defmodule Elixir.Features do
def with_diagnostics? do
Version.match?(System.version(), ">= 1.15.3")
end

def compile_wont_change_directory? do
Version.match?(System.version(), ">= 1.15.0")
end
end
File renamed without changes.
File renamed without changes.
File renamed without changes.
2 changes: 0 additions & 2 deletions apps/remote_control/.tool-versions

This file was deleted.

118 changes: 103 additions & 15 deletions apps/remote_control/lib/lexical/remote_control/build/error.ex
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,27 @@ defmodule Lexical.RemoteControl.Build.Error do

@elixir_source "Elixir"

def normalize_diagnostic(%Compiler.Diagnostic{} = diagnostic) do
@doc """
Diagnostics can come from compiling the whole project,
from compiling individual files, or from erlang's diagnostics (Code.with_diagnostics since elixir1.15),
so we need to do some post-processing.
Includes:
1. Normalize each one to the standard result
2. Format the message to make it readable in the editor
3. Remove duplicate messages on the same line
"""
def refine_diagnostics(diagnostics) do
diagnostics
|> Enum.map(fn diagnostic ->
diagnostic
|> normalize()
|> format()
end)
|> uniq()
end

defp normalize(%Compiler.Diagnostic{} = diagnostic) do
Result.new(
diagnostic.file,
diagnostic.position,
Expand All @@ -17,6 +37,65 @@ defmodule Lexical.RemoteControl.Build.Error do
)
end

defp normalize(%Result{} = result) do
result
end

defp format(%Result{} = result) do
%Result{result | message: format_message(result.message)}
end

@undefined_function_pattern ~r/ \(expected ([A-Za-z0-9_\.]*) to [^\)]+\)/

defp format_message("undefined" <> _ = message) do
# All undefined function messages explain the *same* thing inside the parentheses
String.replace(message, @undefined_function_pattern, "")
end

defp format_message(message) when is_binary(message) do
maybe_format_unused(message)
end

defp maybe_format_unused(message) do
# Same reason as the `undefined` message above, we can remove the things in parentheses
case String.split(message, "is unused (", parts: 2) do
[prefix, _] ->
prefix <> "is unused"

_ ->
message
end
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

defp uniq(diagnostics) do
# We need to uniq by position because the same position can be reported
# and the `end_line_diagnostic` is always the precise one
extract_line = fn
%Result{position: {line, _column}} -> line
%Result{position: {start_line, _start_col, _end_line, _end_col}} -> start_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}

diagnostics
|> Enum.sort_by(extract_line_and_severity)
|> Enum.uniq_by(extract_line)
|> reject_zeroth_line()
end

# Parse errors happen during Code.string_to_quoted and are raised as SyntaxErrors, and TokenMissingErrors.
def parse_error_to_diagnostics(
%Document{} = source,
Expand Down Expand Up @@ -49,21 +128,16 @@ defmodule Lexical.RemoteControl.Build.Error do
)
end

defp uniq(diagnostics) do
# We need to uniq by position because the same position can be reported
# and the `end_line_diagnostic` is always the precise one
extract_line = fn
%Result{position: {line, _column}} -> line
%Result{position: {start_line, _start_col, _end_line, _end_col}} -> start_line
%Result{position: line} when is_integer(line) -> line
end

Enum.uniq_by(diagnostics, extract_line)
end

defp build_end_line_diagnostics(%Document{} = source, context, message_info, token) do
[end_line_message | _] = String.split(message_info, "\n")
message = "#{end_line_message}#{token}"

message =
if String.ends_with?(end_line_message, token) do
end_line_message
else
end_line_message <> token
end

diagnostic = Result.new(source.uri, context_to_position(context), message, :error, "Elixir")
[diagnostic]
end
Expand Down Expand Up @@ -96,6 +170,20 @@ defmodule Lexical.RemoteControl.Build.Error do
end
end

@doc """
The `diagnostics_from_mix/2` is only for Elixir version > 1.15
From 1.15 onwards with_diagnostics can return some compile-time errors,
more details: https://github.com/elixir-lang/elixir/pull/12742
"""
def diagnostics_from_mix(%Document{} = doc, all_errors_and_warnings)
when is_list(all_errors_and_warnings) do
for error_or_wanning <- all_errors_and_warnings do
%{position: position, message: message, severity: severity} = error_or_wanning
Result.new(doc.uri, position, message, severity, @elixir_source)
end
end

def error_to_diagnostic(
%Document{} = source,
%CompileError{} = compile_error,
Expand Down Expand Up @@ -179,7 +267,7 @@ defmodule Lexical.RemoteControl.Build.Error do
[{_, _, _, context}, {_, call, _, second_to_last_context} | _] = reversed_stack

pipe_or_struct? = call in [:|>, :__struct__]
expanding_macro? = second_to_last_context[:file] == 'expanding macro'
expanding_macro? = second_to_last_context[:file] == ~c"expanding macro"
message = Exception.message(argument_error)

position =
Expand Down
138 changes: 138 additions & 0 deletions apps/remote_control/lib/lexical/remote_control/build/file.ex
Original file line number Diff line number Diff line change
@@ -0,0 +1,138 @@
defmodule Lexical.RemoteControl.Build.File do
alias Elixir.Features
alias Lexical.Document
alias Lexical.RemoteControl.Build
alias Lexical.RemoteControl.ModuleMappings

import Lexical.RemoteControl.Build.CaptureIO, only: [capture_io: 2]

def compile(%Document{} = document) do
case to_quoted(document) do
{:ok, quoted} ->
prepare_compile(document.path)

if Features.with_diagnostics?() do
do_compile(quoted, document)
else
do_compile_and_capture_io(quoted, document)
end

{:error, {meta, message_info, token}} ->
diagnostics = Build.Error.parse_error_to_diagnostics(document, meta, message_info, token)
{:error, diagnostics}
end
end

defp to_quoted(document) do
source_string = Document.to_string(document)
parser_options = [file: document.path] ++ parser_options()
Code.put_compiler_option(:ignore_module_conflict, true)
Code.string_to_quoted(source_string, parser_options)
end

defp do_compile(quoted_ast, document) do
old_modules = ModuleMappings.modules_in_file(document.path)

case compile_quoted_with_diagnostics(quoted_ast, document.path) do
{{:ok, modules}, []} ->
purge_removed_modules(old_modules, modules)
{:ok, []}

{{:ok, modules}, all_errors_and_warnings} ->
purge_removed_modules(old_modules, modules)

diagnostics =
document
|> Build.Error.diagnostics_from_mix(all_errors_and_warnings)
|> Build.Error.refine_diagnostics()

{:ok, diagnostics}

{{:exception, exception, stack, quoted_ast}, all_errors_and_warnings} ->
converted = Build.Error.error_to_diagnostic(document, exception, stack, quoted_ast)
maybe_diagnostics = Build.Error.diagnostics_from_mix(document, all_errors_and_warnings)

diagnostics =
[converted | maybe_diagnostics]
|> Enum.reverse()
|> Build.Error.refine_diagnostics()

{:error, diagnostics}
end
end

defp do_compile_and_capture_io(quoted_ast, document) do
# credo:disable-for-next-line Credo.Check.Design.TagTODO
# TODO: remove this function once we drop support for Elixir 1.14
old_modules = ModuleMappings.modules_in_file(document.path)
compile = fn -> safe_compile_quoted(quoted_ast, document.path) end

case capture_io(:stderr, compile) do
{captured_messages, {:error, {:exception, {exception, _inner_stack}, stack}}} ->
error = Build.Error.error_to_diagnostic(document, exception, stack, [])
diagnostics = Build.Error.message_to_diagnostic(document, captured_messages)

{:error, [error | diagnostics]}

{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)

{:error, [error | diagnostics]}

{"", {:ok, modules}} ->
purge_removed_modules(old_modules, modules)
{:ok, []}

{captured_warnings, {:ok, modules}} ->
purge_removed_modules(old_modules, modules)
diagnostics = Build.Error.message_to_diagnostic(document, captured_warnings)
{:ok, diagnostics}
end
end

defp prepare_compile(path) do
# If we're compiling a mix.exs file, the after compile callback from
# `use Mix.Project` will blow up if we add the same project to the project stack
# twice. Preemptively popping it prevents that error from occurring.
if Path.basename(path) == "mix.exs" do
Mix.ProjectStack.pop()
end

Mix.Task.run(:loadconfig)
end

@dialyzer {:nowarn_function, compile_quoted_with_diagnostics: 2}

defp compile_quoted_with_diagnostics(quoted_ast, path) do
Code.with_diagnostics(fn ->
safe_compile_quoted(quoted_ast, path)
end)
end

defp safe_compile_quoted(quoted_ast, path) do
try do
{:ok, Code.compile_quoted(quoted_ast, path)}
rescue
exception ->
{filled_exception, stack} = Exception.blame(:error, exception, __STACKTRACE__)
{:exception, filled_exception, stack, quoted_ast}
end
end

defp purge_removed_modules(old_modules, new_modules) do
new_modules = MapSet.new(new_modules, fn {module, _bytecode} -> module end)
old_modules = MapSet.new(old_modules)

old_modules
|> MapSet.difference(new_modules)
|> Enum.each(fn to_remove ->
:code.purge(to_remove)
:code.delete(to_remove)
end)
end

defp parser_options do
[columns: true, token_metadata: true]
end
end
Loading

0 comments on commit 6afbc75

Please sign in to comment.