WhatsApp / eqwalizer

A type-checker for Erlang
Apache License 2.0
507 stars 27 forks source link

Incompatible type issue headache #64

Closed Zabrane closed 1 week ago

Zabrane commented 1 week ago

Hi guys

I'm facing types incompatibility on a very simple example which I couldn't solve. Here is my code:

-spec table_name() -> atom().
table_name() ->
    {ok, V} = application:get_env(?MODULE, table_name),
    V.

eqwalizer complains about term() non compatible with atom():

`V`.
Expression has type:   term()
Context expected type: atom()
        See https://fb.me/eqwalizer_errors#incompatible_types

I've changed my code forcing it to always return an atom(), or to fail:

-spec table_name() -> atom() | no_return().
table_name() ->
    {ok, V} = application:get_env(?MODULE,table_name),
    true == is_atom(V),
    V.

but eqwalizer still complains:

`V`.
Expression has type:   term()
Context expected type: atom() | none()
  term() is not compatible with atom() | none()
  because
  term() is not compatible with atom()
        See https://fb.me/eqwalizer_errors#incompatible_types

How am i supposed to solve this? Help appreciated. Thanks.

$ elp version
elp 1.1.0+local

$ erl
Eshell V15.0.1 (press Ctrl+G to abort, type help(). for help)

$ sw_vers | head -2
ProductName:        macOS
ProductVersion:     14.6.1
Zabrane commented 1 week ago

My bad, this solves it.

-spec table_name() -> atom() | no_return().
table_name() ->
    {ok, V} = application:get_env(?MODULE,table_name),
    true = is_atom(V),
    V.

Is there a better solution?

Zabrane commented 1 week ago

Hey @michalmuskala @ilya-klyuchnikov

Let's take another example which i don't know how to fix by slightly changing the typespec of the function above:

-spec table_name() -> file:filename_all().
table_name() ->
    {ok, V} = application:get_env(?MODULE,table_name),
    true = io_lib:latin1_char_list(V),
    %% true = io_lib:deep_latin1_char_list(V), % didn't make any difference
    filename:join("/tmp", V).

The error says:

`V`.
Expression has type:   [term()]
Context expected type: file:name_all()

  [term()] is not compatible with file:name_all()
  because
  [term()] is not compatible with string() | atom() | file:deep_list() | binary()
  because
  [term()] is not compatible with string()
        See https://fb.me/eqwalizer_errors#incompatible_types

How to convince eqwalizer that V is of type string() ? I'm unable to solve this with my limited knowledge of eqwalizer.

VLanvin commented 1 week ago

Have you added the eqwalizer_support app to your project? It should be overriding the spec of application:get_env/2 to return {ok, dynamic()} instead of {ok, term()}, which would solve the issue.

Otherwise it's practically impossible to refine type term() to complex types such as file:name_all(). The solution would be to simply ignore the error:

-spec table_name() -> file:filename_all().
table_name() ->
    {ok, V} = application:get_env(?MODULE,table_name),
    % eqwalizer:ignore
    filename:join("/tmp", V).
Zabrane commented 1 week ago

Have you added the eqwalizer_support app to your project? It should be overriding the spec of application:get_env/2 to return {ok, dynamic()} instead of {ok, term()}, which would solve the issue.

@ilya-klyuchnikov In my rebar.config i had this:

{deps, [
  {eqwalizer_support,
    {git_subdir,
        "https://github.com/whatsapp/eqwalizer.git",
        {branch, "main"},
        "eqwalizer_rebar3"}}
]}.

I changed to this and the issue has gone:

{deps, [
  {eqwalizer_support,
    {git_subdir,
        "https://github.com/whatsapp/eqwalizer.git",
        {branch, "main"},
        "eqwalizer_support"}}
]}.

What's the difference between the two? Thanks

VLanvin commented 1 week ago

eqwalizer_support is an app dependency, it adds some type definitions and specs to eqWAlizer.

eqwalizer_rebar3 is a plugin for rebar3 to produce info for eqWAlizer, it is supposed to be added as a plugin, see https://github.com/WhatsApp/eqwalizer/blob/main/README.md.