elixir-lang / elixir

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

assert macro allows for guards in match context #13814

Closed lukaszsamson closed 2 months ago

lukaszsamson commented 2 months ago

Elixir and Erlang/OTP versions

Erlang/OTP 26 [erts-14.2.5.2] [source] [64-bit] [smp:12:12] [ds:12:12:10] [async-threads:1] [jit]

Elixir 1.17.2 (compiled with Erlang/OTP 26)

Operating system

any

Current behavior

The following test unexpectedly passes.

defmodule FooTest do
  use ExUnit.Case, async: true

  test "foo" do
    assert (a when is_integer(a)) = 5
  end
end

Support for guards is not mentioned in https://hexdocs.pm/ex_unit/1.17.2/ExUnit.Assertions.html#assert/1

Expected behavior

Basing on the documentation I expect assert to not change language semantics. This code should emit a compilation error as guards are not allowed in match context. The same code without assert errors with

    error: cannot invoke remote function :erlang.is_integer/1 inside a match
    │
  5 │      (a when is_integer(a)) = 5
    │              ^
    │
    └─ test/foo_test.exs:5:14: FooTest."test foo"/1

Excluding tags: [pending: true]
    error: cannot find or invoke local when/2 inside a match. Only macros can be invoked inside a match and they must be defined before their invocation. Called as: a when is_integer(a)
    │
  5 │      (a when is_integer(a)) = 5
    │         ^
    │
    └─ test/foo_test.exs:5:9: FooTest."test foo"/1

== Compilation error in file test/foo_test.exs ==
** (CompileError) test/foo_test.exs: cannot compile module FooTest (errors have been logged)
sabiwara commented 2 months ago

It seems to be an intended feature at least using match?, and the diffs for it are even nicely supported:

Screenshot 2024-09-07 at 7 25 28

This seems like a valuable feature, perhaps we should try to find a way to make it officially documented and available? Semantically-wise, I wonder if we couldn't make assert support <- which would be nicer than wrapping in match?:

assert [a] when is_integer(a) <- my_list
lukaszsamson commented 2 months ago

match? macro is a different case. It is documented to allow guards. It doesn't change semantics as it expands to case clauses.

assert [a] when is_integer(a) <- my_list

Interesting idea but it brings inconsistency. In other instances <- operator is followed by do block

josevalim commented 2 months ago

I agree with @lukaszsamson. If you want to use guards, you can use assert match?. We should not allow when inside = in assert, it is probably a side-effect of the implementation.