erlang / otp

Erlang/OTP
http://erlang.org
Apache License 2.0
11.31k stars 2.94k forks source link

Specs in OTP 27 with overlapping domains #8810

Open ilya-klyuchnikov opened 1 week ago

ilya-klyuchnikov commented 1 week ago

Describe the bug OTP 27 introduced a bunch of specs with overloaded contracts that have overlapping domains, - that makes them unusable (and detrimental) for tooling (like type-checkers).

Examples:

Expected behavior Specs in OTP are well-written, are an example of best practices and conform to recommendations in documentation

Probably, there are more cases, - we encountered the quoted cases when testing our projects against OTP 27.

A current restriction, which currently results in a warning by Dialyzer, is that the domains of the argument types cannot overlap

Affected versions OTP 27.

Additional context

Probably after https://github.com/erlang/otp/pull/6654 dialyzer runs in OTP repro do not check for overlapping domains anymore.

ilya-klyuchnikov commented 1 week ago

One more spec with overloaded domains:

https://github.com/erlang/otp/blob/3ff36ef6f6f20749b5a03b3667206ede0fdb5791/lib/kernel/src/logger.erl#L553-L556

If the first argument has the type level(), the third argument has the type metadata() and the second argument is an empty list [] - it satisfies both the first and the second subspecs.

kikofernandez commented 5 days ago

I am not sure that we have changed too much the existing types, so the issue may be that Erlang has never enforced the correct typing.

What is the expected behaviour / expectation for something that has been around for some time? We all want correct types and non-overlapping domains, but given that the API is already public and changing the types breaks backwards compatibility, what is the expectation? Any proposal?

ilya-klyuchnikov commented 5 days ago

socket:open was the same in OTP-26 (https://www.erlang.org/docs/26/man/socket#open-2)

The second sub-spec has changed.

Screenshot 2024-09-19 at 09 25 26

Screenshot 2024-09-19 at 09 27 09

The concrete types for Domain and Type were lost and replaced by term(). The return type was also replaced by (_ - synonym to term()).

ilya-klyuchnikov commented 5 days ago

socket:recv we seem to have lost some time information that was replaced by term()...

yes

ilya-klyuchnikov commented 4 days ago

logger:log had mostly this interface as well in OTP-26

But details are different, - note that it had one subspec with StringOrReport that was split into two subspecs - and these new two type specs have overlapping domains now.

ilya-klyuchnikov commented 4 days ago

What is the expected behaviour / expectation for something that has been around for some time?

Reverting mentioned specs to how they were defined in OTP 26 seems to be a bug fix rather than a breaking change. (In a sense, - eg for our usage of eqWAlizer, the mentioned changes were breaking changes since they introduced new type errors in our code).

ilya-klyuchnikov commented 4 days ago

and changing the types breaks backwards compatibility

How do you define backward compatibility here?

kikofernandez commented 3 days ago

ok, the PR you mention certainly made us unaware of the overlapping domains. I will re-enable the overlapping domains flag and fix other issues that we missed. I leave this ticket open until that done has been done and some APIs can go back to how they were. I am not sure that all of them can, due to other reasons