search

eproxus / meck

A mocking library for Erlang
http://eproxus.github.io/meck
Apache License 2.0
811 stars 231 forks source link

Fix some static analysis issues (xref + dialyzer) #219

Closed paulo-ferraz-oliveira closed 3 years ago

paulo-ferraz-oliveira commented 3 years ago

[Fix #218]

It started with me wanting to get rid of hamcrest -related warnings, since it's "used" but not imported by meck. And I write "used" because there is code to make sure it's there, before it's being used (and even catch clauses, at times), but the dialyzer doesn't care about that and complains in any way.

I guess I'll have to import hamcrest in the deps profile (on my lib.) just to force a correct analysis.

Though I did not get rid of all the warnings I wanted to get rid of, I still ended up doing a bit of clean-up, which you might want to merge.

paulo-ferraz-oliveira commented 3 years ago

Do you think there could be further steps to getting rid of hamcrest completely? Or is it not something you'd like to see happen?

eproxus commented 3 years ago

@paulo-ferraz-oliveira I for sure would like to get rid of Hamcrest as a dependency. The problem can be solved in a few different ways:

  1. Just remove Hamcrest and still "be compatible" without any tests to confirm
  2. Remove compatibility (maybe not explicitly in code, but at least not caring about it in the long run)
  3. Somehow make it optional (which is how it is now, but not really satisfactory obviously)
paulo-ferraz-oliveira commented 3 years ago

Just remove Hamcrest and still "be compatible" without any tests to confirm

This is part of the issue, for me. hamcrest is not actually a dependency. It's just an expectation in certain parts of the code, namely in meck.erl and meck_matcher.erl. I guess the solution to this (from the top of my head) could be to isolate the function calls in a new function and dialyzer-ignore it. Types (or any other kind of static element) would simply be "imported" from hamcrest.

Remove compatibility (maybe not explicitly in code, but at least not caring about it in the long run)

I don't understand what you mean by this.

Somehow make it optional (which is how it is now, but not really satisfactory obviously)

Touches what I've said before about function isolation and type importing.

Let me see if I can cook something up.