How to fix this Dialyzer warning "The guard clause can never succeed."

Questions / Help
1

Hi,

I started contributing to a project where the CI checks for Dialyzer warnings. There is this code where I inverted the condition and branches of an if/else statement to make it a tiny bit easier to read.

The code in question:

  def create_friend_request(from_user_id, to_user_id) do
    {allowed, reason} = can_send_friend_request_with_reason?(from_user_id, to_user_id)

    if allowed do
      (...)
    else
      (...)
    end
  end

Previously, it was if not allowed do and Dialyzer was fine with it. Now, Dialyzer emits this warning for the line if allowed do: The guard clause can never succeed.

This is how can_send_friend_request_with_reason? is defined:

  @spec can_send_friend_request_with_reason?(T.userid(), T.userid()) ::
          {true, :ok} | {false, String.t()}
  defdelegate can_send_friend_request_with_reason?(from_id, to_id), to: FriendRequestLib

I don’t have much experience with Dialyzer, so I wonder if I am missing something obvious?

In case you need more context than this, this is all happening in this PR. Thank you :slight_smile:

1 Like
2

Without your refactor but with these lines removed (teiserver/.dialyzer_ignore.exs at master · beyond-all-reason/teiserver · GitHub) is your line or function being warned about?

Given the nearly 220 lines long dialyzer ignore, they can as well remove dialyzer completely.

Until proven otherwise, I assume that your refactor just uncovered an issue again that was hidden before.

Instead of silencing all the warnings, they should get fixed.

1 Like
3

Oh, I should have looked into the Dialyzer ignores. With the 2 lines removed, it uncovers a lot of warnings, but not the one I am facing. As expected, there are loads of unknown type warnings and a few invalid contract warnings as well.

Yes, I agree. I am working on getting Credo to run in the CI now, I have to address a lot of Credo issues. I’ll add working through those Dialyzer warnings to my TODOs.

4

The problem with ignored dialyzer warnings, they bubble up, and will explode elsewhere eventually.

So if there is an ignored warning somewhere in the callchain, it might now just blow up, though without knowing what that ignored warning is, its hard to tell why it blows after the refactor but not before.

Personally I refuse to accomodate dialyzer in projects like this. If dialyzer is used and “enforced”, then there shouldn’t be any ignored warnings, and if there are, there should be a comment explaining every single entry in the ignore file.

In cases like the one you have now, I would just add an entry to the ignore file.

1 Like
5

I can only nod in agreement to what you wrote. I tend to be quite strict with static analysis tools. You either enforce their usage or don’t. Only then do they bring value, otherwise it’s confusing to be in this middle ground.

I really didn’t want to resign myself to ignore this Dialyzer warning :melting_face:, but I believe it is the right call for now.