Rustfmt: Evaluate rustfmt on the Rust repo

Created on 21 Sep 2017  路  18Comments  路  Source: rust-lang/rustfmt

Pick a crate from the Rust repo (I'd start small), run rustfmt. Check that

  • rustfmt does not crash
  • rustfmt completes in a reasonable amount of time
  • the code compiles afterwards
  • tests (and in particular the tidy check) still pass
  • as best as possible, check that there are no mistakes in the reformatting (there might be a lot of changes, so being 100% thorough is not going to be possible)

Then either file issues on this repository or report back in this issue that everything is looking good.

Note that we shouldn't send PRs to Rust just yet, we want to know that this works. Actually making the changes depends on some policy decisions and should be fairly mechanical.

TODO list

  • [x] liballoc (thanks @opilar)
  • [ ] liballoc_jemalloc
  • [ ] liballoc_system
  • [ ] libarena
  • [ ] libbacktrace
  • [ ] libcollections
  • [ ] libcompiler_builtins
  • [x] libcore (thanks @topecongiro) - issues to address
  • [ ] libfmt_macros
  • [ ] libgetopts
  • [ ] libgraphviz
  • [ ] liblibc
  • [ ] libpanic_abort
  • [ ] libpanic_unwind
  • [ ] libproc_macro
  • [ ] libprofiler_builtins
  • [ ] librand
  • [ ] librustc
  • [ ] librustc_allocator
  • [ ] librustc_apfloat
  • [ ] librustc_asan
  • [ ] librustc_back
  • [ ] librustc_borrowck
  • [ ] librustc_const_eval
  • [ ] librustc_const_math
  • [ ] librustc_cratesio_shim
  • [ ] librustc_data_structures
  • [x] librustc_driver
  • [x] librustc_errors
  • [x] librustc_incremental
  • [x] librustc_lint
  • [x] librustc_llvm
  • [x] librustc_lsan
  • [x] librustc_metadata
  • [x] librustc_mir
  • [x] librustc_msan
  • [x] librustc_passes
  • [x] librustc_platform_intrinsics
  • [x] librustc_plugin
  • [x] librustc_privacy
  • [x] librustc_resolve
  • [x] librustc_save_analysis
  • [x] librustc_trans
  • [x] librustc_trans_utils
  • [x] librustc_tsan
  • [x] librustc_typeck
  • [x] librustdoc
  • [x] libserialize
  • [x] libstd
  • [x] libstd_unicode
  • [x] libsyntax
  • [x] libsyntax_ext
  • [x] libsyntax_pos
  • [x] libterm (thanks @smt923)
  • [x] libtest
  • [x] libunwind
fun!

Most helpful comment

I fixed the above tidy errors by hand and tested cargo fmt --all and ./x.py test again. This time I hit some bugs (#2212, #2214 and a tidy check panic caused by formatting declare_features!() calls in libsyntax/feature_gate.rs). I fixed them, tried again, and finally all the tests have passed without any error. FWIW diffs can be seen here.

All 18 comments

cc @nikomatsakis

Picked one off at random, libterm compiles and passes all tests after having rustfmt ran on it, completed within a second and a quick check of the code seems fine, definitely no expert though and this is my first time even compiling bits of the compiler/std, I couldn't fully work out how to manually run tidy checks on just a single crate - if that's even possible - but other than that it appears to be fine

@smt923 awesome, thanks! I don't think you can run tidy on a single crate, but running it on the whole project shouldn't take too long.

Run on liballoc:

Rustfmt failed at /Users/opilar/rust/rust/src/liballoc/btree/node.rs:585: line exceeded maximum length (maximum: 100, found: 101) (sorry)

The same on libcore:

Rustfmt failed at /Users/opilar/rust/rust/src/libcore/ops/try.rs:29: line exceeded maximum length (maximum: 100, found: 116) (sorry)
Rustfmt failed at /Users/opilar/rust/rust/src/libcore/ops/try.rs:34: line exceeded maximum length (maximum: 100, found: 106) (sorry)
Rustfmt failed at /Users/opilar/rust/rust/src/libcore/fmt/mod.rs:1240: line exceeded maximum length (maximum: 100, found: 116) (sorry)
Rustfmt failed at /Users/opilar/rust/rust/src/libcore/macros.rs:53: line exceeded maximum length (maximum: 100, found: 103) (sorry)

And librustc:

Rustfmt failed at /Users/opilar/rust/rust/src/librustc/macros.rs:76: line exceeded maximum length (maximum: 100, found: 127) (sorry)
Rustfmt failed at /Users/opilar/rust/rust/src/librustc/macros.rs:79: line exceeded maximum length (maximum: 100, found: 102) (sorry)
Rustfmt failed at /Users/opilar/rust/rust/src/librustc/macros.rs:80: line exceeded maximum length (maximum: 100, found: 123) (sorry)
Rustfmt failed at /Users/opilar/rust/rust/src/librustc/macros.rs:95: line exceeded maximum length (maximum: 100, found: 129) (sorry)
Rustfmt failed at /Users/opilar/rust/rust/src/librustc/macros.rs:98: line exceeded maximum length (maximum: 100, found: 102) (sorry)
Rustfmt failed at /Users/opilar/rust/rust/src/librustc/macros.rs:99: line exceeded maximum length (maximum: 100, found: 123) (sorry)
Rustfmt failed at /Users/opilar/rust/rust/src/librustc/macros.rs:109: line exceeded maximum length (maximum: 100, found: 129) (sorry)
Rustfmt failed at /Users/opilar/rust/rust/src/librustc/macros.rs:112: line exceeded maximum length (maximum: 100, found: 102) (sorry)
Rustfmt failed at /Users/opilar/rust/rust/src/librustc/macros.rs:113: line exceeded maximum length (maximum: 100, found: 123) (sorry)
Rustfmt failed at /Users/opilar/rust/rust/src/librustc/traits/mod.rs:241: line exceeded maximum length (maximum: 100, found: 103) (sorry)
Rustfmt failed at /Users/opilar/rust/rust/src/librustc/traits/mod.rs:510: line exceeded maximum length (maximum: 100, found: 101) (sorry)
Rustfmt failed at /Users/opilar/rust/rust/src/librustc/session/mod.rs:696: line exceeded maximum length (maximum: 100, found: 103) (sorry)

@opilar thanks! You may not be using the latest rustfmt, could you please try again with the latest rustfmt-nightly?

On libcore, cargo fmt ends within a second. Diff. There are 4 errors:

error: line exceeded maximum width (maximum: 100, found: 103)
  --> /home/uchida/rust/src/libcore/macros.rs:53
   |
53 | /// [testing]: ../book/second-edition/ch11-01-writing-tests.html#checking-results-with-the-assert-macro
   |                                                                                                     ^^^
   = note: use `error_on_line_overflow_comments = false` to suppress the warning against line comments

error: line exceeded maximum width (maximum: 100, found: 116)
    --> /home/uchida/rust/src/libcore/fmt/mod.rs:1237
     |
1237 |                     const ZEROES: &'static str = "0000000000000000000000000000000000000000000000000000000000000000";
     |                                                                                                     ^^^^^^^^^^^^^^^^

error: line exceeded maximum width (maximum: 100, found: 116)
  --> /home/uchida/rust/src/libcore/ops/try.rs:29
   |
29 |                                          label = "cannot use the `?` operator in a function that returns `{Self}`"),
   |                                                                                                     ^^^^^^^^^^^^^^^^

error: line exceeded maximum width (maximum: 100, found: 106)
  --> /home/uchida/rust/src/libcore/ops/try.rs:34
   |
34 |                                          label = "the `?` operator cannot be applied to type `{Self}`")))]
   |                                                                                                     ^^^^^^

warning: rustfmt may have failed to format. See previous 4 errors.

The erros on comment and big strings in attributes are fine IMO. However, rustfmt should be able to fit the const ZEROS: &'static str = "00.."; within max width by splitting after =.

Tested some crates. I have not tested that code compiles after cargo fmt or tidy tests. I have confirmed that cargo fmt is idempotent by running it twice.

Environment

  • rustmft version: 0.2.8
  • rustc: 1.22.0-nightly (6f87d20a7 2017-09-29)
  • CPU: Core i3-4150 3.50GHz
  • RAM: 8GB

Results Overview

| crate | time | error | note |
| --- | --- | --- | --- |
| librustc_mir | 0.5s | 2 errors (#2022) | |
| librustc_msan | < 0.1s | no error | missing offet in attributes (#2017) |
| librustc_passes | < 0.1s | no error | |
| librustc_platform_intrinsics | 0.5s | no error | |
| librustc_plugin | < 0.1s | no error | |
| librustc_privacy | < 0.1s | no error | missing offet in attributes (cc #2017) |
| librustc_resolve | 0.2s | 5 errors (#2018) | 4 errors are on comments exceeding max width |
| librustc_save_analysis | 0.1s | 1 error | an error is on comment exceeding max width |
| librustc_trans | 0.7s | 1 error (#2021) ||
| librustc_trans_utils | < 0.1s | no error | |
| librustc_tsan | < 0.1s | no error | missing offset in attributes (cc #2017) |
| librustc_typeck | 0.6s | 1 error (#2020) | |
| librustdoc | 0.4s | no error ||
| libserialize | 0.1s | no error | |
| libstd | 1.2s | 2 errors | 2 errors are on comments exceeding max width |
| libstd_unicode | 0.3s | no error | huge arrays are getting vertically formatted |
| libsyntax | 1.0s | 16 errors (#2018) | |
| libsyntax_ext | 0.2s | 1 error (#2018) | |
| libsyntax_pos | < 0.1s | no error | |
| libtest | < 0.1s | no error | missing offset in attributes (cc #2017) |
| libunwind | < 0.1s | no error | |

Detailed Results

librustc_mir

error: line exceeded maximum width (maximum: 100, found: 105)
    --> /home/topecongiro/workspace/rust/src/librustc_mir/borrow_check.rs:1326
     |
1326 |                     ProjectionElem::Downcast(..) => ("", format!(""), None), // (dont emit downcast info)
     |                                                                                                     ^^^^^
     = note: use `error_on_line_overflow_comments = false` to suppress the warning against line comments

error: line exceeded maximum width (maximum: 100, found: 130)
    --> /home/topecongiro/workspace/rust/src/librustc_mir/borrow_check.rs:1327
     |
1327 |                     ProjectionElem::Field(field, _ty) => ("", format!(".{}", field.index()), None), // FIXME: report name of field
     |                                                                                                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
     = note: use `error_on_line_overflow_comments = false` to suppress the warning against line comments

warning: rustfmt may have failed to format. See previous 2 errors.

librustc_tsan

 #![unstable(feature = "sanitizer_runtime_lib",
-            reason = "internal implementation detail of sanitizers",
-            issue = "0")]
+           reason = "internal implementation detail of sanitizers", issue = "0")]

rustfmt misses to add a offset of !. This should be fixed by #2017.

libstd

error: line exceeded maximum width (maximum: 100, found: 116)
   --> /home/topecongiro/workspace/rust/src/libstd/lib.rs:206
    |
206 | //! [deref-coercions]: ../book/second-edition/ch15-02-deref.html#implicit-deref-coercions-with-functions-and-methods
    |                                                                                                     ^^^^^^^^^^^^^^^^
    = note: use `error_on_line_overflow_comments = false` to suppress the warning against line comments

error: line exceeded maximum width (maximum: 100, found: 111)
   --> /home/topecongiro/workspace/rust/src/libstd/net/ip.rs:507
    |
507 |     /// [ipv4-sr]: https://www.iana.org/assignments/iana-ipv4-special-registry/iana-ipv4-special-registry.xhtml
    |                                                                                                     ^^^^^^^^^^^
    = note: use `error_on_line_overflow_comments = false` to suppress the warning against line comments

warning: rustfmt may have failed to format. See previous 2 errors.

libunwind

 pub type _Unwind_Trace_Fn = extern "C" fn(ctx: *mut _Unwind_Context, arg: *mut c_void)
-                                          -> _Unwind_Reason_Code;
+    -> _Unwind_Reason_Code;

This should be formatted like the following IMO:

pub type _Unwind_Trace_Fn =
    extern "C" fn(ctx: *mut _Unwind_Context, arg: *mut c_void) -> _Unwind_Reason_Code;

libsyntax_pos

Comment after extern crate is not moved when reordering:

-extern crate serialize;
-extern crate serialize as rustc_serialize; // used by deriving
+extern crate serialize as rustc_serialize;
+extern crate serialize; // used by deriving

More tests :)

| crate | time | error | note |
| --- | --- | --- | --- |
librustc_driver | 0.2s | no error | |
librustc_errors | 0.1s | no error | |
librustc_incremental | 0.1s | no error | |
librustc_lint | 0.1s | no error | |
librustc_llvm | 0.1s | no error | |
librustc_lsan | < 0.1s | no error | |
| librustc_metadata | 0.2s | no error | |

Using rustfmt libcore takes 1.6s but has a couple of errors

  • 5 errors due to literal string or comment too long - avoidable with error_on_line_overflow* configs
  • error[E0583]: file not found for modulemem`- could be due to me not invoking correctly? It happens when doingrustfmt src/libcore/benches/lib.rs`
  • Syntactic change leading to compilation break: #2144

src/libcore/benches/lib.rs

Hmm, I wonder if there are adjustments done while in tests which rustfmt is not taking into account?

A few more, on rustfmt 0.2.15-nightly (8a27a2d 2017-11-13)

crate | time | error | note
------ | ------ | ------- | ------
libcore | 1.6s | 3 errors | 2 exceeded line widths on comments with URLs ; 1 error[E0583]: file not found for module 'mem' but this is rust's problem (also happens when running benchmarks ./x.py bench src/libcore)
libfmt_macros | <0.1s | no error |
libgetopts | <0.1s | no error |
libgraphviz | <0.1s | no error |
liblibc | 0.3s | 6 errors | 5 exceeded line widths on comments with URLs ; 1 exceeded line width on match arm with guard (#2152)

Tried cargo fmt --all from rust/src using 0.2.16-nightly (4e04e82 2017-11-28), then run ./x.py test. I got the following tidy errors:

tidy error: /home/uchida/rust/src/tools/compiletest/src/runtest.rs:1075: line longer than 100 chars
tidy error: /home/uchida/rust/src/libterm/terminfo/parser/compiled.rs:311: line longer than 100 chars
tidy error: /home/uchida/rust/src/librustc_trans/mir/mod.rs:517: line longer than 100 chars
tidy error: /home/uchida/rust/src/librustc_trans/mir/mod.rs:518: line longer than 100 chars
tidy error: /home/uchida/rust/src/librustdoc/clean/mod.rs:789: line longer than 100 chars
tidy error: /home/uchida/rust/src/librustc_resolve/lib.rs:2673: line longer than 100 chars
tidy error: /home/uchida/rust/src/librustc_resolve/lib.rs:2674: line longer than 100 chars
tidy error: /home/uchida/rust/src/librustc_resolve/resolve_imports.rs:49: line longer than 100 chars
tidy error: /home/uchida/rust/src/librustc_resolve/macros.rs:564: line longer than 100 chars
tidy error: /home/uchida/rust/src/librustc_resolve/macros.rs:565: line longer than 100 chars
tidy error: /home/uchida/rust/src/librustc_resolve/macros.rs:566: line longer than 100 chars
tidy error: /home/uchida/rust/src/librustc/traits/mod.rs:237: line longer than 100 chars
tidy error: /home/uchida/rust/src/librustc/traits/mod.rs:508: line longer than 100 chars

I'd guess they are all legitimate rustfmt errors since comments should be under 100 chars already due to the tidy scripts. I guess there could be string literals causing problems?

How long did it take to run?

Actually most of the errors are due to comments getting moved around or indented. I opened issues for them.

It took about 12 seconds on my laptop (Intel(R) Core(TM) i5-5200U CPU @ 2.20GHz, 8GM RAM and Linux 4.13.12-1-ARCH).

Nice 12 seconds is plenty fast enough, I was worried it would take minutes.

I fixed the above tidy errors by hand and tested cargo fmt --all and ./x.py test again. This time I hit some bugs (#2212, #2214 and a tidy check panic caused by formatting declare_features!() calls in libsyntax/feature_gate.rs). I fixed them, tried again, and finally all the tests have passed without any error. FWIW diffs can be seen here.

Nice!

I think this issue has run its course. I'm still interested in applying Rustfmt to large projects like Rust to find bugs and egregious mis-formatting.

Was this page helpful?
0 / 5 - 0 ratings