Edition 2018 Support

[muhuk]

Edition 2018 changes the way use works in a few different ways. I would like to add support resolving dependencies in projects that use edition = "2018".

My plan is as below:

• Add a command-line option --enable-edition-2018. Initially this will be ignored. Current code does not take edition into consideration anyway. New option will be documented in --help.
• Implement an alternate version of the dependency graph in a separate module. Keep the existing code intact. The new algorithm will only support edition 2018. There will be tests.
• When --enable-edition-2018 is given and edition is "2018" in Cargo.toml, use the alternate implementation. Otherwise keep everything same.

This issue should also close #29 when implemented fully.

[regexident]

Hi @muhuk,

sounds good to me! (Especially the "Implement an alternate version of the dependency graph in a separate module." bit.)

Optimally we should generate a kind of IR of the dependency graph. The best fit for such an IR would probably be an actual graph representation using e.g. petgraph.

// lib.rs:

mod foo {
    use std::mem;

    mod bar {
        struct Baz;
    }

    use bar::Baz;
}

use foo::bar::Baz;

// petgraph IR:

crate --[mod]-> crate::foo;
crate --[use]-> crate::foo::bar::Baz;

crate::foo --[use]-> std::mem;
crate::foo --[mod]-> crate::foo::bar;
crate::foo --[use]-> crate::foo::bar::Baz;

crate::foo::bar --[struct]-> crate::foo::bar::Baz;

As such we would only need to have two implementations of the "frontend" generating the dependency-graph IR from the parsed Rust source, one for 2015, one for 2018 edition.

We could then share everything else between editions and even port the tree mode to generate its output from the IR.

And last but not least a petgraph IR would allow us to implement this: #18.

[muhuk]

I have started working on detection of edition (not in the plan above 😕 ). WIP branch is here

[muhuk]

I have a couple of questions. I have notices these two things while working on this feature:

1. Do we really need to prepend "./" in front of build scripts? read-manifest produces absolute paths. Could this be something left over from earlier versions of cargo?

   fn get_build_scripts(target_cfgs: &[json::JsonValue]) -> Vec<path::PathBuf> {
       target_cfgs
           .iter()
           .filter_map(|cfg| {
               if cfg["kind"].contains("custom-build") {
                   cfg["src_path"]
                       .as_str()
                       .map(|s| path::Path::new("./").join(s))
               } else {
                   None
               }
           })
           .collect()
   }

2. Do we event need to ignore these files? I've disabled detection and they still don't show up in tree or graph. (with --orphan on of course)

(I am using Debian, cargo 1.34.0-nightly)

[regexident]

If I remember correctly cargo-modules would wrongly list those files as modules even though they are not part of the crate.

[muhuk]

Currently we have the following files:

Path	Description	What to do with it?
builder.rs	Builder for tree::Tree.	Modify.
dot_printer.rs	Visits a Tree to generate graphviz output (graph subcommand)	Replace.
error.rs	Contains Error enum. Used by main & manifest.	Keep.
main.rs	Entry point.	Keep.
manifest.rs	Project manifest parser and data type.	Keep.
printer.rs	Visits a Tree to generate output for a tree subcommand.	Replace.
tree.rs	Data type that represents both the dependency-tree and the (raw) imports. Relevant both in tree and graph modes.	Replace with the graph implementation.

Looks like we will be:

• Replacing tree::Tree with a petgraph graph
• Adding richer use information. Currently uses are lists of strings. Based on edition and the rest of the project graph we need to resolve members.
• Replacing the two printer modules to use this graph.

Note I: Replace above means eventually replace when the implementation is complete.
Note II: I just wanted to do this quick analysis before starting with the code.

[muhuk]

Hi @regexident ,

I have pushed my WIP branch to 34-new-dependency-graph. It is not yet in a shape to be merged, but can be reviewed. I'll crate a pull request once it's more complete.

[muhuk]

Just checking in. I was in a business trip and now I'm back. Part III starts.

[muhuk]

Some more progress here, still needs a couple more sessions to put it all together.

[muhuk]

The rules for use path prefixes, as far as I can understand is as below:

| Prefix Style                                          | E2015 | E2018 |
|-------------------------------------------------------+-------+-------|
| Refer modules in the crate using relative path        | YES   | YES   |
| Refer to external crate using absolute path           | YES   | YES   |
| `crate` keywords to refer to the current crate        | NO    | YES   |
| `::` prefix to refer to current crate of extern crate | YES   | NO    |
| `::` prefix to refer to current crate                 | NO    | YES   |
| `super` prefix to refer to parent                     | YES   | YES   |
| `self` prefix to refer current module                 | YES   | YES   |

Two things are not handled (AFAICU):

• Creating aliases via as
• Resolving dependencies when something is referred with its full path without a use.

[muhuk]

Work in progress. Turns out petgraph does not support multiple edges (same direction) between two nodes. I will have to combine parent-child edge and use edge.

[regexident]

Looking great! Putting a bitflags label on the edge should work, I suppose? Wrapping the petgraph in a wrapper type that does the projection from/to multigraph might help, too?

[muhuk]

Hi @regexident ,
Thanks for the suggestions. Let me think about them. I was thinking of manually merging the variants of Dependency since it was one enum.

As for rendering (turning a single petgraph edge into multiple dot edges) that should be a non-issue.

[muhuk]

I ended up with this Edge implementation.

[regexident]

@muhuk: @skade just pointed me towards the ra_hir_def crate (thanks, @skade!) from @matklad's awesome rust-analyzer, which implements path resolving. Might be worth a look, I guess?

[muhuk]

Right now I'm leaning more towards cleaning things up than making the scope bigger. Let me see though. 😉

[muhuk]

I have tried to upgrade all deps and the toolchain to more recent versions but couldn't make it work in the end. Looks like parse_crate_from_file is moved to rustc_parse, but it seems this one is not published.

I was hoping to do this and somehow unify the two sets of analyzers and printers (getting rid of the ridiculous ng namespace in the process).

Maybe rustc_parse can be added as a dependency somehow and I missed it. But otherwise it seems the only way forward is to build something new on top of rust-analyzer. Again, I might be missing something. In that case please let me know.

Cleanup aside, I think we can close this ticket as we have (some sort of) Edition 2018 support now.

[regexident]

This should now be fixed with #66 having been merged. 🎉 😃
_{(The release still needs a it more work. If you're feeling lucky you can install from source and give it a try, though.)}