I like a lot about this proposal: the postfix bang syntax is clever and 
clear syntax; and the map access shorthand would be a nice new way to 
manipulate data structures assertively.

I am not yet fully sold on codifying the convention around result tuples at 
the syntax level, though. There is utility in the flexibility and 
expressiveness of loose conventions in general, and tagged tuples in 
particular.

While this proposed syntax takes nothing away from existing mechanisms, it 
does install some heavy tracks along the happy path. I worry that it could 
eventually lead to any code that does not fit into the shapes we choose for 
this macro to be deemed "unidiomatic". But the shapes we choose to 
recognize are a slippery slope: if we select too many, this feature becomes 
hard to reason about; if we select too few, either the feature would become 
an ignored curiosity and syntactic overhead to the language, or we risk 
biasing the community in favor of a handful of tagged tuples in particular 
when bespoke ones would serve better. Additionally, it could stifle new 
patterns and idioms arising in the community.

On the other hand, I love gently encouraging folk to return Exception 
structs in their error tuples, as it's a very useful pattern with little 
upfront cost.

On the whole I am somewhat in favor. So here's some lightweight 
bike-shedding on the tuples themselves.
------------------------------
Your proposal covers these cases:

   - *{:ok, value} -> value*
   - *{:error, exception} when is_exception(exception) -> raise exception*
   - *other -> raise BangNotOkError, message: ...*

Some other common tagged tuple cases I've used or seen, in rough order of 
frequency/utility, that might be worth thinking through:

   - Asserting ok-ness with nothing to return, in the case of side-effects. 
   This is probably the most common alternative case I can think of, I would 
   want to see this one added for sure:
      - *:ok -> nil*
   - Accepting multiple exceptions in the case of something like a batch of 
   Tasks. Rarer in codebases:
      - *{:error, multiple_exceptions} when is_list(multiple_exceptions) -> 
      somehow_raise(multiple_exceptions)*
   - Representing both errors and warnings in the case of something like a 
   compiler diagnostic run. Very niche:
      - *{:problems, warnings, errors} when is_list(warnings) and 
      is_list(errors) -> somehow_log(warnings) and somehow_raise(errors)*
   
------------------------------
One option to explore this space would be to develop support for *()!* (and/or 
*[]!*, *()?*, and *[]?*) in a fork of the lexer/parser, so that the 
implementation macros could be explored separately and concretely. This 
would also surface the required whitespace/context sensitivity and 
precedence for it to work well, ex with *|>*, *@*, and the like. 

I could see operators like *defmacro data[key]!* being easier to support. 
Typing this I see now why you propose *defmacro ok!(call_ast)* operator as 
*defmacro 
function(...args)!* is by necessity variadic and not supportable. 
Alternatively just ! and ? could be implemented as unary postfix operators 
but you'd have to negotiate with the unary prefix negation *!booleanable* in 
an even more confusing way.
------------------------------
Some other thoughts:

*> `!` at the end can be hard to spot especially on functions with a lot of 
arguments.*

This is probably my largest reservation with the syntax you've proposed.

*> Initially I thought it's a shame that the only option is to have `!` 
after argument parens, not before, but I believe it's actually a good 
thing. This leaves option to continue having "raising" variants as 
mentioned further.*
*> People could be confused by difference `fun!()` vs `fun()!` and which 
they should use. I'd say if `fun!` exists, it should be used.*

I would argue that if we adopt this, we should start soft-deprecating 
*fun!()* forms from the standard library entirely in that same release of 
Elixir, excepting functions that do not fit the *ok!* macro shape.
On Friday, May 3, 2024 at 1:05:50 AM UTC-5 woj...@wojtekmach.pl wrote:

> (Rendered gist: 
> https://gist.github.com/wojtekmach/6966fd4042b623a07119d3b4949c274c)
>
> Proposal 1: fun(...)!
>
> It's very common to define functions that return `{:ok, value} | :error`, 
> `{:ok, value} | {:error, reason}`, and similar and then also their 
> "raising" variants that return `value` (or raise). By convention the name 
> of such functions end with a `!`.
>
> Some obvious examples from the standard library are:
>
> Base.decode16!/1
> Version.parse!/1
> Date.from_iso8601!/1
> Date.new!/3
>
> I'd like to propose to encode this pattern, when something has a `!` it 
> raises, at the language level.
>
> Initially I thought it's a shame that the only option is to have `!` after 
> argument parens, not before, but I believe it's actually a good thing. This 
> leaves option to continue having "raising" variants as mentioned further.
>
> Examples:
>
> Req.get(url)!
> Application.ensure_all_started(:phoenix)!
> :zip.extract(path, [:memory])!
>
> I believe the main benefits are:
>
>   * less need to define functions that just wrap existing functions. For 
> example, if this existed I'd just have `Req.get` and no `Req.get!`.
>
>   * more easily write assertive code. One mistake that I tend to make is 
> forgetting `{:ok, _} =` match in places like 
> `Application.ensure_all_started(...)`, `SomeServer.start_link(...)`, etc. 
> This is especially useful in tests.
>
>   * more pipe friendly, instead of first `{:ok, value} = ` matching (or 
> using `|> then(fn {:ok, val} -> ... end)` one could simply do, say, `|> 
> :zip.extract([:memory])!`.
>
>   * this is fairly niche but people could write DSLs where trailing ! 
> means something entirely different.
>
> I believe the tradeoffs are:
>
>   * given it is a syntactic feature the barrier to entry is very high. If 
> adopted, parsers, LSPs, syntax highlighters, etc all need to be updated.
>
>   * given it is a syntactic feature it is hard to document.
>
>   * people could be confused by difference `fun!()` vs `fun()!` and which 
> they should use. I'd say if `fun!` exists, it should be used. For example, 
> given `Date.from_iso8601!(string)` exists, instead of writing 
> `Date.from_iso8601(string)!` people should write 
> `Date.from_iso8601!(string)` however there's no automatic mechanism to 
> figure that out. (A credo check could be implemented.)
>
>   * `!` at the end can be hard to spot especially on functions with a lot 
> of arguments.
>
>   * code like `!foo!()` would look pretty odd. `foo!()!` is probably even 
> more odd.
>
>   * can't use trailing `!` for anything else in the future.
>
> Finally, while `foo!()` is more common (Elixir, Rust, Ruby, Julia), 
> `foo()!` is not unheard of:
>
> Swift:
>
> ~% echo 'import Foundation ; URL(string: "https://swift.org";)!' | swift 
> repl
> $R0: Foundation.URL = "https://swift.org";
> ~% echo 'import Foundation ; URL(string: "")!' | swift repl
> __lldb_expr_1/repl.swift:1: Fatal error: Unexpectedly found nil while 
> unwrapping an Optional value
>
> Dart:
>
> ~% echo 'void main() { print(int.tryParse("42")!); }' > main.dart ; dart 
> run main.dart
> 42
> ~% echo 'void main() { print(int.tryParse("bad")!); }' > main.dart ; dart 
> run main.dart
> Unhandled exception:
> Null check operator used on a null value
>
> The way it would work is the parser would parse `fun(...)!` as 
> `Kernel.ok!(fun(...))`, the exact naming is to be determined. (Another 
> potential name is `Kernel.unwrap!/1`.) Consequently `Macro.to_string`, the 
> formatter, etc would print such AST as again `fun(...)!`.
>
> Pipes like `foo() |> bar()!` would be equivalent to `foo() |> ok!(bar())`.
>
> I'd like to propose the following implementation of such `Kernel.ok!/1`:
>
> defmacro ok!(ast) do # ast should be a local or a remote call
>   string = Macro.to_string(ast)
>
>   quote do
>     case unquote(ast) do
>       {:ok, value} ->
>         value
>
>       {:error, %{__exception__: true} = e} ->
>         raise e
>
>       other ->
>         raise "expected #{unquote(string)} to return {:ok, term}, got: 
> #{inspect(other)}"
>     end
>   end
> end
>
> The macro would allow us to have slightly better stacktrace.
>
> As seen in the implementation, we have extra support for functions that 
> return `{:error, exception}` but also gracefully handle functions that just 
> return `:error`, `{:error, atom()}`, etc.
>
> Existing functions like `Version.parse/1` (can return `:error`), 
> `Date.from_iso8601/1` (can return `{:error, :invalid_format}`), etc would 
> not benefit from such `!` operator, it would be a downgrade over calling 
> their existing raising variants because the error return value does not 
> have enough information to provide as good error messages as currently. 
> Another example outside of core is `Repo.insert/1` returns `{:error, 
> changeset}` but `Repo.insert!` raises `Ecto.InvalidChangesetError` with 
> very helpful error message. It is not the intention of this proposal to 
> deprecate "raising" variants but to complement them. Function authors 
> should continue implementing "raising" variants if they can provide extra 
> information over just calling their function with the ! operator. There is 
> also matter of pairs of functions like `struct/2` and `struct!/2` which of 
> course will stay as is.
>
> An alternative implementation to consider is:
>
> defmacro ok!(ast) do
>   quote do
>     {:ok, value} = unquote(ast)
>     value
>   end
> end
>
> where we would, say, define `Exception.blame/2` on `MatchError` to special 
> case `{:error, exception}`, pretty print it using `Exception.message/1`. 
> Printing such exceptions would not be equivalent to _raising_ them however 
> perhaps this would be good enough in practice. Such macro would certainly 
> generate less code as well as other places where we raise MatchError right 
> now could give better errors. (If we're implementing Exception blame anyway 
> we could change the stacktrace _there_ and ok!/1 could potentially be just 
> a regular function.)
>
> Proposal 2: term[key]!
>
> Similar to proposal above, I'd like to propose enhancement to the `[]` 
> operator: `term[key]!` which raises if the key is not found.
>
> This proposal is completely orthogonal to the former one. (If I would have 
> to choose only one it definitely would be the former.) I believe 
>
> I believe the main benefits are:
>
>   * `Map.fetch!(map, "string key")` can be replaced by more concise 
> `map["string key"]!`
>
>   * Custom Access implementations have easy assertive variant. One example 
> is recent Phoenix form improvements, writing `@form[:emaail]` silently 
> "fails" and it'd be trivial to make more assertive with `@form[:emaail]!`
>
>   * If `fun(...)!` is accepted, the pattern starts becoming more familiar.
>
> I can't think of any downsides other the ones with `fun(...)!`.
>
> I think they way it would work is `term[key]!` would be parsed as 
> `Access.fetch!(term, key)` (which is an already existing function!) and 
> `Macro.to_string/1` and friends would convert it back to `term[key]!`.
>
> It can be combined: `map[a]![b]![c]` would work too. (`map[a][b]!` should 
> probably be a warning though.)
>

-- 
You received this message because you are subscribed to the Google Groups 
"elixir-lang-core" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to elixir-lang-core+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/elixir-lang-core/e3dfb8ec-8b67-4b1f-ae9d-040eb47f0c56n%40googlegroups.com.

Reply via email to