add function to find all imports

[euclio]

First pass at an API. It gets the job done, but I'd appreciate some feedback :)

Fixes #123.

[nrc]

With a couple of minor tweaks I think this PR is fine to add the all_imports API. However, I think it is probably not the best API.

I haven't been keeping up with the rustdoc development, so I'm not sure how things are panning out, but here are some thoughts (based on how we do similar things in the RLS):

• are imports a first class piece of data? Or are they an implementation detail?
• what are the first class data? And how do imports relate to them?
• the implementation details should be encapsulated within rls-analysis, and we should expose an API which has the detail in the right place
• in particular, it seems that rustdoc is going to have to do quite a lot of computation on imports to get the information you really want - I expect it would be better for that to be inside rls-analysis.

[nrc]

I would just call this def, not def_id

[nrc]

I would call this all_imports - it's not really finding anything, just dumping everything at once.

[euclio]

Sorry I've been letting this sit around! Let's get this going again.

(cc @steveklabnik, who might like to add to this discussion)

I agree that the API is overly simple. My intent was to mirror the analysis data as much as possible to avoid adding significant overhead to the rls side of things. Perhaps this is a premature optimization and it makes more sense to pull this logic into rls-analysis, but I don't think it's a huge burden to offload that computation to rustdoc2, either.

As it stands today, rustdoc2 transforms the analysis data into a JSON API response which is intended to be consumed by a frontend (e.g., JSON -> HTML). You can find an example of this response here. Each item in this JSON is identified by its qualified name. We generate the JSON by walking the reachable module tree from the crate root via rls-analysis. Notably, this means that any items that are only reachable through re-exports are not in the JSON response (the problem which this PR is meant to address).

rustdoc2 needs to display all exported items in the documentation and create links to the item definition. To this end, imports should likely be first class, but it's an open question on how to identify them in the current scheme. Simple exports and re-exports are easy: rustdoc2 can just generate another item with module_path::ExportedItem as the ID. Current rustdoc displays re-exports and normal exports in different sections, so one might think that we would need to distinguish the two in the analysis output, but it's possible to infer this by looking at the rest of the analysis. If the import is reachable through other means, it's a re-export, otherwise, you can just display the item as if it were defined at that path.

However, glob imports and import lists prove more difficult. I haven't settled on a way to identify them uniquely in the JSON.

[nrc]

So, I would make public/reachable explicit on imports. I assume we can do this in a similar way as we do for other items, but I haven't checked.

For list imports and globs, I think you want an import for each name imported, and as far as the rls-analysis API we should not distinguish between the three kinds of import. Note that this means expanding the glob import - this is pretty easy since the names which are imported are in the value field.

I guess we would like to filter imports in the save-analysis data by reachability in the same way as other items, rather than at the rls-analysis level?

I expect that at the rls-analysis API, it might be worth treating imports as just another kind of item. I assume you could layer the 'chaining' of imports on top of that?

[euclio]

Is it possible to determine which defs a glob imports with the raw analysis data as it stands today?

[nrc]

Is it possible to determine which defs a glob imports with the raw analysis data as it stands today?

I think so, but not directly - you can determine the fully qualified names by appending the names from the value list to the prefix of the glob, then use that name to look up the def.

[nrc]

@euclio do you still want to go ahead with this PR? Do you need help with anything?

[euclio]

Yes, haven't had much time for OSS development lately. Hoping to get back to this soon; it's a big blocker for rustdoc. I'll let you know if I run into any problems.

…

On Thu, Mar 15, 2018 at 2:20 PM Nick Cameron ***@***.***> wrote: @euclio <https://github.com/euclio> do you still want to go ahead with this PR? Do you need help with anything? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#124 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABTxFpBf8CIN1CGxJWa5snXvw7sg7MJdks5terD4gaJpZM4RGGCL> .

[euclio]

Ok, finally had some time to get back to this!

With the following code:

pub use private::*;

mod private {
    pub struct Struct1;
    pub struct Struct2;
}

I'm getting an import with an empty string for value, so it seems like I can't use that to determine the items the glob imports. Is this a bug in how the analysis data is generated?

The Debug output of the import looks like this:

Import { kind: GlobUse, ref_id: None, span: SpanData { file_name: "src/lib.rs", byte_start: 382, byte_end: 383, line_start: Row(27, PhantomData), line_end: Row(27, PhantomData), column_start: Column(18, PhantomData), column_end: Column(19, PhantomData) }, name: "*", value: "", parent: Some(Id { krate: 0, index: 0 }) }

[nrc]

@euclio that looks correct, since the code is not using anything from private, nothing is being imported so the value for the glob is nothing. If you use a name from private, then it should be included in the value.

[euclio]

Is the pub use not enough to consider the structs "used" in a library crate? Struct1 and Struct2 are certainly being exported, as rustdoc1 will render them.

[nrc]

Is the pub use not enough to consider the structs "used" in a library crate? Struct1 and Struct2 are certainly being exported, as rustdoc1 will render them.

Hmm, interesting. That might be a bug in the implementation from the compiler. I suspect it doesn't take the pub into account and just lists the names used inside the module. This could (and should!) be fixed in the compiler. I think it would be quite an easy change to the name resolution code, but I might be wrong.

[euclio]

I'm going to close this, as the upstream project (rustdoc2) is dormant.