elixir-lang / elixir

Elixir is a dynamic, functional language for building scalable and maintainable applications
https://elixir-lang.org/
Apache License 2.0
24.62k stars 3.38k forks source link

assert_receive regression #9899

Closed ityonemo closed 4 years ago

ityonemo commented 4 years ago

Precheck

Environment

Current behavior

module code:

defmodule TestMatches do
  defmacro m do
    quote do
      {:ok, m} when is_integer(m)
    end
  end
end

test code:

defmodule TestMatchesTest do
  use ExUnit.Case

  require TestMatches

  test "assert_receive can match macros with when" do
    send(self(), {:ok, 2})
    assert_receive TestMatches.m
  end
end

fails due to:

(CompileError) test/test_matches_test.exs:7: cannot find or invoke local when/2 inside match. Only macros can be invoked in a match and they must be defined before their invocation. Called as: {:ok, m} when is_integer(m)

Previous behaviour

In elixir 1.9.4:

> mix test
.

Finished in 0.02 seconds
1 test, 0 failures

Etc.

Notably the following fails to compile on both version of elixir, but succeeds without the last assertion on 1.9 and 1.10. Similarly, it succeeds if the "when" clause is omitted from the main code (so macros are generally supported inside of match?/2, just not macros with when/2):

defmodule TestMatchesTest2 do
  use ExUnit.Case

  require TestMatches

  test "matches can't match macros with when" do
    assert match?({:ok, foo} when is_integer(foo), {:ok, 2})
    refute match?({:ok, foo} when is_integer(foo), {:ok, "foo"})
    assert match?(TestMatches.m, {:ok, 2})
  end
end

if the implementation of assert_receive/1 has changed between 1.9 and 1.10, this might be expected behavior, in which case this is not an error.

josevalim commented 4 years ago

Thanks for the report @ityonemo!

Quicck question: are you sure this worked on v1.9? I have tried v1.9.4 and it couldn't compile either. In any case, the behaviour seems to be consistent with how match?/receive behave, as this does not compile either:

  test "assert_receive can match macros with when" do
    send(self(), {:ok, 2})
    receive do
      TestMatches.m -> :ok
    end
  end
josevalim commented 4 years ago

It also didn't work on v1.8.

ityonemo commented 4 years ago

https://asciinema.org/a/R4nEbbfkEHfbGxtImVKH1KADm

I'll try 1.8

--edit--

its seems to work on 1.8 for me, as well. I'll make a git repo with the code.

ityonemo commented 4 years ago

https://github.com/ityonemo/test_matches/actions/runs/60452346

josevalim commented 4 years ago

@ityonemo oh, thank you for the repo! It was my bad, I was testing the already expanded version. In v1.9 and v1.8, none of them work like this:


  test "receive can match macros with when" do
    send(self(), {:ok, 2})
    receive do
      TestMatches.m -> :ok
    end
  end

So I am more inclined to consider the current behaviour as more consistent. You shouldn't be able to assert_receive or assert match? on an expression you cannot receive or match on.

ityonemo commented 4 years ago

agreed that you shouldn't be able to assert_receive on something you cannot receive on, but you can do this:

iex(1)> send(self(), {:ok, 2})
iex(2)> receive do
...(2)>   {:ok, q} when is_integer(q) -> :ok 
...(2)> end

And you can also generally put macros in matches, I think when doesn't seem work with macros in matches, anymore. It doesn't necessarily have to, but it would be nice.

josevalim commented 4 years ago

And you can also generally put macros in matches

Yes, but you can't put guards in matches. The reason is that it wouldn't really compose. Imagine if then you want to rewrite it to this:

    receive do
      {:ok, TestMatches.m} -> :ok
    end

This now wouldn't work as the when would be moved inside the tuple. Sure, we could allow a guard anywhere in a match to work but that itself is a whole separate discussion - with its complications. For now I will update the CHANGELOG to talk about this, thanks!

ityonemo commented 4 years ago

Beautiful. That rationale makes a ton of sense.