domluna / JuliaFormatter.jl

An opinionated code formatter for Julia. Plot twist - the opinion is your own.
https://domluna.github.io/JuliaFormatter.jl/dev/
MIT License
568 stars 65 forks source link

Long expressions with binary operators in return or macrocall #174

Open tkf opened 4 years ago

tkf commented 4 years ago

Here is an output of JuliaFormatter 0.2.4:

function _()
    return some_expression *
    some_expression *
    some_expression *
    some_expression *
    some_expression *
    some_expression *
    some_expression
end

@some_macro some_expression *
some_expression *
some_expression *
some_expression *
some_expression *
some_expression *
some_expression

function _()
    return some_expression |> some_expression |> some_expression |> some_expression |>
           some_expression |> some_expression |> some_expression
end

@some_macro some_expression && some_expression && some_expression &&
            some_expression && some_expression && some_expression && some_expression

I think there are a few problems:

Combining these points, I think it'd be nice if the output is something like

function _()
    return some_expression *
        some_expression *
        some_expression *
        some_expression *
        some_expression *
        some_expression *
        some_expression
end

@some_macro some_expression *
    some_expression *
    some_expression *
    some_expression *
    some_expression *
    some_expression *
    some_expression

function _()
    return some_expression |>
        some_expression |>
        some_expression |>
        some_expression |>
        some_expression |>
        some_expression |>
        some_expression
end

@some_macro some_expression |>
    some_expression |>
    some_expression |>
    some_expression |>
    some_expression |>
    some_expression |>
    some_expression
domluna commented 4 years ago

This is due to parsing. All three of these have different precedence and hence are parsed differently. Now in the case of a formatter it's arguable this doesn't matter and so all of these cases should be normalized such that the output is consistent.

That aside the formatting for chainopcall in this case should be better. I think that should be an easy fix.

* example:

FunctionDef  175(175)
 FUNCTION  9(8)
 Call  8(3)
  _  1(1)
  (
  )
 Block  155(154)
  Return  155(154)
   RETURN  7(6)
   ChainOpCall  148(147)
    some_expression  16(15)
    OP: STAR  6(1)
    some_expression  16(15)
    OP: STAR  6(1)
    some_expression  16(15)
    OP: STAR  6(1)
    some_expression  16(15)
    OP: STAR  6(1)
    some_expression  16(15)
    OP: STAR  6(1)
    some_expression  16(15)
    OP: STAR  6(1)
    some_expression  16(15)
 END  3(3)

|> example:

FunctionDef  169(168)
 FUNCTION  9(8)
 Call  8(3)
  _  1(1)
  (
  )
 Block  148(147)
  Return  148(147)
   RETURN  7(6)
   BinaryOpCall  141(140)
    BinaryOpCall  122(121)
     BinaryOpCall  103(102)
      BinaryOpCall  73(72)
       BinaryOpCall  54(53)
        BinaryOpCall  35(34)
         some_expression  16(15)
         OP: RPIPE  3(2)
         some_expression  16(15)
        OP: RPIPE  3(2)
        some_expression  16(15)
       OP: RPIPE  3(2)
       some_expression  16(15)
      OP: RPIPE  14(2)
      some_expression  16(15)
     OP: RPIPE  3(2)
     some_expression  16(15)
    OP: RPIPE  3(2)
    some_expression  16(15)
 END  4(3)

&& example:

FunctionDef  169(168)
 FUNCTION  9(8)
 Call  8(3)
  _  1(1)
  (
  )
 Block  148(147)
  Return  148(147)
   RETURN  7(6)
   BinaryOpCall  141(140)
    some_expression  16(15)
    OP: LAZY_AND  3(2)
    BinaryOpCall  122(121)
     some_expression  16(15)
     OP: LAZY_AND  3(2)
     BinaryOpCall  103(102)
      some_expression  16(15)
      OP: LAZY_AND  3(2)
      BinaryOpCall  84(83)
       some_expression  16(15)
       OP: LAZY_AND  14(2)
       BinaryOpCall  54(53)
        some_expression  16(15)
        OP: LAZY_AND  3(2)
        BinaryOpCall  35(34)
         some_expression  16(15)
         OP: LAZY_AND  3(2)
         some_expression  16(15)
 END  4(3)
tkf commented 4 years ago

Thanks for the explanation. It sounds challenging when you have different associativity and also ChainOpCall.

By the way, I think something like

function _()
    return some_expression * some_expression * some_expression * some_expression *
        some_expression * some_expression * some_expression
end

is also a good alternative. I guess it's better when you have many short terms like

function _()
    return a * a * a * a * a * a * a * a * a * a * a * a * a * a * a * a * a * a * a * a *
        a * a * a * a * a * a
end

Also, I wonder what should happen when different operators are mixed. Maybe something like this?

function _()
    return a => a => a => a => a => a => a => a => a => a => a => a => a =>
        a + a + a + a + a + a +
        a * a * a * a * a * a * a +
        a + a + a + a + a + a =>
        a => a => a => a
end

That is to say, use the lower precedence operator at the end of the line when changing the operators. Or maybe it's better to go ahead and automatically insert parentheses in this case?

function _()
    return a => a => a => a => a => a => a => a => a => a => a => a => a => (
        a + a + a + a + a + a + (
            a * a * a * a * a * a * a
        ) + a + a + a + a + a + a
    ) => a => a => a => a
end
domluna commented 4 years ago

Also, I wonder what should happen when different operators are mixed. Maybe something like this?

A Chainopcall occurs when the same operator is chained 3 or more times. Of course there are exceptions to this such as &&, ||, &, |, |>, etc

This is a really tricky situation though, there are all sort of gotchas. I've tried a couple of times to come up with a unified approach but haven't gotten anything I'm satisfied with.

I looks like we're more consistent than Prettier though:

Original

const a = argument1 * argument + argument3 * argument4

const b = argument1 * argument * argument3 * argument4

Julia output:

const a =
    argument1 *
    argument +
    argument3 *
    argument4

const b =
    argument1 *
    argument *
    argument3 *
    argument4

Prettier output:

const a =
    argument1 *
        argument +
    argument3 *
        argument4;

const b =
    argument1 *
    argument *
    argument3 *
    argument4;

ref

tkf commented 4 years ago

Yeah, I can imagine that it would be a very hairy thing to write...

domluna commented 4 years ago

The examples here are better off now but I would say this is an on-going dance between conveying precedence and aesthetic output.

mlhetland commented 3 years ago

Not entirely sure if this is exactly the same issue, but it seems to be. My example includes binary relations in macro calls, as in JuMP. For example:

function foobar()
    # ...
    for i = 1:n, j=1:m
        @constraint(model, x[i] <= f(j) +
                    (f(j+i)-f(j)) * sum(y[i,k]*z[i,k]-j for k=1:p))
    end
end

If I format this with margin 80, I end up with:

function foobar()
    # ...
    for i = 1:n, j = 1:m
        @constraint(
            model,
            x[i] <=
            f(j) + (f(j + i) - f(j)) * sum(y[i, k] * z[i, k] - j for k = 1:p)
        )
    end
end

Here I'd ideally want the right-hand side to begin on the same line as the operator (<=), though at least some indentation would be useful, I guess.