Do not allow guards in assert/1 #13817
Conversation
|
|
||
| @doc false | ||
| def __match__({:when, _, _} = left, _, _, _, _) do | ||
| raise ArgumentError, """ |
There was a problem hiding this comment.
I'm not sure what the best way to raise here would be.
Ideally this would be a CompileError.
BTW, I noticed that right now without assert we're getting:
error: undefined function when/2 (there is no such import)But I think we can have a better error message in this case, given that when is a reserved word and can't be used as a function name?
| raise ArgumentError, """ | ||
| invalid pattern in assert/1: #{Macro.to_string(left)} | ||
| Use assert match?(... when ...) if you need to assert with a guard. |
There was a problem hiding this comment.
We could even print out the actual code they were trying to use, right? We have all the ASTs and stuff. Maybe
Use this if you need to assert with a guard:
assert match?(actual_code)There was a problem hiding this comment.
I like it, but I can see how it could end up with a big suggestion if the pattern is a huge map for instance (and Macro.to_string/2 doesn't have a limit like inspect).
What do you think?
There was a problem hiding this comment.
I’m not sure someone would assert on a huge map with a guard at the end, as in I’m not sure that's a common pattern. If that ends up happening, you could always format the code to make it fit nicely.
There was a problem hiding this comment.
SGTM! I went with 6cb2a91, WDYT?
Co-authored-by: Andrea Leopardi <[email protected]>
3 participants
Close #13814
Indeed as pointed out by José guards being able were a side effect of the shared implementation between
match?and regular=.The
match?version would not be strictly equivalent, since there is no way to to reuse extracted variables after matches, but this is probably an OK limitation in practice.