Help test out the 2018 module system changes!

Good morning everyone! Leading up to the Rust 2018 edition release we’d like to showcase a new upcoming feature once a week to ensure it’s well tested and well vetted for the 2018 release, ensuring that everything goes smoothly at release time! Starting off this week I’d like to sound a call to arms for the module system changes in the 2018 edition!

The module system changes in 2018 can largely be summarized with:

  • Imports of items from your own crate must start with crate:: now
  • Paths anywhere in the crate may start with crate:: or an external crate name (like std::)
  • Ideally, you shouldn’t even need extern crate any more!

The easiest way to try out this feature is to have cargo fix update all your code for you. You can do this by following the transition guide online, or following these steps:

  1. Add #![feature(rust_2018_preview)] to your crate
  2. Run cargo +nightly fix --prepare-for 2018 --all-targets --all-features
  3. Keep running until all warnings are gone!

Afterwards be sure to run cargo doc, try it out with the RLS, run rustfmt to see what happens, otherwise try to stress this feature! We want to ensure that the experience of the new module system changes is as polished as possible for the 2018 release, and we can make it that way with your help!

In addition to testing our your own code and reporting bugs, we’d love to see some community-driven tutorials/documentation/podcasts about the feature! If you’re busy this week don’t worry as well, we’ll be making a call to action for a new feature next week!

If you encounter any bugs please report them on the main Rust issue tracker. Otherwise please feel free to leave feedback here as well!

13 Likes

Ah one thing I should also mention! You can find documentation for the module systeem changes in the online edition guide as well!

1 Like

The documentation doesn’t specify what the final implementation of paths in use is.

Are use statements still always absolute? (must start with crate/crate name/self/super)

1 Like

Something I haven’t seen explained clearly in the guide yet is what the recommended approach is for aliasing using the new syntax. A small note on that would be massively helpful!

1 Like

Nit: or self:: or super::

I’ve been slowly updating my workspace one crate at a time. From what I’ve seen so far, cargo fix works spectacularly well, far exceeding my expectations. It updated even some of my most macro-heavy crates without breaking a sweat.

So far, I’ve run into three issues, two of which were really not the fault of rust, but rather they were due to minor and temporary holes in the ecosystem. I’ll share them here to help others work around them (hopefully enabling users to discover more meaningful bugs in crates higher up the dependency chain!):

failure 0.1.1 uses an old version of syn

#[derive(Fail)] panics on crate syntax because it uses an old version of syn. This is fixed on the master branch of failure, but the next release currently seems to be in limbo.

For now, to resolve this, depend on the git repo:

# (note: make sure to include the version field to minimize breaking changes,
#        as the repo actually contains two different versions of the crate!)
failure = { git = "https://github.com/rust-lang-nursery/failure", version = "0.1.2", rev = "9e1952de8e2" }

Try enabling proc-macro2's nightly feature if you use any custom derives (like serde)

cargo fix can get tripped up by errors in code generated by custom derives:

// output of 'cargo fix':
#[derive(Serialize, crate::Deserialize)]   // <--- lol oops
pub struct Thing {
    foo: ::module::Foo,
    bar: ::module::Bar,
}

While the above is due to something that is an obvious bug in rustc (in that it gives the suggestion to write “crate::Deserialize”), you’re likely still going to have trouble even once that bug is fixed, because the core issue here is that the code generated by the macro has no span information.

Luckily, serde already supports accurate span info, because it makes effective use of the proc-macro2 shim crate. All you need to do is add a direct dependency to your Cargo.toml, to enable the feature:

# note: you should check your Cargo.lock first to find out what version of
#       proc-macro2 your dependencies depend on, and use that version number

proc-macro2 = { version = "0.4.9", features = ["nightly"] }

Be aware this won’t work for all custom derives (only ones that depend on the proc-macro2 crate and use it properly).

3 Likes

Based on my reading of the transition guide, I only need to enable the rust_2018_preview feature flag, but that doesn’t seem to let me omit the extern crate part:

src/main.rs:

#![feature(rust_2018_preview)]

use regex::Regex;

fn main() {
    println!("Hello, world!");
}

Cargo.toml:

[package]
name = "testbin"
version = "0.1.0"
authors = ["Andrew Chin <achin@eminence32.net>"]

[dependencies]
regex = "*"

Yet I get this error:

error[E0432]: unresolved import `regex`
 --> src/main.rs:3:5
  |
3 | use regex::Regex;
  |     ^^^^^ Maybe a missing `extern crate regex;`?
5 Likes

I wanted to rename a crate that I was using, and I had to either use extern crate, or add use crate_a as b everywhere. I wish I could just write:

crate use foo as bar;

in my lib.rs file, and that I could just use bar normally everywhere :confused:

Also, I ended up having to add extern crate anyways to import macros. If bar is a macro in crate foo, use foo::bar should just work.

I’ve been using the preview for a couple of weeks, and the module system changes I think are for the better. These issues I mentioned are paper cuts in Rust 2018 that did not feel like paper cuts in Rust 2015, so I kind of view these as regressions.

(I know about renaming crates in the Cargo.toml, but… I don’t really know yet how I feel about that: I am still used to writing and reading these things in the lib.rs and I haven’t changed that habit yet).

2 Likes

I really like the new module system. I especially like that use statements always contain an absolute path because this makes it unambiguous where the imported module comes from. E.g. use self:: makes it clear that it’s another file from within the directory.

1 Like

Hmm. Is there/will there be a way to tease cargo fix into eliminating leading :: from paths?

In rust 2015 I used leading :: everywhere religiously (even in use, to ensure I did not forget to include it outside of use), but in rust 2018 this practice is unnecessary and can lead to ambiguous grammar. (pub struct Newtype(crate ::std::rc::Rc<[f64]>))

1 Like

I had this issue as well. The instructions above are incomplete. You need to follow the transition guide, specifically the section Commit to the next edition (after cargo fix).

Oh dear these issues sound bad! Are you sure you were testing with an up-to-date nightly? Recent changes in lint emissions should supress lints related to foreign macros (like derive in these cases) but if they’re still showing up that’s a bug!

Would you be able to share some test cases if it’s still showing problems?

That’s correct! That feature is only supported, I believe, when the 2018 edition itself is enabled via:

cargo-features = ['edition']

[package]
# ...
edition = '2018'
2 Likes

I opened a PR which attempts to clarify some of the transition steps. I welcome the community to review it! https://github.com/rust-lang-nursery/edition-guide/pull/63

How can import procedural macros in a way that is idiomatic for Rust 2018? More specifically, how should I replace the following code:

#[cfg(feature="serde1")] #[macro_use] extern crate serde_derive;

#[cfg_attr(feature="serde1", derive(Serialize, Deserialize))]
...

Edit: I figured it out:

#[cfg(feature="serde1")] use serde_derive::{Serialize, Deserialize};

#[cfg_attr(feature="serde1", derive(Serialize, Deserialize))]
...

Just wanted to mention that we unearthed a nasty bug yesterday on IRC: inside a module named “X” you cannot use an external crate named “X” because the module shadows the crate.

Issue 52705: “2018: current module shadows crate in use statement”

2 Likes

@vks indeed! Looks like you’ve found that use serde_derive::Deserialize works

@alexcrichton The latest nightly rustup is pulling down for me is nightly-x86_64-pc-windows-msvc unchanged - rustc 1.29.0-nightly (6a1c0637c 2018-07-23). It looks like this forum post was posted on the 24th. Is this expected and should I wait until a more recent nightly is available if things have been fixed in past couple of days?

> rustup show
Default host: x86_64-pc-windows-msvc

...

active toolchain
----------------

nightly-x86_64-pc-windows-msvc (default)
rustc 1.29.0-nightly (6a1c0637c 2018-07-23)

Feedback

  1. Having rustfix merged into cargo was a great idea. As someone who does a lot of Python at home and work, and often educates people on how/why to migrate to Python 3, having an automated, officially supported tool at everyone's finger tips is going to pay dividends here. :+1:

  2. I found it a little confusing that after adding #![feature(rust_2018_preview)] to main.rs and no other changes, I was doomed to hit this error:

error: the working directory of this project is detected as dirty, and `cargo fix` can potentially perform destructive changes; if you'd like to suppress this error pass
 `--allow-dirty`, or commit the changes to these files:

  * src/main.rs

Is there any way to special case the #![feature(rust_2018_preview)] diff since literally everyone will hit it? Or when this is stable, will that flag not be needed so that this transition would be smoother for everyone?

  1. Git-ignored files shouldn't be worried about as dirty, but I already saw an issue for that somewhere.

  2. cargo fix should automatically fix extern crate's. I was getting warnings, but they all seemed like they should be fixed mechanically.

cargo-features = ["edition"]

...
[package]
edition = '2018'

Running

> cargo +nightly fix --prepare-for 2018 --all-targets --all-features --allow-dirty
    Checking rust-belt v1.2.0 (file:///C:/Users/User/PycharmProjects/rust-belt)
warning: unused extern crate
 --> src\main.rs:6:1
  |
6 | extern crate ai_behavior;
  | ^^^^^^^^^^^^^^^^^^^^^^^^^ help: remove it
  |
note: lint level defined here
 --> src\main.rs:4:9
  |
4 | #![warn(rust_2018_idioms)]
  |         ^^^^^^^^^^^^^^^^
  = note: #[warn(unused_extern_crates)] implied by #[warn(rust_2018_idioms)]

warning: `extern crate` is not idiomatic in the new edition
 --> src\main.rs:7:1
  |
7 | extern crate music;
  | ^^^^^^^^^^^^^^^^^^^ help: convert it to a `use`

warning: unused extern crate
 --> src\main.rs:8:1
  |
8 | extern crate opengl_graphics;
  | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: remove it

warning: unused extern crate
 --> src\main.rs:9:1
  |
9 | extern crate piston_window;
  | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: remove it

warning: `extern crate` is not idiomatic in the new edition
  --> src\main.rs:10:1
   |
10 | extern crate rand;
   | ^^^^^^^^^^^^^^^^^^ help: convert it to a `use`

warning: unused extern crate
  --> src\main.rs:11:1
   |
11 | extern crate sprite;
   | ^^^^^^^^^^^^^^^^^^^^ help: remove it

...

    Finished dev [unoptimized + debuginfo] target(s) in 2.65s
  1. Some of the extern crate lints were invalid. For example it told me:
warning: `extern crate` is not idiomatic in the new edition
 --> src\main.rs:7:1
  |
7 | extern crate music;
  | ^^^^^^^^^^^^^^^^^^^ help: convert it to a `use`

But when I changed it to a use, I got another warning:

warning: unused import: `music`
 --> src\main.rs:6:5
  |
6 | use music;
  |     ^^^^^
  |
  = note: #[warn(unused_imports)] on by default

Other times the lint told me to remove the extern crate correctly.

Overall, the experience was positive, but I wonder if it would be possible to provide an option to condense the 3 steps (#![feature(rust_2018_preview)], #![warn(rust_2018_idioms)], and Cargo.toml) changes into a single step.

Something like: cargo fix --migrate-to 2018 that would condense all of these steps into a single go.

In the end, I had rust-belt fully migrated to Rust 2018 in about 30 minutes using cargo fix on Windows (probably less tested, which is why I chose to do it on Windows). Diff can be viewed on GitHub.

1 Like

Thanks for testing things out @johnthagen! As you probably figured out we unfortunately don't have a newer nightly yet, but it's still good to test!

This is an excellent point, but recall that this is the preview period so during the actual transition you won't be hitting this error beacuse you won't need to add such a feature to your crate.

I do think though that the error here is a bit aggressive, but I'm not entirely sure what to do about it... On one hand we may just want to remove it due to how conservative cargo-fix is with actually changing your code!

Correct! This should be fixed once relevant PRs merge and a nightly is released.

Interesting! It looks like the steps may have been mixed up here slightly, though. When running --prepare-for 2018 you shouldn't have the 2018 edition enabled, and similarly only after the 2018 edition is enabled should the idioms lint group be enabled (if at all). We've got a lot of work to do on the idioms lint group and they're likely to be much less aggressive by the time of the edition release.

Ideally yes, although we may be unlikely to get this done. We do have rust-lang/cargo#5778, though, which is intended to ease this transition.