Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Wrong "uses" edges for reexported inner modules' types #79

Closed
asomers opened this issue Jul 11, 2021 · 2 comments
Closed

Wrong "uses" edges for reexported inner modules' types #79

asomers opened this issue Jul 11, 2021 · 2 comments

Comments

@asomers
Copy link

asomers commented Jul 11, 2021

A common pattern is for a module to define an inner private module, then reexport parts of that private module as its own public API. Two ways of doing this are pub use and pub type. But cargo-modules doesn't seem to generate the correct output for either. For example, when using this input as lib.rs:

pub mod a {
    use self::b::X;
    use self::d::Y;

    pub mod b {
        use self::c;
        pub use c::X;

        mod c {
            pub struct X{}
        }

    }

    pub mod d {
        use self::e;
        pub type Y = e::Y;

        mod e {
            pub struct Y{}
        }
    }
}

I would expect the output to include the following uses edges:

a -> b
a -> d
b -> c
d -> e

However, cargo-modules omits the d -> e edge, and changes the a -> b edge into a -> c. Here are the actual uses edges it generates:

"pub_use::a::b" -> "pub_use::a::b::c" [label="uses", color="#7f7f7f", style="Dashed"]; // "uses" edge
"pub_use::a" -> "pub_use::a::d" [label="uses", color="#7f7f7f", style="Dashed"]; // "uses" edge
"pub_use::a" -> "pub_use::a::b::c" [label="uses", color="#7f7f7f", style="Dashed"]; // "uses" edge
@regexident
Copy link
Owner

I'll have to do a more thorough investigation for this, but my guess is that this difference is due to the fact that we make use of rust-analyzer's HIR, rather than it's AST. The HIR might be providing an already resolved dependency graph?

Previously we did depend on an AST (the one from rustc), but that resulted in lots of issues related to path solution,
which eventually lead me to switch to rust-analyzer and it's HIR, which ended up closing about a dozen issues at once.

However I have the feeling that in future we might want to analyze both, the AST, as well as the HIR, since the latter doesn't seem to have any information on inter-type dependencies, either.

regexident added a commit that referenced this issue Sep 11, 2021
regexident added a commit that referenced this issue Sep 12, 2021
regexident added a commit that referenced this issue Sep 12, 2021
@regexident
Copy link
Owner

I'm closing this

$ cargo modules generate graph --with-uses generates the following graph:

                          ┌───────┐           
                          │ crate │           
                          └───────┘           
                              │               
                              │               
                              ▼               
                        ┌──────────┐          
                       ─│ crate::a │─ ─ ─ ─ ─ 
                      │ └──────────┘         │
                              │               
            ┌─────────┼───────┴────┐         │
            ▼                      ▼          
     ┌─────────────┐  │     ┌─────────────┐  │
┌ ─ ─│ crate::a::b │        │ crate::a::d │◀─ 
     └─────────────┘  │     └─────────────┘   
│           │                      │          
            │         │            │          
│           ▼                      ▼          
   ┌────────────────┐ │   ┌────────────────┐  
└ ▶│ crate::a::b::c │◀    │ crate::a::d::e │  
   └────────────────┘     └────────────────┘  

Where solid lines denote "owns" edges and dashed lines denote "uses" edges.

Corresponding dot code

$ cargo modules generate graph --with-uses:

digraph {

    graph [
        label="github_issue_79",
        labelloc=t,
        pad=0.4,
        layout=neato,
        overlap=false,
        splines="line",
        rankdir=LR,
        fontname="Helvetica", 
        fontsize="36",
    ];

    node [
        fontname="monospace",
        fontsize="10",
        shape="record",
        style="filled",
    ];

    edge [
        fontname="monospace",
        fontsize="10",
    ];

    "github_issue_79" [label="crate|github_issue_79", fillcolor="#5397c8"]; // "crate" node
    "github_issue_79::a" [label="pub mod|a", fillcolor="#81c169"]; // "mod" node
    "github_issue_79::a::b" [label="pub mod|a::b", fillcolor="#81c169"]; // "mod" node
    "github_issue_79::a::b::c" [label="pub(self) mod|a::b::c", fillcolor="#db5367"]; // "mod" node
    "github_issue_79::a::d" [label="pub mod|a::d", fillcolor="#81c169"]; // "mod" node
    "github_issue_79::a::d::e" [label="pub(self) mod|a::d::e", fillcolor="#db5367"]; // "mod" node

    "github_issue_79" -> "github_issue_79::a" [label="owns", color="#000000", style="solid"] [constraint=true]; // "owns" edge
    "github_issue_79::a" -> "github_issue_79::a::b" [label="owns", color="#000000", style="solid"] [constraint=true]; // "owns" edge
    "github_issue_79::a::b" -> "github_issue_79::a::b::c" [label="owns", color="#000000", style="solid"] [constraint=true]; // "owns" edge
    "github_issue_79::a::b" -> "github_issue_79::a::b::c" [label="uses", color="#7f7f7f", style="dashed"] [constraint=false]; // "uses" edge
    "github_issue_79::a" -> "github_issue_79::a::d" [label="owns", color="#000000", style="solid"] [constraint=true]; // "owns" edge
    "github_issue_79::a::d" -> "github_issue_79::a::d::e" [label="owns", color="#000000", style="solid"] [constraint=true]; // "owns" edge
    "github_issue_79::a" -> "github_issue_79::a::b::c" [label="uses", color="#7f7f7f", style="dashed"] [constraint=false]; // "uses" edge
    "github_issue_79::a" -> "github_issue_79::a::d" [label="uses", color="#7f7f7f", style="dashed"] [constraint=false]; // "uses" edge

}

Since rust-analyzer resolves re-exports the edges in the generated graph do not directly correlate to use statements in the code.

As such:

  • the use self::b::X; in mod a ends up producing an edge a -> c, since a::b::X is a re-export of a::b::c::X.
  • the use self::d::Y; in mod a ends up producing an edge a -> d, since a::d::Y is a type-alias for a::b::c::X and not merely a re-export of it.
  • the use self::c; and use self::e; in mod b and mod d on the other hand have no effect whatsoever, since they are superfluous.
  • the pub use c::X; in mod b on the other hand does import X from mod c and is thus reflected in the graph as such.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants