rebar3 icon indicating copy to clipboard operation
rebar3 copied to clipboard

Dialyzer provider eats up `unknown` types warnings

Open ferd opened this issue 6 years ago • 12 comments

Environment

All rebar3 releases in a long long while

Current behaviour

Dialyzer since OTP-18 removed the unknown value out of the warnings by default. To keep a backwards-compatible behaviour, the provider removes the data from there if in pre-18, and leaves the option explicitly after the fact. We followed suit, and To get a similar behaviour, one must add:

{dialyzer, [{warnings, [unknown]}]}.

to their rebar.config file. The problem though, is that this was not a dialyzer long term view, it only happened in the programmatic API. The CLI tool still kept the warning, and we didn't breaking compatibility of behaviour and expectations.

Unfortunately, this happens to now be a pretty useful warning, and we have had user complaints that this was not turned out by default.

Even more unfortunately, this is an option that is not 'cancellable' (it does not take an {unknown, false} counter-value), so if it's put in the config by default, the user cannot easily take it out.

Expected behaviour

Since this type of analysis could stall and break entire builds, we do not want to turn it on by default as a surprise, and fail projects. A few options are available to us:

  • turn it on by default, but filter these out from causing a failure of the run, and add a warning saying it will be on by default eventually
  • turn it on by default, but remove it if any custom configuration is passed to rebar3. The caveat here is that if someone wants the default config except that one, they must set a new dialyzer config with another value in it, forced to be a default one. This is not intuitive
  • leave it off and risk worse analysis than the CLI tool.
  • turn it on and break builds

We have to make a choice and implement it.

ferd avatar Apr 18 '18 18:04 ferd

From my experience:

  1. if unknown is not set, it'll not be used,
  2. if it is set, it'll be used. (all good here).

Is the expected behavior to have it on by default? If so, there's a bug; otherwise I don't see the current issue happening.

paulo-ferraz-oliveira avatar Sep 14 '20 02:09 paulo-ferraz-oliveira

Yeah the idea would have been to turn it on by default. That being said, since the 3.14 release, relx can run xref analysis and detect these, so it's less urgent than it was and we'll likely table it for a future major release instead.

ferd avatar Sep 14 '20 13:09 ferd

👍, I didn't know xref to be capable of it (identifying unknown types). Are you solely referring to unknown for the undefined_functions bit, or did I miss something in the docs?

paulo-ferraz-oliveira avatar Sep 14 '20 14:09 paulo-ferraz-oliveira

Ah yeah you're right. The unknown types are broader than the relx stuff. Either way, we are not likely to change the default now and IIRC I had added a "workflow" section to the docs that mentions putting the unknown warnings in dialyzer in the meanwhile.

ferd avatar Sep 14 '20 14:09 ferd

😄

Either way, we are not likely to change the default now

I wasn't suggesting it (I'm sorry if my message was misleading in that sense - I know this is an "enhancement" Issue); I just wanted to understand the general direction of it.

I had added a "workflow" section to the docs that mentions putting the unknown warnings in dialyzer in the meanwhile.

Right you are (@http://rebar3.org/docs/workflow/):

{dialyzer, [
    {warnings, [
       %% Warn about undefined types and unknown functions
       unknown
    ]}
]}.

paulo-ferraz-oliveira avatar Sep 14 '20 14:09 paulo-ferraz-oliveira

Re:

"Either way, we are not likely to change the default now"

Command-line dialyzer reports unknown types and functions by default. Is there a reason for rebar3 to not report them, other than consistency with previous versions of rebar?

thanks for your work on this and for updating the docs, please feel free to ignore this question that comes from my curiosity.

mheiber avatar Dec 23 '20 10:12 mheiber

I'd like it enabled by default and to just break shit, hehe.

tsloughter avatar Dec 23 '20 14:12 tsloughter

According to the doc. -Wunknown reads

Let warnings about unknown functions and types affect the exit status of the command-line version.
The default is to ignore warnings about unknown functions and types when setting the exit status.
When using Dialyzer from Erlang, warnings about unknown functions and types are returned;
**the default is not to return these warnings**.

Either I mis-understood @mheiber's comment or we're talking about different Erlang/OTP versions.

paulo-ferraz-oliveira avatar Jan 04 '21 12:01 paulo-ferraz-oliveira

The command-line Dialyzer warns about unkown types. I just tested with dialyzer v4.2.

mheiber avatar Jan 04 '21 17:01 mheiber

The command-line Dialyzer warns about unkown types. I just tested with dialyzer v4.2.

Do you mean it does this by default?

Edit: yes, I see. You were talking about the actual warnings, I was mostly referring to the exit status, which is what the doc. mentions: "Let warnings (...) affect the exit status of the command-line (...)"

paulo-ferraz-oliveira avatar Jan 06 '21 20:01 paulo-ferraz-oliveira

Yes, it does it by default, as far as I can tell.

It is surprising (to me) that the behavior of the command line tool and dialyzer:run are so different.

On Wed, Jan 6, 2021 at 8:58 PM Paulo F. Oliveira [email protected] wrote:

The command-line Dialyzer warns about unkown types. I just tested with dialyzer v4.2.

Do you mean it does this by default?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/erlang/rebar3/issues/1751#issuecomment-755685369, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABDZJFKBSKE4SB6PWNOASRLSYTFHLANCNFSM4E3KEG6Q .

mheiber avatar Jan 07 '21 22:01 mheiber

It's possible the default was different back in Erlang/OTP R15 and we've never adjusted it, but at that point we're talking of 9 years old versions.

ferd avatar Jan 08 '21 20:01 ferd