Commit Graph

12 Commits

Author SHA1 Message Date
David Tolnay
cf412e0d6b rustfmt: Use use_try_shorthand default
Summary:
I observed that for whatever reason our setting of `use_try_shorthand = true` in rustfmt.toml was causing entire functions to not get processed by rustfmt. Even files that contain neither `try` nor `?`. Remove it and reformat fbsource.

Documentation of that config:

- https://github.com/rust-lang/rustfmt/blob/master/Configurations.md#use_try_shorthand

We don't particularly care about the value anymore because nobody writes `r#try!(...)` in 2018 edition code.

Minimized:

```
fn f() {
    g(
        )
        // ...
        .h
}
```

This function gets formatted only if use_try_shorthand is not set.

The bug is fixed in the rustfmt 2.0 release candidate.

Reviewed By: jsgf

Differential Revision: D21878162

fbshipit-source-id: b028673c0eb703984d24bf0d2983453fc2a8c212
2020-06-10 19:29:15 -07:00
David Tolnay
c854d5ef5b Remove configurations where we are using the default
Summary: We don't need to include settings in rustfmt.toml where we just use the default value. Maybe we set these back when they used to have a different default value.

Reviewed By: zertosh

Differential Revision: D21497076

fbshipit-source-id: 687d1aef1af9aca33261f679a91cc049bbf70d0a
2020-05-10 16:06:06 -07:00
Michael Bolin
8c724108fb Get rustfmt/rls working in fbcode again.
Summary:
tl;dr Got things working again by ignoring the `--config-path` from rls.

Despite my efforts in D17089540, `rustfmt` appeared to have stopped
working again in VS Code. I thought maybe something changed in our move
to 1.38.0-beta.3. I saw something about a `VersionMismatch` in the
console (which was the issue last time), but unfortunately the error
did not include what the expected and observed versions were, so I
built `rustfmt` from source locally and modified it to include this
information (which I should try to upstream).

Unfortunately, our VS Code/rls setup is not great when it comes to
logging errors to a file, so once again I ran:

```
sudo execsnoop.py -n rust
```

and observed the external `rustfmt` command that `rls` ultimately
constructed and executed so I would have a simpler repro case.
Incidentally, after rls had a `rustfmt` error of some sort, it seemed
to end up in a broken state, so debugging the issue outside of rls
was a lot simpler. Anyway, here was an example of such a command I saw
from `execsnoop`:

```
/data/users/mbolin/eden-fbsource/fbcode/common/rust/tools/common/rustfmt_wrapper.sh --unstable-features --skip-children --emit stdout --quiet --config-path /dev/shm/tmp/i0gVWxrcpp
```

As explained in D17089540, `/dev/shm/tmp/i0gVWxrcpp` ends up being this
fully serialized `rustfmt` configuration. I took this command and repro'd
by making a local formatting error and piping the file in:

```
/data/users/mbolin/eden-fbsource/fbcode/common/rust/tools/common/rustfmt_wrapper.sh --unstable-features --skip-children --emit stdout --quiet --config-path /dev/shm/tmp/i0gVWxrcpp < scm/dotslash/src/main.rs | head
```

and got:

```
Warning for license template file "": No such file or directory (os error 2)
1 282 282
// Copyright 2004-present Facebook. All Rights Reserved.

mod backend;
mod config;
#[cfg(not(build = "instrumentation"))]
mod curl;
mod curl_args;
mod decompress;
mod dotslash_cache;
```

What was that garbage at the top about a license template file? I ended up
looking at `/dev/shm/tmp/i0gVWxrcpp` and found this line:

```
license_template_path = ""
```

Apparently the default value for this configuration option is `""`
and `rustfmt` tries to open the path without bothering to check whether
it is the empty string:

4449250539/src/config/license.rs (L215-L221)

Great. (I filed an issue for this at https://github.com/rust-lang/rustfmt/issues/3802.)

It finally dawned on me that between `required_version` and `license_template_path`,
this generated config file from rls  was not doing us any favors. Since we are already
using `rustfmt_wrapper.sh` to rewrite some options that we care about, why not rewrite
`--config-path` to use the `.rustfmt.toml` config in fbcode?

So that's what I did and now everything seems to work again.

Also, I appeared to have figured out why we need a wrapper script in the first place!
rls tries to set the `emit_mode` to `ModifiedLines`, which would generate the output
from rustfmt in the way that rls wants. It looks like someone then made reading from
stdin incompatible with `emit_mode=ModifiedLines`. I filed this as:

https://github.com/rust-lang/rls/issues/1555

Reviewed By: dtolnay

Differential Revision: D17492254

fbshipit-source-id: 3415bdab3c1030d3082ae2b8fab0c2e6b312534a
2019-09-19 18:06:23 -07:00
Michael Bolin
563c5ac18e Update version of rustfmt specified by .rlsconfig.
Summary:
First, checked the following to ensure that all existing `.rlsconfig`
files inherited their `rustfmt` setting from `fbcode/.rlsconfig`:

```
fbcode$ fbgf -s -l .rlsconfig | xargs rg rustfmt
.rlsconfig
5:  "rustfmt": "$BUCKROOT/common/rust/tools/common/rustfmt_wrapper.sh",

experimental/ljw/r6/.rlsconfig
4:  "rustfmt": "$BUCKROOT/common/rust/tools/common/rustfmt_wrapper.sh",

experimental/ljw/r4/.rlsconfig
4:  "rustfmt": "$BUCKROOT/common/rust/tools/common/rustfmt_wrapper.sh",
```

Effectively, this means that all uses of `rustfmt` via our internal
Rust extension for VS Code/Nuclide should go through this script.

Though to make things complicated, when an external `rustfmt` is specified
to `rls`, it performs this step where it takes a complete in-memory config
and serializes it to a temp file, using it as the value of `rustfmt`'s
`--config-path` option.

Unfortunately for us, that includes a config value named `required_version`:

https://github.com/rust-lang/rustfmt/blob/master/Configurations.md#required_version

It defaults to `CARGO_PKG_VERSION`, so for the current `rls` in tp2, this
ends up being `1.2.2` even though we want to use it with an external `rustfmt`
whose version is `1.3.3`. We can workaround this by specifying
`required_version = "1.3.3"` in our own `rustfmt.toml` so that the value in
the serialized config matches the version with which it will be used.

Reviewed By: dtolnay

Differential Revision: D17089540

fbshipit-source-id: 082ceef0f983e7cece6373603a0f318a7bc2ab2f
2019-08-28 13:14:53 -07:00
Lucian Wischik
3440129ca6 provide edition in .rustfmt.toml
Summary:
When you invoke `rustfmt` tool, it defaults to 2018 edition of formatting.

When RLS is asked to format, it first determines the edition. It will fail to do any formatting if it can't find the edition.

Putting the edition inside fbcode/.rustfmt.toml means that RLS will be willing to go ahead and format any file in fbcode. (unless that file has a more local .rustfmt.toml which omits edition).

Reviewed By: jsgf

Differential Revision: D15425617

fbshipit-source-id: 2cb20778af2383d291d22ba272fc16264741155c
2019-05-29 19:55:37 -07:00
Jeremy Fitzhardinge
1d142a6ae8 Back out D13589633 "revert D13584553, D13175922, D13166085 to fix rust builds"
Summary:
Rust thrift code generation now uses the right rustfmt.

Original commit changeset: 78c68068ddcb

Reviewed By: zertosh

Differential Revision: D13599740

fbshipit-source-id: be0ae80d36302be3f6f3532ab327c60ba4d460ef
2019-01-09 13:31:19 -08:00
Stanislau Hlebik
629484f646 revert D13584553, D13175922, D13166085 to fix rust builds
Summary:
D13166085 breaks Mononoke builds. We got errors

```
rustfmt may have failed to format. See previous 119 errors.
```

These errors happen on formatting thrift files. It looks like the problem is
in the fact that rustfmt for linting is different from rustfmt we are using
while building rust thrift files - https://fburl.com/hn0g95wt. But they seem to
use the same config files, which are incompatible.

For now revert the diff to unblock the builds. Also we have to revert D13584553 and D13175922
because it depends on D13166085.

Reviewed By: lukaspiatkowski

Differential Revision: D13589633

fbshipit-source-id: 78c68068ddcb4c594fd1463e98b48f8f438e46a2
2019-01-07 06:19:27 -08:00
Jeremy Fitzhardinge
75db1ea8bf linter: use in-tree rustfmt binary for Rust formatting
Summary:
Use rustfmt binaries committed to tools/third-party/rustfmt for
formatting.

Reviewed By: zertosh

Differential Revision: D13166085

fbshipit-source-id: 2ba9568c3f463740b3e48b72196ef7c716e20784
2019-01-04 14:42:26 -08:00
Siddharth Agarwal
df1590c586 updates for rustfmt 0.3.4
Summary:
One config option is now obsolete, and the other one got renamed.

(Note: this ignores all push blocking failures!)

Reviewed By: farnz

Differential Revision: D6668021

fbshipit-source-id: aff949662a9584b7aead3f571512787aa1e9a260
2018-01-05 12:07:01 -08:00
Siddharth Agarwal
e5315a765b config for rustfmt-nightly 0.2.6
Summary:
Without this, any lines that are too long would cause `rustfmt` to
error out. That seems not very useful.

Reviewed By: kulshrax

Differential Revision: D5853676

fbshipit-source-id: b7335a14079ebf6245da0c028039d88745b32785
2017-09-18 10:50:27 -07:00
Siddharth Agarwal
a05c88a442 add a couple of rustfmt configs for rustfmt-nightly 0.2.5
Summary:
These prevent most errors while running the new rustfmt against
Mononoke.

Reviewed By: jsgf

Differential Revision: D5760091

fbshipit-source-id: 42d73fd282bf3c9cf56db1138e4f63c4c4416c41
2017-09-05 17:28:34 -07:00
facebook-github-bot
2b6af6b941 Initial commit
fbshipit-source-id: f75baa4ff6aa71973f677b752d7aba582cf4927f
2017-07-27 18:00:19 -07:00