esl / gradient

Gradient is a static typechecker for Elixir
Apache License 2.0
437 stars 13 forks source link

`mix gradient` fails with errors that seem to all be from Gradient #38

Closed dfalling closed 2 years ago

dfalling commented 2 years ago

I just installed Gradient and running it fails. Any suggestions to make this work?

❯ mix gradient
===> Analyzing applications...
===> Compiling gradualizer
src/gradualizer.erl:45: Warning: opaque type top() is underspecified and therefore meaningless

src/typechecker.erl:3234: Warning: function type_check_cons_in/4 is unused
src/typechecker.erl:3247: Warning: function type_check_cons_union/4 is unused
src/typechecker.erl:4612: Warning: function verbose/3 is unused
src/typechecker.erl:4870: Warning: function gen_partition/3 is unused
src/typechecker.erl:4872: Warning: function paux/3 is unused

==> gradient
Compiling 14 files (.ex)
Generated gradient app
==> ido
Generated ido app
Typechecking files...
** (MatchError) no match of right hand side value: :error
    (gradient 0.1.0) lib/gradient/ast_specifier.ex:194: Gradient.AstSpecifier.mapper/3
    (gradient 0.1.0) lib/gradient/ast_specifier.ex:122: Gradient.AstSpecifier.context_mapper_fold/4
    (gradient 0.1.0) lib/gradient/ast_specifier.ex:148: Gradient.AstSpecifier.mapper/3
    (gradient 0.1.0) lib/gradient/ast_specifier.ex:122: Gradient.AstSpecifier.context_mapper_fold/4
    (gradient 0.1.0) lib/gradient/ast_specifier.ex:123: Gradient.AstSpecifier.context_mapper_fold/4
    (gradient 0.1.0) lib/gradient/ast_specifier.ex:93: Gradient.AstSpecifier.run_mappers/2
    (gradient 0.1.0) lib/gradient.ex:26: Gradient.type_check_file/2
    (elixir 1.13.1) lib/enum.ex:1593: Enum."-map/2-lists^map/1-0-"/2
    (elixir 1.13.1) lib/enum.ex:1593: Enum."-map/2-lists^map/1-0-"/2
    (elixir 1.13.1) lib/enum.ex:1597: anonymous fn/3 in Enum.map/2
    (stdlib 3.13.2) maps.erl:233: :maps.fold_1/3
    (elixir 1.13.1) lib/enum.ex:2408: Enum.map/2
    (gradient 0.1.0) lib/mix/tasks/gradient.ex:20: Mix.Tasks.Gradient.run/1
    (mix 1.13.1) lib/mix/task.ex:397: anonymous fn/3 in Mix.Task.run_task/3
    (mix 1.13.1) lib/mix/cli.ex:84: Mix.CLI.run_task/2
erszcz commented 2 years ago

Hi, @dfalling!

Thanks for the report. Could you share a link to the code you're experiencing the problem on? Or a minimised example if the code is not open source?

dfalling commented 2 years ago

It's not open source, but I'm happy to send an invite to the codebase if you'd like.

erszcz commented 2 years ago

@dfalling I'd be happy to take a look, please send an invite. Fingers crossed we'll be able to extract a minimal example which leads to the crash.

dfalling commented 2 years ago

@dfalling I'd be happy to take a look, please send an invite. Fingers crossed we'll be able to extract a minimal example which leads to the crash.

Thanks! Invite sent.

erszcz commented 2 years ago

Thanks for the invite, @dfalling!

For the record, here's more precise info:

$ cat .tool-versions
elixir 1.13.1
erlang 23.1.4
$ git l -1
f8af28f 2022-02-09 Enforce struct types in functions (#819)

Gradient patch to get more detailed error info:

$ git diff
diff --git a/lib/gradient/ast_specifier.ex b/lib/gradient/ast_specifier.ex
index 9f444fe..e67dc01 100644
--- a/lib/gradient/ast_specifier.ex
+++ b/lib/gradient/ast_specifier.ex
@@ -755,8 +755,8 @@ defmodule Gradient.AstSpecifier do
             anno = :erl_anno.set_line(line, anno)
             {:ok, line, anno, opts, false}

-          err ->
-            err
+          :error ->
+            {:not_found, anno, opts}
         end

       line ->

The error:

$ mix gradient
Generated ido app
Typechecking files...
** (MatchError) no match of right hand side value: {:not_found, [generated: true, location: 0], [end_line: 2]}
    (gradient 0.1.0) lib/gradient/ast_specifier.ex:194: Gradient.AstSpecifier.mapper/3
    (gradient 0.1.0) lib/gradient/ast_specifier.ex:122: Gradient.AstSpecifier.context_mapper_fold/4
    (gradient 0.1.0) lib/gradient/ast_specifier.ex:148: Gradient.AstSpecifier.mapper/3
    (gradient 0.1.0) lib/gradient/ast_specifier.ex:122: Gradient.AstSpecifier.context_mapper_fold/4
    (gradient 0.1.0) lib/gradient/ast_specifier.ex:123: Gradient.AstSpecifier.context_mapper_fold/4
    (gradient 0.1.0) lib/gradient/ast_specifier.ex:93: Gradient.AstSpecifier.run_mappers/2
    (gradient 0.1.0) lib/gradient.ex:26: Gradient.type_check_file/2
    (elixir 1.13.1) lib/enum.ex:1593: Enum."-map/2-lists^map/1-0-"/2
    (elixir 1.13.1) lib/enum.ex:1593: Enum."-map/2-lists^map/1-0-"/2
    (elixir 1.13.1) lib/enum.ex:1597: anonymous fn/3 in Enum.map/2
    (stdlib 3.13.2) maps.erl:233: :maps.fold_1/3
    (elixir 1.13.1) lib/enum.ex:2408: Enum.map/2
    (gradient 0.1.0) lib/mix/tasks/gradient.ex:20: Mix.Tasks.Gradient.run/1
    (mix 1.13.1) lib/mix/task.ex:397: anonymous fn/3 in Mix.Task.run_task/3
    (mix 1.13.1) lib/mix/cli.ex:84: Mix.CLI.run_task/2

So it seems that Gradient is assuming that :line should be present in opts, but it's not. @Premwoik could it stem from a specific Erlang x Elixir version match? I can see a comment just above line 194 in Gradient which states: # anno has line, yet it doesn't seem to be the case 🤔

Premwoik commented 2 years ago

@erszcz Yea, It seems that some generated clauses don't contain the location. I had never observed it before and thought that a clause always contains line info. It would be nice to find out the Elixir construct that generates such a clause.

Maybe we should write the name of the currently checked file? It will be easier to localize the code that produces the error.

Premwoik commented 2 years ago

The use Pow.Ecto.Schema macro injects __impl__ function that has generated clauses without lines that resulted in an unexpected error. Fixed in #39 by propagating a missing line from the parent function form.

defmodule TypedSchemaTest do
  @moduledoc false

  use TypedEctoSchema

  use Pow.Ecto.Schema,
    user_id_field: :email

  typed_schema "users" do
    pow_user_fields()
  end
end
dfalling commented 2 years ago

Awesome, thank you both! That fixed it for me.

erszcz commented 2 years ago

@dfalling You were faster closing this than I was typing ;) I've run a local copy of Gradient with #40 merged in on my machine. Once #40 lands, you should be able to replicate this yourself. I'll comment here on some of the results.

There are plenty of diagnostics like this one (project name substituted with project):

lib/project/auth.ex: The pattern %Project.Accounts.User{role: "admin"} on line 23 doesn't have the type any()

These stem from functions with no specs using pattern matching in their heads. In other words, Gradient assumes argument type any() if there's no spec. The function matches on %User{role: "admin"} as one of the params - this obviously isn't of type any(). Therefore, we get the above warning. This is a common pattern both in Erlang and Elixir, so I imagine inference of such more specific specs would be handy, but currently the only way to get rid of them is to add specs to such functions.

Next one, just a few appearances:

lib/project/auth.ex: The variable on line 325 is expected to have type String.t() but it has type nil | binary()
323     # pattern matching below to ensure non-empty set of cose_keys
324     with {:ok, %User{:id => user_id}} <- Accounts.get_user_by_email(email),
325*         [_ | _] = cose_keys <- list_user_cose_keys(user_id) do
326       result =
327         Enum.map(cose_keys, fn %CoseKey{:key_id => key_id, :cose_key => cose_key} ->

user_id matched on line 324 is not guaranteed to be a String.t as it might also be a nil. This warning is correct, though it does not play completely well with the with construct, since I see there's also a corresponding else handling this.

Next one:

lib/project/elements/site_parsers/generic.ex: The variable on line 30 is expected to have type String.t() but it has type binary() | {:comment, String.t()} | {:doctype, String.t(), String.t(), String.t()} | {:pi, String.t(), list(html_attribute())} | {String.t(), list(html_attribute()), list(html_node())}
28       nil ->
29         case Floki.find(document, "title") do
30*          [{"title", [], [title]}] -> title
31           _ -> "Unknown page"
32         end

This is a valid warning. The enclosing function's spec declares to return a String.t, but title is not guaranteed to be a String.t (according to the available type specs).

Another one:

lib/project_web/ensure_role_plug.ex: The variable on line 23 is expected to have type atom() but it has type list({key(), value()})
21   @doc false
22   @spec init(Config.t()) :: atom()
23*  def init(config), do: config
24
25   @doc false

This one is also valid - there's a mistake in the spec, it should declare returning Config.t().

I have no idea how to explain this one:

lib/project_web/views/error_view.ex: The map on line 16 is expected to have type no_return() but it has type %{required(:errors) => %{optional(:message) => any()}}
14     message = Phoenix.Controller.status_message_from_template(template)
15
16     %{errors: %{message: message}}
17   end
18

But I suspect some macro magic 😆

Lastly, there is a lot of:

lib/project_web/controllers/user_controller.ex: Timeout checking function index/2 on line 14

Which are caused by specs "too hard" (lots more details is available in other tickets here and in Gradualizer) to currently type check successfully. This means homework for us :)

All in all, thanks for interest in Gradient and creating the issue!

P.S. Thanks for sharing access to the repo - since we have a minimised example of the original issue I'm deleting it from my machine. @dfalling, please revoke my access, too.