The Great Module Adventure Continues

I think putting all crates in Cargo.toml into the prelude is a horrible idea. The “global namespace” is a really precious resource and we should be very careful with what we add there.

Would the following code produce a shadowing warning?

use image::load;

let image = load(...);

If we put the external crates also in the crate root then the leading-:: variant doesn't require this transition, right? I think breaking less code is more important than solving edge case grammatical inconsistencies. And also more important than aesthetic preference.

@pizzaiter There is no shadowing there at all, modules and variables have separate namespaces, see how this works fine.

Another question: The image crate depends on the png crate and also has a (top level) png module. The png module contains an extern crate png; declaration.

As far as I understand, in the new system one would delete the extern crate declaration and this would still work, but produce a shadowing warning. Presumably, one would then either silence the lint or start renaming stuff.

So my question is, will this style — wrapping a crate inside a stub module — be considered unidiomatic?

I know that I can still use this style in the new system, but it will get slightly awkward. First because of the naming and second because the external crate will be more easily available in other parts of my crate than my own module.

@leodasvacas I see, thanks for clearing that up.

2 Likes

Probably not, though to achieve this effect, one would need to rename the png crate in Cargo.toml:

[package]
name = "image"

[dependencies]
png = { version = "1.0", package = "image_png" }
pub mod png {
    pub struct SomwWrapper(image_png::SomeStruct);
}
2 Likes

@matklad And then how do I notice that image_png is the png crate from crates.io?

I still don’t understand why we are deprecating extern crate when that means there will be no way for RLS to point out to what image_png is. How do I notice that image_png is just the png crate without going through hoops having to open the crate’s Cargo.toml manifest?

I know from experience that I’ve very often did such things in Servo, where we rename the cookie crate to cookie_rs because we have a cookie module. Telling VSCode to “jump to definition” with the cursor on cookie_rs will bring me to extern crate cookie as cookie_rs. How do I do this now?

It will jump to Cargo.toml.

RLS uses the Cargo.toml via cargo rather than trying to parse and understand the syntax file. This is also how it builds and analyses more complex projects.

The removal of extern crate should not have an impact on the abilities of RLS. Jump to definition will continue to work how it does today.

The problem and the source of the concern is that it _doesn't_ work today.
I'm using VSCode and neither @KalitaAlexey's plugin nor RLS plugin can't jump into locations in Cargo.toml.
At least the first one can jump into the lib.rs of the extern crate, RLS doesn't do even that.

That could be. I’m just saying that this is a separate issue from the removal of extern crate or not. The RLS will use what it knows from the Cargo.toml file via cargo. The behaviour we allow for goto definition is separate and more of a dev tools team discussion on RLS functionality.

Hmm... does this transitively include all dependencies of dependencies? That might get a bit overwhelming for large projects.

No, just the direct dependencies, i.e. things you're able to use extern crate with today.

2 Likes

I love this proposal. I agree with the “it just feels right” comment. Well done!

… however.

I just thought of one tiny snag with eliminating mod.rs. It’s not insurmountable, but something to think about.

When writing integration tests, if you want to create some sort of utilities module, you can’t make test/utils.rs, because then it tries to compile utils.rs as a test as well, and that often fails. So you use utils/mod.rs, and it works. For more, see here: https://doc.rust-lang.org/book/second-edition/ch11-03-test-organization.html#submodules-in-integration-tests

What should we do here?

The answer is clear. Only support foo/mod.rs-style modules, not foo.rs.

</ducks>

So, that is annoying. Still, the existing situation is not great -- I feel like modules from integration tests should not be all "muddled together" in a common directory.

Personally, I think it'd be nice if integration tests were, well, more like today's modules, either tests/some_test.rs or tests/some_test/..., right? But that kind of puts us back where we started (what do we call the ...). Also, the existing behavior of ignoring directories must be accommodated, since clearly we are relying on it.

TL;DR I don't know =) but good you raised it...

1 Like

When writing integration tests, if you want to create some sort of utilities module

Do you want to use this utils module for a single integration test, or for several integration tests? If the former, you can now write multi-file integration tests:

test/
  foo/
    main.rs 
    utils.rs

If you want to reuse this module for several tests, you'd better to extract it into a crate and use it as a path dev-dependency. Otherwise, this module will be compiled multiple times, once per integration test, and it doesn't feel exactly right.

3 Likes

That's what I was looking for....

Whoah when did this happen?

In 1.22 https://github.com/rust-lang/cargo/pull/4496/files & https://github.com/rust-lang/rust/blob/master/RELEASES.md#cargo-2.

The question is, why the docs over https://doc.rust-lang.org/cargo/reference/manifest.html do not mention it, although the source code for the docs (https://github.com/rust-lang/cargo/blob/master/src/doc/src/reference/manifest.md#the-project-layout) does (search for (optional) directories containing multi-file tests)

EDIT: nightly book is OK though EDIT2: looks like the missing docs are found, recovered, and will be installed in the legitimate place with the next relase: https://github.com/rust-lang/cargo/issues/4906

Can we have a summarised post here with all the options, example for each one and reasons for/against? Makes it easier to follow and draw up a conclusion.

@petrochenkov let’s talk implementation for a second. Do you think there are remaining details from what @aturon described that need to be cleared up before we can implement? Do you think you’ll have time to hack on this stuff? (If not, would you be up for mentoring and/or just e-mailing me a few brief notes and I can take a stab at mentoring?)

I think that @aturon’s post here is still basically “the latest”.

2 Likes