Apply rustfmt formatting (partial)

[dhardy]

Rustfmt is now widely used and mostly gives a good result. This PR:

• accepts changes which are reasonable (some a clear improvement, some neither much better nor worse)
• explicitly skips formatting in a few cases
• applies manual reformatting in a few cases (to bring formatting closer to rustfmt's style)
• thus makes Rand more consistent with typical Rust styles and easier to use rustfmt on

However, some reformatting was rejected without a skip command:

• in some cases, content is split over too many or too few lines, impairing readability
• some of the arrays used in tests are problematic
• there is an incident of incorrect operator binding priority: index..=index + 1
• some vertical alignment is lost (I would just use rustfmt::skip, but it can't be applied to if expressions or inside blocks)

I have created several new issues on the rustfmt repository; ideally the remaining formatting issues can be resolved via rustfmt improvement.
For now, we still can't just run cargo fmt on the project.

[dhardy]

Since no one else has reviewed this and I'm not "allowed" to self-review (GH restriction), I'm just going to make some comments and merge anyway:

• I'm pretty sure there are no semantic changes here (other than to rustfmt config)
• Syntactic changes are mixed and use a few more lines overall, but for the most part are an improvement
• Aligning with Rustfmt formatting is (broadly speaking) sensible
• It's a shame this PR doesn't get all the way there

[vks]

I think this decreases readability, but I guess it's good enough.

[dhardy]

Yeah, I guess you're right. Feel free to post rustfmt config changes / a skip annotation if you prefer.

I'd really like to get the point where we can just run cargo fmt on the project so would prefer not to just revert stuff like this — but it may not be until Rustfmt 2.0 that we get there (and even then, we'll have to make further compromises).

[vks]

I think there are some minor readability regressions, where the code is spread over too many lines, but I think this is a small annoyance outweighed by the advantages of automatic formatting.

[dhardy]

Glad we agree on that then.

Try running cargo fmt locally and see if you have ideas for dealing with that lot :-/

[vks]

Ok, here are my observations with rustfmt 1.4.11-nightly (1838235 2019-12-03):

I think the following changes will hopefully improve in a later rustfmt version:

• ..a + b should be formatted reflecting the operator precedence (see the issue you opened).
• Some copy_nonoverlapping calls get reformatted with too much stuff on one line. This is a case were the soft 80 characters limit should be preferred over the hard 100 characters limit. There are a few other similar cases.
• The ziggurat calls get reformatted in a less readable way.

I think some changes have to be manually disabled by annotating the source code:

• Arrays of "random" numbers get reformatted, hurting readability by spreading them over more lines. (I remember this being an issue with clang-format as well. I don't think this will ever get fixed.)
• The alignment of the HC128 code gets broken. The current formatting is non-standard, so I think we have to add an exception if we want to keep it.

In my opinion, the remaining changes suggested by rustfmt are acceptable (even if not great).

This actually looks better:

diff --git a/src/distributions/binomial.rs b/src/distributions/binomial.rs
index c096e4a862..53363c6e99 100644
--- a/src/distributions/binomial.rs
+++ b/src/distributions/binomial.rs
@@ -229,7 +229,7 @@ impl Distribution<u64> for Binomial {
                 }
 
                 if alpha
-                        > x_m * (f1 / x1).ln()
+                    > x_m * (f1 / x1).ln()
                         + (n - (m as f64) + 0.5) * (z / w).ln()

[dhardy]

Generally I agree, though there are a couple of cases where current formatting deliberately breaks over multiple lines.

Arrays of "random" numbers get reformatted,

I think the only way this type of thing can be done correctly is to weight styles by similarity to previous/next lines as well as a soft length limit.

The alignment of the HC128 code gets broken.

There is some interest in vertical alignment; it's already implemented for a couple of things (e.g. enum values).

This actually looks better:

It appears I got this wrong (but right in the rand_distr equivalent). It's one of the examples I brought up in the operator precedence issue.