Edition 2018 Support about cargo-modules HOT 17 CLOSED

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.

from cargo-modules.

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

from cargo-modules.

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

from cargo-modules.

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

from cargo-modules.

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)

from cargo-modules.

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

from cargo-modules.

Currently we have the following files:

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.

from cargo-modules.

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.

from cargo-modules.

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.

from cargo-modules.

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.

from cargo-modules.

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?

from cargo-modules.

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.

from cargo-modules.

I ended up with this Edge implementation.

from cargo-modules.

@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?

from cargo-modules.

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

from cargo-modules.

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.

from cargo-modules.

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.)}

from cargo-modules.