Convert between => and { return ...; } based on line length (WONTFIX)

[Hixie]

It would be interesting if the formatter could automatically add or remove braces based on the line length.

For example, turning this:

   bool get hasSize {
     return true;
   }
   int crazyLongFunction(int withMany, int manyMany, int arguments) => withMany + manyMany + arguments +
      withMany * 2 + manyMany * 5 + arguments;

...into this:

   bool get hasSize => true;
   int crazyLongFunction(int withMany, int manyMany, int arguments) {
     return withMany + manyMany + arguments + withMany * 2 + manyMany * 5 + arguments;
   }

[munificent]

Like I mentioned on #451, this is out of scope for the formatter as it's currently specced.

[Hixie]

In that case, I think there should be a restriction that the formatter never insert a line break into an expression that contains a => operator.

[jmesserly]

I think it's time to move past the "only whitespace" rule. It's a self-imposed restriction. The theory was to help adoption by making the formatter seem less scary. But it seems to be doing the opposite, by preventing improvements that users would like to see.

[munificent]

See the comments on #451. I think it's a useful restriction. Making more significant changes is a lot more complex and has a lot more failure modes. I don't think that's a good default behavior for the formatter. That's what the linter package is for.

[alan-knight]

Also, typical style for Intl messages is as lambda's, even though they might be several lines. I wouldn't like having

  hello(name) => Intl.message("Hello $name", 
      name: "hello", 
      desc: "Say Hello to the user",
      examples: { "name" : "Alice"});

turned into a function with braces because it didn't fit.

[Hixie]

String hello(name) {
  return Intl.message(
    'Hello $name',
    name: 'hello',
    desc: 'Say Hello to the user',
    examples: <String, String>{ 'name': 'Alice' }
  );
}

[kasperpeulen]

I actually like how dart fmt handles this now:

Adding the return statement, would make the line a bit longer then 80 chars:

which would give this as result:

I'm not totally convinced if dart fmt should only handle whitespace, but in this case, I'm glad that it doesn't convert my => in { return ...; }.

[Hixie]

I agree that the version with the wrapped return looks bad, but that's because of the misaligned "withMany" vs "manyMany", IMHO. It still looks better than the newline after the =>.

Personally I am in favour of "soft" 80 character limits which would lead to the second one above.

But the ideal wrapping for that particular function is probably the following:

int crazyLongFunction(int withMany, int manyMany, int arguments) {
  return withMany     + manyMany     + arguments +
         withMany * 2 + manyMany * 5 + arguments;
}

None of the other wrappings should what's actually going on as well as that one. And once you see that, you quickly see that the code should really be:

int crazyLongFunction(int withMany, int manyMany, int arguments) {
  return withMany * 3 + manyMany * 6 + arguments * 2;
}

[kasperpeulen]

@Hixie I agree about the symmetry argument (you also raised somewhere else I think). It would be nice if dartfmt could do something like:

int crazyLongFunction(int withMany, int manyMany, int arguments) {
  return withMany +
         manyMany +
         arguments +
         withMany * 2 +
         manyMany * 5 +
         arguments;
}

or what you propose, but I guess that may be very hard to get implemented? (no idea tbh)

I don't really see though why this:

int crazyLongFunction(int withMany, int manyMany, int arguments) {
  return withMany * 3 + manyMany * 6 + arguments * 2;
}

would be prefered over:

int crazyLongFunction(int withMany, int manyMany, int arguments) =>
    withMany * 3 + manyMany * 6 + arguments * 2;

I always format it with an arrow, where possible.

[Hixie]

At the end of the day, it's a matter of personal preference. I find that => works well when you have a very short expression, because then it looks like you're doing something cheap. I find that if you're doing something long, then { ... } looks better because it's more obvious that it's a bunch of code.

If I am scanning through code and see the line:

withMany * 3 + manyMany * 6 + arguments * 2;

...then it's not obvious to me that it's part of a function unless it's wrapped in braces. The braces are aligned so that the closing brace lines up with the start of the opening lines, so I can easily tell which block the code is in. When there's a bunch of lines:

int crazyLongFunction(int withMany, int manyMany, int arguments) =>
    withMany * 3 + manyMany * 6 + arguments * 2;
int crazyLongFunction(int withMany, int manyMany, int arguments) =>
    withMany * 3 + manyMany * 6 + arguments * 2;
int crazyLongFunction(int withMany, int manyMany, int arguments) =>
    withMany * 3 + manyMany * 6 + arguments * 2;

...then I find it much harder to tell what's going on, because to interpret the indented lines, I have to look at the end of the previous line. With blocks:

int crazyLongFunction(int withMany, int manyMany, int arguments) {
    return withMany * 3 + manyMany * 6 + arguments * 2;
}
int crazyLongFunction(int withMany, int manyMany, int arguments) {
    return withMany * 3 + manyMany * 6 + arguments * 2;
}
int crazyLongFunction(int withMany, int manyMany, int arguments) {
    return withMany * 3 + manyMany * 6 + arguments * 2;
}

...then I don't have to move my eyeline away from the left margin. I can immediately tell that the expression is in a block, I can immediately tell it's a return value, I can immediately tell where the block begins.

[Hixie]

Now that dartfmt has the --fix mode, it seems like this is now in-scope again, FWIW.

[munificent]

Yes, this could be a fix.

In practice, I'm not sure how many users would actually apply it. I see many member bodies that use => and where the expression is longer than one line.

Technically speaking, this is challenging to implement because of how the formatter is architected. It doesn't know how long a line is until after calculating line splitting and indentation. But once it's done that, it doesn't expect the text to change. For a fix, though, we might be able to do something clever/hacky/slow (like format it twice) since fixes aren't as perf critical as general formatting.

[leecommamichael]

I disliked having this fix enabled in Javascript. It's not fun fiddling an arrow-function into a block-scoped function. It would be clunkier in Dart, since => is only valid for lambdas.

This was usually to sneak a console.log somewhere that had been tightly crafted, but it still annoyed me. As ever, it's the tradeoff between dead-simple and beautiful.

[munificent]

I'm going to close this since we're getting rid of dart format --fix in favor of dart fix: #1153. We want all non-whitespace code changes to be handled by the much more powerful dart fix. I believe it may already have lint rules and quick fixes around the different forms of function bodies.