Rustfmt: In Rust 2018, `use ::foo::bar` and `use foo::bar` are distinct

Created on 15 Oct 2018  路  20Comments  路  Source: rust-lang/rustfmt

I had a problem where rustfmt was reformatting use ::foo::bar to use foo::bar -- the problem is that, in Rust 2018, use ::foo::bar is sometimes needed to disambiguate:

See e.g.

mod rayon { }

use rayon::join;

fn main() {
    join(|| (), ||());
}

which -- in Rust 2018 -- fails with an ambiguity error (playground) if you have rayon in the prelude. Changing to use ::rayon::join disambiguates and is accepted (playground).

bug p-high

Most helpful comment

Todos:

  • [x] rustfmt: add --edition command line option
  • [x] cargo-fmt: read Cargo.toml and pass --edition flag to rustfmt if necessary

I think that when multiple editions are specified, the precedence should be

  1. edition passed as command line argument
  2. edition specified in Cargo.toml
  3. edition specified in rustfmt.toml
  4. the default edition (2015 for now)

All 20 comments

You need to add edition = "2018" to rustfmt.toml. Or should rustfmt defaults to edition 2018?

Why don't rustfmt read edition = "2018" in Cargo.toml

Why don't rustfmt read edition = "2018" in Cargo.toml

Good point. No technical reason, we did not consider it at the first place 馃槥
I think rustfmt should just overwrite the edition config option if the edition is set in Cargo.toml.

I think rustfmt should definitely not require a rustfmt.toml to work. I didn't even know that was a thing.

(OK, I guess I did, but I had forgotten.)

Also: I tend to run rustfmt via the emacs integration, which may or may not be passing --edition etc. I think we should probably be conservative if we don't have a "positive indication" of the edition.

Basically I'm just saying: we should consider all the ways people run rustfmt :)

Does cargo fmt pass --edition to rustfmt? I think that's an important part of the workflow.
(but people running rustfmt manually seems a bit worrying wrt Cargo-centric bits)

OK, we need to make a decision here, I guess. I think cargo-fmt should definitely just check the Cargo.toml, and then rustfmt.toml if there is no edition specified. In principle rustfmt either as a binary or a library should be able to run independent of Cargo, so we can't rely on Cargo.toml. However, should we look for Cargo.toml if there is one? I think it is right to default to 2015, since that is what Rust does too. However, I'm not sure if that leaves us in a good place for people running rustfmt via an editor or the RLS (I think the RLS could set the edition, if it doesn't already. I assume editors can't do that?).

cc @topecongiro @scampi @YaLTeR @otavio

My understanding is that when Rust 2018 is released, cargo will default to it ... or am I mistaken?

If an editor would call rustfmt on save, it is the editor's responsibility to properly pass the --edition accordingly. If using RLS it is alleviated as the language server must be aware of the edition in use.

@nikomatsakis

Also: I tend to run rustfmt via the emacs integration, which may or may not be passing --edition etc. I think we should probably be conservative if we don't have a "positive indication" of the edition.

I do as well. I use lsp-mode and in this case, it relies on RLS to know about the edition in use, I assume. If the editor does it by itself, it is on its own to "discover" it and Cargo.toml seems to be the first logical place to look I guess.

@nikomatsakis I assume that for "conservative" you mean to follow the default as Rust compiler uses. Am I right?

I agree that rustfmt should check Cargo.toml for edition (since it already checks rustfmt.toml, I don't see anything wrong with checking another relevant file).

As for the default behavior (no Cargo.toml and not specified in rustfmt.toml), I think defaulting to the 2015 edition is the right choice in order to stay compatible with the current default behavior. That is, if someone's currently formatting free-standing .rs files with default settings, they won't get sudden breakages.

I never really ran rustfmt from editors by itself, only through the RLS (which should be able to report the correct edition), doesn't rustfmt read rustfmt.toml when ran through editors? If not then it should probably be editors' responsibility to set the correct edition (so rustfmt should support a flag like that).

Todos:

  • [x] rustfmt: add --edition command line option
  • [x] cargo-fmt: read Cargo.toml and pass --edition flag to rustfmt if necessary

I think that when multiple editions are specified, the precedence should be

  1. edition passed as command line argument
  2. edition specified in Cargo.toml
  3. edition specified in rustfmt.toml
  4. the default edition (2015 for now)
  1. edition specified in Cargo.toml
  2. edition specified in rustfmt.toml

Maybe also show a warning if it comes down to these two and they explicitly specify different editions? Like "warning: different editions specified in Cargo.toml and rustfmt.toml; using the X edition".

  • [ ] check that there is API for passing the edition in the config and that the RLS is doing this correctly (based on Cargo.toml).

I'd like to inform I am looking at this issue, so there is no duplicated work.

  • [x] - stabilise the edition option

What is missing on this one?

I think that the API is already covered by https://github.com/rust-lang-nursery/rustfmt/commit/2eab9714e457cd5d7715a880cb4535e453e861e7. The stabilization is handled by #3134.

The missing piece here is to prove out the API by having the RLS use it. Once we have that, then we should release Rustfmt and the RLS.

Was this page helpful?
0 / 5 - 0 ratings