-
-
Notifications
You must be signed in to change notification settings - Fork 139
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
Failed to parse warning #234
Comments
Huh interesting. I totally don't have any concept of whens yet in Erlex (underlying parsing lib) and no one's reported one of these yet. Will work on getting that added. Thanks for the report. |
Can you paste the surrounding function head and spec, if you don't mind @blatyo |
The line it's referring to is here: |
I believe a simplified case that will generate this is (untested): defmodule X do
def x do
Atom.to_string("not an atom")
end
end |
The issue is that this is incorrectly typed as string instead of atom: https://github.com/conduitframework/conduit/blob/dialyxir-issue/lib/conduit/broker/publish_route.ex#L10 |
Thanks. Might not get to it for a few days, but this is on my radar. If you're feeling adventurous, PRs are certainly welcome =) |
I have a tentative fix for it on asummers/erlex#8 but I don't have time this evening to test on your project -- can you give that a spin and let me know if it looks okay? I should get some time to run on your project tomorrow, if not. |
Doesn't seem to work. I ran with erlex overriden to your branch: https://github.com/conduitframework/conduit/blob/dialyxir-issue/mix.exs#L61
|
Hmm yeah that should be working. I'll see if I can figure out why it's still upset. |
@blatyo Interesting. Seems to be working, but overriding it in that manner wasn't seeming to take it? Unsure why. I had to pull down a copy of Dialyxir and override it in there. Will merge the Erlex PR and make the associated bump here. |
Using: 1.0.0-rc.3
The text was updated successfully, but these errors were encountered: