WhatsApp / eqwalizer

A type-checker for Erlang
Apache License 2.0
513 stars 28 forks source link

Wrong incompatible type error reported #62

Closed Zabrane closed 2 months ago

Zabrane commented 2 months ago

Hi guys

Eqwalizer is reporting an error which doesn't seem to be correct IMHO:

Filter = {fun logger_filters:domain/2, {log,  equal, [selenium]}}, %% <--- eqwalizer reported an error here
ok = logger:add_handler_filter(default, ftp_logger, Filter),

Screenshot 2024-08-26 at 21 08 56 Screenshot 2024-08-26 at 21 09 04

But according to the Erlang official doc, the call is legit:

> logger:set_handler_config(h1, filter_default, log). % this is the default
> Filter = {fun logger_filters:domain/2, {stop, sub, [otp, sasl]}}.
> logger:add_handler_filter(h1, no_sasl, Filter).
ok

My config:

$ sw_vers | head -2
ProductName:            macOS
ProductVersion:         14.6.1

$ erl
Erlang/OTP 27 [erts-15.0.1] [source] [64-bit] [smp:16:16] [ds:16:16:10] [async-threads:1] [jit:ns] [lock-counting]
Eshell V15.0.1 (press Ctrl+G to abort, type help(). for help)

$ elp version
elp 1.1.0+build-2024-04-17

%% VSCode
$ code --version
1.92.2
fee1edb8d6d72a0ddff41e5f71a671c23ed924b9
x64

Help appreciated. Many thanks

ilya-klyuchnikov commented 2 months ago

Type-wise (wrt types and specs) the type error is correct.

Since

-type filter() :: {fun((log_event(),filter_arg()) -> filter_return()),   filter_arg()}.
-type filter_arg() :: term().

See

https://github.com/erlang/otp/blob/5fa0be7feca7738e8d91615a56b00468cfa2747f/lib/kernel/src/logger.erl#L259-L260

The function fun logger_filters:domain/2 has the spec

-spec domain(LogEvent,Extra) -> logger:filter_return() when
      LogEvent :: logger:log_event(),
      Extra :: {Action,Compare,MatchDomain},
      Action :: log | stop,
      Compare :: super | sub | equal | not_equal | undefined,
      MatchDomain :: list(atom()).

https://github.com/erlang/otp/blob/5fa0be7feca7738e8d91615a56b00468cfa2747f/lib/kernel/src/logger_filters.erl#L113-L118

So, fun logger_filters:domain/2 is not a subtype of fun((log_event(),filter_arg()) -> filter_return()) which is expected.

(Note that sutyping of functions is contravariant in parameters)

michalmuskala commented 2 months ago

FWIW I think that the type filter should be defined as:

-type filter() :: {fun(log_event(), Arg) -> filter_return()), Arg}.

but AFAIK this is not supported by specs today

ilya-klyuchnikov commented 2 months ago

I think it could be supported if written directly as part of the spec (without the filter alias). Something like:

-spec add_handler_filter(HandlerId,FilterId,Filter) -> ok | {error,term()} when
      HandlerId :: handler_id(),
      FilterId :: filter_id(),
      Filter :: {fun((log_event(), Arg) -> filter_return()), Arg}.
Zabrane commented 2 months ago

@ilya-klyuchnikov @michalmuskala Thanks for the feedback. What should I do to clear this error in my code then?

ilya-klyuchnikov commented 2 months ago

What should I do to clear this error in my code then?

You can use an ignore escape hatch in a comment. Something like.

% eqwalizer:ignore bad add_handler_filter spec
ok = logger:add_handler_filter(default, ftp_logger, Filter),
Zabrane commented 2 months ago

What should I do to clear this error in my code then?

You can use an ignore escape hatch in a comment. Something like.

% eqwalizer:ignore bad add_handler_filter spec
ok = logger:add_handler_filter(default, ftp_logger, Filter),

@ilya-klyuchnikov it worked. Many thanks.