Operator precedence

[dhardy]

Currently Rustfmt doesn't seem to care much about operator precedence. IMHO it should.

Single-line

Example:

x*x + y*y

vs:

x * x + y * y

Arguably, this is a matter of preference, and the default (latter option) should likely not be changed.

Support for the former style may be nice, but may depend on heuristics:

• only allow omission of spaces around operators where the operands are small
• possibly also allow omission next to brackets (e.g. 4*(x + y))
• possibly only do this in combination with another, lower-priority operator

The last option (changing behaviour depending on presence of other operators) sounds undesirable, but consider the following example:

for i in 0..n {
    println!("{}", i);
}

looks fine so far, but:

for i in 0..n + 1 {
    println!("{}", i);
}

this confuses operator priorities! Currently this can be worked around with the spaces_around_ranges option, which simply forces spaces in all ranges, but is otherwise usually not desirable (and probably why this option exists).

In this case, there are two sensible ways of formatting the line; equal spacing:

for i in 0 .. n + 1 {
    println!("{}", i);
}

and spacing emphasising operator precedence:

for i in 0 .. n+1 {
    println!("{}", i);
}

Which option to choose should depend on a configuration option, quite likely the same option as for the x*x + y*y example above.

Multi-line

This was worked on already in one specific case (logical || operator): #3034

As another example, consider this diff from applying rustfmt to rand_distr:

@@ -259,7 +259,8 @@ impl Distribution<u64> for Binomial {
                     (13860. - (462. - (132. - (99. - 140. / a2) / a2) / a2) / a2) / a / 166320.
                 }
 
-                if alpha > x_m * (f1 / x1).ln()
+                if alpha
+                    > x_m * (f1 / x1).ln()
                     + (n - (m as f64) + 0.5) * (z / w).ln()
                     + ((y - m) as f64) * (w * p / (x1 * q)).ln()
                     // We use the signs from the GSL implementation, which are

Granted, this is a horrible example to start with (possibly an extra let binding should be used). Also, granted, the current formatting of this example is wrong in exactly the way complained about in #3034: tightly-binding items are split before the less-tightly binding > operator. The problem with rustfmt's version is that it considers the > and + operators as having equal priority.

Similar to function arguments and struct fields, I believe there should be options for how much splitting-over-lines occurs in an example like this, and the correct option with maximal splitting-over-lines would be to use an extra level of indentation for each additional level of priority:

    if alpha
        > x_m * (f1 / x1).ln()
            + (n - (m as f64) + 0.5) * (z / w).ln()
            + ((y - m) as f64) * (w * p / (x1 * q)).ln()
            // We use the signs from the GSL implementation, which are
            // different than the ones in the reference. According to
            // the GSL authors, the new signs were verified to be
            // correct by one of the original designers of the
            // algorithm.
            + stirling(f1)
            + stirling(z)
            - stirling(x1)
            - stirling(w)
    {
        continue;
    }

Note that > gets pushed to a new line to indicate visually that it binds less closely than the following operators (*, +), and also that * and / may be used within a line, but that each + or - (outside of brackets) forces a new line.

This style should be used as a result of the following rules:

1. the section alpha .. - stirling(w) is too long for a line, so split on loosest-binding operators
2. the sub-section x_m .. - stirling(w) is too long for a line, so split on loosest-binding operators
3. the section x_m * (f1 / x1).ln() fits on a line (same for each line following)
4. since we split on a tighter-binding operator, we must also split on >

(Note: rustfmt is already happy to leave this result alone if already formatted this way. However, the results if the indentation level of the if statement is incorrect are quite surprising.)

Another example, with multiple levels of indentation:

        match (self, other) {
            (&U32(ref v1), &U32(ref v2)) => v1 == v2,
            (&USize(ref v1), &USize(ref v2)) => v1 == v2,
            (&U32(ref v1), &USize(ref v2)) => {
                v1.len() == v2.len()
                && v1.iter().zip(v2.iter()).all(|(x, y)| *x as usize == *y)
            }
            (&USize(ref v1), &U32(ref v2)) => {
                v1.len() == v2.len()
                && v1.iter().zip(v2.iter()).all(|(x, y)| *x == *y as usize)
            }
        }

=> is just another operator, no? This one already has special handling, but arguably doesn't need it (save for the convention of using { .. } brackets around multi-line RHS content and the option to omit separating commas).