search

rust-lang / rust-analyzer

A Rust compiler front-end for IDEs
https://rust-analyzer.github.io/
Apache License 2.0
14.29k stars 1.61k forks source link

hir::*::module doesn't make sense #7952

Open matklad opened 3 years ago

matklad commented 3 years ago

Many hir types have ::module method, but this is wrong, as this can't have reasonable semantics: items can be nested. So we should remove all those module calls and replace them with something which makes sense. Super unclear what that should be.

jonas-schievink commented 3 years ago

I guess ContainerId could fill this role, which I just deleted :laughing:

One question to inform the API here would be: What are the current callers of module() actually using that information for?

flodiebold commented 3 years ago

This is kind of related to #7951, isn't it? IMO module does make sense, if it can return either a real module or a block module. Users of the HIR API can then decide whether they're interested in the containing real module, item, or block.

matklad commented 3 years ago

Jup, they are related.

IMO module does make sense, if it can return either a real module or a block module.

I think that's worse than that. hir::TypeParam also has a ::module, but the semantics is different from other items. Type parameters are not declared in a module, and they are not visible in a module.

It feels like what we need is a .declaration_scope() -> DeclarationScope, which returns some object which can be used in search, name qualification and string path resoulution (for doc links). Internally, it can be a module, block, or item scope (or maybe some other things like buildin scope or what not. Find usage working on usize would be fun, isn't it?).

We also need canonical_name() -> Option<String> for things like hover.

I do think that the overall hir API would be simpler for consumers if module is just a module, and weird stuff related to actual places where an item can be declared has a different name.