WhatsApp / eqwalizer

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

False positive on map update #45

Open MarkoMin opened 10 months ago

MarkoMin commented 10 months ago

I've noticed what I believe is false positive and shrank it down to the minimal example:

-module(lib1).

-type error_map() :: #{binary() => binary()}.
-type elem() :: #{id:=binary(), elements:=list(elem())} |
                #{id:=binary(), value:=term(), error:=binary()}.

-typing([eqwalizer]).

-export([embedd_error/2]).

-spec embedd_error(error_map(), elem()) -> elem().
embedd_error(Errs, #{id:=_Id, elements:=Elems}=Group) ->
    Group#{elements=>
            lists:map(fun (Elem) ->
                        embedd_error(Errs, Elem)
                    end, Elems)};
embedd_error(Errs, #{id:=Id}=ReqItem) ->
    Err = maps:get(Id, Errs),
    ReqItem#{error => Err}.

Program goes through tree-like struct and places error in each leaf node. When I eqwalize it, I get the following error message:

error: incompatible_types
   ┌─ src/lib1.erl:13:5
   │    
13 │ ╭ ╭     Group#{elements=>
14 │ │ │             lists:map(fun (Elem) ->
15 │ │ │                         embedd_error(Errs, Elem)
16 │ │ │                     end, Elems)};
   │ ╰─│────────────────────────────────^ ..#{..}.
Expression has type:   #S{elements := [elem()], id := binary()} | #S{elements := [elem()], error := binary(), id := binary(), value := term()}
Context expected type: elem()

If Group is of type elem() and I update one of the field that already exist in it, should the expression remain of type elem()?

I use OTP 26.1, latest eqwalizer (ref "41869ab6eb30e9ff34545fe5f9c0a4caa797cfc2") and elp 1.1.0+build-2023-08-31

VLanvin commented 10 months ago

Yes this is a false positive, thank you for reporting it.

This is a known issue in eqWAlizer's handling of pattern matching: it currently does not refine the type of variables bound to a pattern using Pat = Var, so here, the type of Group is not properly refined to the case #{id:=binary(), elements:=list(elem())}.

Our current implementation of occurrence typing makes it non-trivial to support, but we're exploring possible solutions.