Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix dialyzer failures. #244

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

jimsynz
Copy link

@jimsynz jimsynz commented Jun 23, 2024

Hi there.

Running dialyzer on an Elixir project which has emqtt as a dependency results in the following dialyzer failures:

< path redacted >deps/emqtt/src/emqtt.erl:137:42:unknown_type
Unknown type: :emqx_types.message/0.
________________________________________________________________________________
< path redacted >deps/emqtt/src/emqtt.erl:151:31:unknown_type
Unknown type: :ssl.ssl_option/0.
________________________________________________________________________________
done (warnings were emitted)
Halting VM with exit status 2

The SSL one is easy to fix - there is no ssl:ssl_option (or at least not anymore), but there is ssl:tls_option.

The other failure is because the emqx_types module doesn't exist. I found it inside the emqx app source, but emqx is not listed as a project dependency. I'm open to suggestions on how to fix that.

@jimsynz jimsynz marked this pull request as draft June 23, 2024 22:38
@zmstone
Copy link
Member

zmstone commented Jun 24, 2024

thank you @jimsynz
could you try to keep the compatibility with old version otp?

@jimsynz
Copy link
Author

jimsynz commented Jul 1, 2024

@zmstone I'm not sure that it's possible.

@zmstone
Copy link
Member

zmstone commented Jul 1, 2024

@zmstone I'm not sure that it's possible.

-if(?OTP_RELEASE < 27).
-type tls_options() :: [ssl:tls_option()].
-else.
-type tls_options() :: [ssl:ssl_option()].
-endif.

@jimsynz
Copy link
Author

jimsynz commented Jul 2, 2024

Aha! This is why we don't get Elixirists to write Erlang :)

I'll update the PR.

@jimsynz jimsynz force-pushed the fix-dialyzer-warnings branch from 09b1ee1 to 670641b Compare July 3, 2024 22:43
@jimsynz jimsynz marked this pull request as ready for review July 3, 2024 22:43
@jimsynz
Copy link
Author

jimsynz commented Jul 3, 2024

@zmstone good to go for your review. Cheers!

@@ -33,7 +33,7 @@

-type(sockname() :: {inet:ip_address(), inet:port_number()}).

-type(option() :: gen_tcp:connect_option() | {ssl_opts, [ssl:ssl_option()]}).
-type(option() :: gen_tcp:connect_option() | {ssl_opts, [ssl:tls_option()]}).
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

need same fix here ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants