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

The pattern 'false' can never match the type 'true' #21

Open
thiagomajesk opened this issue Apr 9, 2020 · 7 comments · May be fixed by #25
Open

The pattern 'false' can never match the type 'true' #21

thiagomajesk opened this issue Apr 9, 2020 · 7 comments · May be fixed by #25

Comments

@thiagomajesk
Copy link

Cheers everyone!
I'm having this Dialyzer warning just by implementing the perform/1 function:

The pattern 'false' can never match the type 'true'

There's also another warning:

The test  1 == 'infinity' can never evaluate to 'true'

I'm using VS Code with ElixiLS Fork and Elixir 1.9.4 compiled with OTP 20 (OTP 22 local)

image

@sheharyarn
Copy link
Owner

Hey @thiagomajesk, we don't plan on adding Dialyzer support anytime soon, but PRs are welcome!

@thiagomajesk
Copy link
Author

@sheharyarn Thanks for the reply!
Could you point me on the direct direction of what can I do to contribute to this specific Issue?

@sheharyarn
Copy link
Owner

sheharyarn commented Apr 9, 2020 via email

@NobbZ
Copy link

NobbZ commented Nov 7, 2022

The OP already says what and why the problem is.

@concurrency gets hard replaced with its literal value, resulting in checks to be emitted that will from dialyzer viewpoint never come true or never come false.

The second check in the generated __after_compile__/2 will either statically raise or not, and the condition can be fully resolved even outside of the quote, before the code is even generated.

I created a prototype recently, though that fails the current test suite.

If there is still some interest in this, I will see to create a draft PR next time I get access to the computer and no one deleted my data their :D

@thiagomajesk
Copy link
Author

Thanks for your insights @NobbZ!

I haven't had the time to look into this yet even though I've been using Que in production for the past few years... For the time being, my team is tracking up a list of potential fixes we want to see in the lib, but given the project's low maintenance status, we are still figuring out if we should branch the work or PR the fixes.

Fortunately (or unfortunately) Que is one of the best libs we've encountered that solve exactly the problems we want from a job processing lib, that being: 1) 100% native elixir (nothing outside of the beam) and 2) Easy to use and low setup/ overhead.

If you can draft a PR for this I'm sure it would be very welcome!👍

@NobbZ
Copy link

NobbZ commented Nov 7, 2022

It works in production for us as well, though we do not like the fact that que is the last library that produces code that dialyzer complains about, we have been able to fix all the other dialyzer problems in the code base that we took over, some of them indeed pointing out stuff that only worked by accident.

Though I have to say, even if you were running dialyzer, it wouldn't have complained, as dialyzer can not see into macros. Only into "materialzed" code. So unless you were doing use Que.Worker within que, the error would never had been materialized. And this one really isn't problematic, it just is annoying to pop up in our editors all the time (which doesn't respect dialyzer ignore files)

@NobbZ
Copy link

NobbZ commented Feb 24, 2023

I finally managed to get back to this and threw away my initial approach.

That other approach still tried to stay within the generated module and I think it is much better to have just a single hook, rather than having a "noop" function in every queue module.

In theory this is a breaking change as it removes a public function from the generated modules. Though __after_compile__/2 is usually ignored by the docs system and should therefore by convention be considered an implementation detail anyway.

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

Successfully merging a pull request may close this issue.

3 participants