Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Compiler diagnostics originating from typesystem are incomplete #13646

Closed
lukaszsamson opened this issue Jun 6, 2024 · 5 comments
Closed

Compiler diagnostics originating from typesystem are incomplete #13646

lukaszsamson opened this issue Jun 6, 2024 · 5 comments

Comments

@lukaszsamson
Copy link
Contributor

lukaszsamson commented Jun 6, 2024

Elixir and Erlang/OTP versions

Erlang/OTP 27 [erts-15.0] [source] [64-bit] [smp:12:12] [ds:12:12:10] [async-threads:1] [jit]

Elixir 1.17.0-rc.0 (db89cbf) (compiled with Erlang/OTP 27)

Operating system

any

Current behavior

Given this code

defmodule Foo do
  defstruct asd: nil
end

defmodule Bar do
  a = %Foo{}
  @asd a

  def s do
    @asd.ccc
  end
end

The emitted diagnostic has truncated message and no details

%Mix.Task.Compiler.Diagnostic{
    file: "/Users/foo/lib/some.ex",
    source: "/Users/foo/lib/some.ex",
    severity: :warning,
    message: "unknown key .ccc in expression:\n\n    %{__struct__: Foo, asd: nil}.ccc\n\nthe given type does not have the given key:\n\n    %Foo{asd: nil}\n\ntyping violation found at:",
    position: {10, 10},
    compiler_name: "Elixir",
    span: nil,
    details: nil,
    stacktrace: [
      {Bar, :s, 0,
       [
         file: ~c"lib/some.ex",
         end_of_expression: [newlines: 1, line: 10, column: 13],
         no_parens: true,
         line: 10,
         column: 10
       ]}
    ]
  }

Whereas the same code emits this in terminal

    warning: unknown key .ccc in expression:

        %{__struct__: Foo, asd: nil}.ccc

    the given type does not have the given key:

        %Foo{asd: nil}

    typing violation found at:
    │
 10 │     @asd.ccc
    │          ~
    │
    └─ lib/some.ex:10:10: Bar.s/0

Expected behavior

Message not truncated, detail filled, possibly span filled

@josevalim
Copy link
Member

Thank you!

The message is not truncated, we don't include the snippet because we expect you to show it. I think however it can be confusing. Perhaps we should end the message by saying "typing violation found"?

We could compute the spam though, that would benefit both the console warning and the terminal one.

And what do you expect to be in detail?

@lukaszsamson
Copy link
Contributor Author

we don't include the snippet because we expect you to show it

I don't think I follow. This is clearly different than TokenMissingError etc. where snippets are returned. I don't insist that the snippet is needed. Position/span is sufficient for LSP

And what do you expect to be in detail?

Other errors have exception in detail. I would expect to find some structured info about the type problem:

  • positions where conflicting types are defined
  • the conflicting types as data

In LSP Diagnostic besides the main position/span (range property) and message, there is a collection relatedInformation which can contain additional positions and messages. It's possible to build diagnostics that reference type definitions in another files. This is an example from elixir-ls
Screenshot 2024-06-06 at 22 02 39

@josevalim
Copy link
Member

This is clearly different than TokenMissingError etc. where snippets are returned. I don't insist that the snippet is needed.

I think that's rather a flaw in TokenMissingError. I don't think we include it anywhere else. If you do IO.warn("hello", __ENV__) in the module body, does the diagnostic have the snippet as well or not really?

Other errors have exception in detail. I would expect to find some structured info about the type problem:

Got it. I will see what we can expose. Perhaps we can expose the traces.

@lukaszsamson
Copy link
Contributor Author

does the diagnostic have the snippet as well or not really?

No, only some of them do. The changes to include snippets were introduced in 1.16. In fact they do look ugly if the UI displaying it is not using a monospaced font (and there is no markdown support in diagnostic messages but that's VSCode issue microsoft/vscode#54272)

Screenshot 2024-06-09 at 08 09 42 Screenshot 2024-06-09 at 08 10 20 Screenshot 2024-06-09 at 08 11 03

josevalim added a commit that referenced this issue Jun 10, 2024
@josevalim
Copy link
Member

josevalim commented Jun 10, 2024

For completeness, the reason why syntax error, token missing error, etc include the snippets is because they are exceptions. You can probably intercept those exceptions and render something without the snippets if desired.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

2 participants