Cryptic type violation on Elixir 1.20 in older StreamData prop-based test

[sabiwara]

In Elixir 1.20.0-rc.1.

Feel free to close since it seems fixed on the latest StreamData (1.1.3+), but reporting just in case it is the symptom of an error message that could be improved.

Reproduction as a standalone script:

Mix.install([stream_data: "1.1.2"])
ExUnit.start()

defmodule ReproTest do
  use ExUnit.Case, async: true
  use ExUnitProperties

  property "causes a type violation" do
    check all(
            int1 <- positive_integer(),
            int2 <- positive_integer(),
            sum = int1 + int2
          ) do
      assert sum > int1
      assert sum > int2
    end
  end
end

 warning: this clause in cond will always match:

        sum = int1 + int2

    since it has type:

        dynamic(float() or integer())

    where "int1" was given the type:

        # type: dynamic(float() or integer())
        # from: scripts/stream_data_check_all.exs:12:24
        int1 + int2

    where "int2" was given the type:

        # type: dynamic(float() or integer())
        # from: scripts/stream_data_check_all.exs:12:24
        int1 + int2

    where "sum" was given the type:

        # type: dynamic(float() or integer())
        # from: scripts/stream_data_check_all.exs:12:17
        sum = int1 + int2

    typing violation found at:
    │
 12 │             sum = int1 + int2
    │             ~~~~~~~~~~~~~~~~~
    │
    └─ scripts/stream_data_check_all.exs:12: ReproTest."property causes a type violation"/1

I can understand why a number would always match in a cond if that's the code being generated, but reporting a "typing violation" in the sum feels really confusing here.

Will try to come up with a minimal repro if I can.

[josevalim]

The type system sees expanded code and the expanded code uses code, which is why it says cond. We have two options:

1. don't say the construct name, for cases like this, but then it is more vague for when we know the construct name
2. keep it as is

Thoughts?

[sabiwara]

Here is a minimal repro with cond:

defmodule Repro2 do
  def foo(int1, int2) do
    cond do
      sum = int1 + int2 -> sum
      true -> :skip
    end
  end
end

We have two options:

I agree that keeping the construct name is helpful to debug metaprogramming, I think we can probably keep it as is.
Or maybe add an extra info like "cond (generated)", which might help reduce confusion for people that are wondering where cond comes from, but we can probably add this later if there is a demand / is feasible.

The part that confused me in the first place was not really the cond part, but the last part:

    typing violation found at:
    │
 12 │             sum = int1 + int2

I first misunderstood this as:

• int1 and int2 are numbers
• sum is a number
• so sum = int1 + int2 is invalid (which didn't make any sense 😄)

Maybe we don't need the typing violation found at part at all here, since the useful part ("this clause in cond will always match") is already there. Or we can edit the message to avoid that confusion?

[josevalim]

I think the simplest fix is to remove the word violation? type-related warning found at: or similar? Or just type warning?

[sabiwara]

That could work. Or we could remove it altogether maybe, since we already have the code and the line number in this case?

[josevalim]

The reason it is there is to anchor most warnings, because the thing immediately preceeding the snippet is another snippet or a type declaration. Unless we change all of our warnings to show the snippet first. So I think removing it is tricky.

[sabiwara]

Maybe I'm overthinking this and type violation is actually fine?

Actually, looking at my cond minimal example above, it's not that confusing because we're pointing at the clause:

But in the StreamData example, we're missing the arrow, which is why it confused me so much.

[josevalim]

But I don't think it is a typing violation in this case. There are no types being violated, but rather we know some branches won't be executed.

[sabiwara]

That's true. type warning or type-related warning (or typing issue?) sound good then!