ftp: fix types in ftp module

[kikofernandez]

Fixes some type specs from the ftp module. These have been validated by eqWAlizer.

[github-actions]

CT Test Results

    2 files    13 suites   5m 35s ⏱️
144 tests 142 ✔️ 2 💤 0 ❌
204 runs  202 ✔️ 2 💤 0 ❌

Results for commit 8122362.

♻️ This comment has been updated with latest results.

To speed up review, make sure that you have read Contributing to Erlang/OTP and that all checks pass.

See the TESTING and DEVELOPMENT HowTo guides for details about how to run test locally.

Artifacts

• Complete CT logs (Download Logs)
• HTML Documentation (Download HTML Docs)
• Windows Installer

// Erlang/OTP Github Action Bot

[IngelaAndin]

Branch name and comments are misleading as you also change inets files!

[kikofernandez]

Thanks for the feedback.

I am updating two modules and will close this PR and create the appropriate one with a better branch name.

I would like to get feedback regarding the changes, if we think that those guards make sense.

• If we add them, eqWAlizer is happy about the types because it can type check those functions.
• If we do not have them, it would reject those functions.

From my point of view:

• it makes sense to add them, to say that the function expects lists / values different from undefined or similar
• there will be a small overhead in those checks, but AFAIK they are almost for free (?)
• if there is a catch all clause in those functions, adding these guards changes the semantics of the program, as instead of crashing at runtime, the code will go through the catch-all function

[kikofernandez]

I have remove the parts that dealt with the inets module to be in its own PR (#6661 )
A new review is welcome :)

[kikofernandez]

• I decided that to keep the types that OTP maintainers could benefit, and leave a more general interface to our users; I used an internal FTP module that contains the expected types for maintainers (ftp_internal.erl), while I removed the expected reason of the error from the public API (ftp.erl). In this way, ftp.erl is simply a wrapper around our internal API.

• Type-wise, we are not breaking anything in Dialyzer or other typing tools, since the change means that (Reason :: {error, common_reason() | ...}) <: (Reason :: {error, term()}) because (common_reason() | ...) <: term()

Please let me know if this is a viable solution or if we need to find another way. Alternatives:

• Remove the reason types (but I think that we maintainers do benefit from having the expected type error...)
• Collpase all Reason types into a single one (but I think this gives a superset of types to some functions where we do not expect certain kinds of errors)

[IngelaAndin]

I think that this solution feels good. I do not think that for example then commit
[ftp: removal of defensive programming checks] feels like an interesting one to keep for the final solution. I think you should try to squash some of the commits to logical steps of what we think the final result should look like.

[kikofernandez]

Thanks for the feedback. I have placed everything in two commits.

1. Added the documentation to be read from source (at least the types) instead of hard-coded in XML
2. Created the ftp_internal.erl module and made ftp.erl a simple wrapper around ftp_internal.erl, such that ftp.erl contains the user types (and documentation) and ftp_internal.erl contains the types that OTP team may care (and dialyzer 😄 )