reasonml / reason

Simple, fast & type safe code that leverages the JavaScript & OCaml ecosystems
http://reasonml.github.io
MIT License
10.14k stars 428 forks source link

Implicit blocks for else. #2477

Open jordwalke opened 5 years ago

jordwalke commented 5 years ago

Many people want the benefits of early return. There's a way to get many of them without the complexity of early return:

We can simply allow the else blocks to not be wrapped in { }. Instead of this:

let fn = (shouldRun, name) => {
  if (!shouldRun()) {
    []
  } else {
    if (name == "test") {
      []
    } else {
      if (String.length(name) > 80) {
        ["name too long"]
      } else {
        []    
      };
    };
  };
};

We could allow:

let fn = (shouldRun, name) => {
  if (!shouldRun()) {
    []
  } else
  if (name == "test") {
    []
  } else
  if (String.length(name) > 80) {
    ["name too long"]
  } else {
    []
  }
};
jordwalke commented 5 years ago

That particular example already parses with today's Reason parser - we would just need to preserve the indentation in that case (track it in a ppx annotation in the AST).

But there are other examples which do not parse with today's parser, or others which would parse but be interpreted differently: Today, this:

let fn = (shouldRun, name) => {
  if (!shouldRun()) {
    []
  } else
  foo();
  let x = 0;
 };

Is parsed as:

let fn = (shouldRun, name) => {
  if (!shouldRun()) {
    []
  } else {
    foo();
  }
  let x = 0;
 };

But we would want it to be interpreted as:

let fn = (shouldRun, name) => {
  if (!shouldRun()) {
    []
  } else {
    foo();
    let x = 0;
  }
 };

This is a breaking change w.r.t. non-refmted code, but it's not a breaking change w.r.t. refmted code. In general, we promise not to break code that has been refmted (without a major version bump).

anmonteiro commented 5 years ago

This proposal feels odd to me. It just doesn't make the code clear IMO and to my knowledge no other language does this.

At the risk of sounding arrogant, if you want less braces just write in OCaml syntax?

jordwalke commented 5 years ago

@anmonteiro: Because:

jordwalke commented 5 years ago

Do you have any other suggestions for how to respond to people coming from JS that lament the extra indentation in functions that handle a couple simple cases early? This proposal in this issue is just a proposal that aims to solve that complaint, but I'm sure there's other proposals that address that same motivation. This is just the first one that came to my mind. I would like to discourage people from resorting to full-on early return because it destroys local reasoning about your functions. If we need to build a new syntactic construct to avoid early return, then I would be all for it.

anmonteiro commented 5 years ago

I don't have any alternative suggestions to offer but I still think this will create more problems than it solves. Early return is possible in e.g. JS because you don't have to guarantee that both branches return the same type.

I'm fairly certain that this will cause problems down the road wrt the type checker where people will not understand why the compiler is telling them to return unit or whatever type error they encounter.

Lupus commented 5 years ago

In JavaScript lack of explicit curly braces around body of if-else blocks implicitly puts them around first statement. Else block automagically extending till the end of function will probably look highly un-intuitive for JavaScript developer. For OCaml/ReasonML developer it just looks wrong.

Monadic let bindings solve this problem nicely and idiomatically, especially given that let+ operators landed to mainline ocaml and require no ppx. Yet I will demonstrate my point with pseudocode using ppx_let:

let request_handler = (ctx) => {
  open Result.Let_syntax;
  /* the below let binding will just "return early" in case of Error(...) */
  let%bind param = Request_context.get_query_param(ctx, "my-option");
  /* starting at this point param contains whatever was in Ok(...) */
  Stdio.printf("param: %s\n", param);
  /* more bindings can be added to the same function */
  let%bind header = Request_context.get_header(ctx, "Content-Length");
  Stdio.printf("content length is %s\n", header);

  /* imagine a lot more business logic code here */

  /* final return value must be wrapped in Result.t */
  Result.return(Response.of_string("Hello, world!"));
};

For me the selling point of monads as something practical and valuable was ability to have sound early return pattern, which I happily use.

ozanmakes commented 5 years ago

With reason-macros early returns in imperative code can be made to look decent:

let%macro.let returnWhen = (pattern, value, continuation) =>
  switch (eval__value) {
  | eval__pattern => ()
  | _ => eval__continuation
  };

// usage
let%returnWhen true = exit^;
johnhaley81 commented 5 years ago

When writing code like this where I have an early return, I'll shadow the function with the the early return.

e.g.

let doTheThing = () => {
  foo();
  let x = 0;
};
let doTheThing = shouldRun =>
  !shouldRun() ? () : doTheThing();

It's nice since it flattens and splits out the actual logic for the whatever the thing is doing and the cases needed to be satisfied for doing the thing. Definitely not something that'll you intuitively know to do coming from JS-land but I think really shows off some nice ergonomics of the language after you get your head around it.

kyldvs commented 5 years ago

Originally I was proposing some return ppx like:

let fn = (shouldRun, name) => {
  if (!shouldRun()) {
    [@return] []
  };

  if (name == "test") {
    [@return] []
  };

  if (String.length(name) > 80) {
    [@return] ["name too long"]
  };

  []; 
};

Which has behavior like:

And compiles to:

let fn = (shouldRun, name) => {
  if (!shouldRun()) {
    []
  } else {
    if (name == "test") {
      []
    } else {
      if (String.length(name) > 80) {
        ["name too long"]
      } else {
        []    
      };
    };
  };
};

Then we realized it's similar to:

let fn = (shouldRun, name) => {
  if (!shouldRun()) {
    []
  } else

  if (name == "test") {
    []
  } else

  if (String.length(name) > 80) {
    ["name too long"]
  } else

  []; 
};

But I'm also not a big fan of implict block else syntax. It doesn't seem very clear to me