From 1d6f291335c58aac95c1124f55d7fb0834baff2a Mon Sep 17 00:00:00 2001 From: Zac Pullar-Strecker Date: Sat, 13 Jun 2020 22:34:59 +1200 Subject: Move resolver into impls, work on tests --- Cargo.lock | 1 + crates/ra_hir/src/code_model.rs | 37 ++++++--- crates/ra_ide/src/hover.rs | 148 ++++++++++++++++++++++------------- crates/ra_ide/src/mock_analysis.rs | 2 +- crates/ra_ide_db/Cargo.toml | 1 + crates/ra_ide_db/src/defs.rs | 17 +++- crates/rust-analyzer/src/handlers.rs | 3 +- 7 files changed, 138 insertions(+), 71 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index be97c16ff..0c1d2e8b9 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1303,6 +1303,7 @@ dependencies = [ "once_cell", "ra_db", "ra_hir", + "ra_hir_def", "ra_prof", "ra_syntax", "ra_text_edit", diff --git a/crates/ra_hir/src/code_model.rs b/crates/ra_hir/src/code_model.rs index e86077dd6..5766cc3b8 100644 --- a/crates/ra_hir/src/code_model.rs +++ b/crates/ra_hir/src/code_model.rs @@ -186,21 +186,34 @@ impl ModuleDef { module.visibility_of(db, self) } - pub fn name(self, db: &dyn HirDatabase) -> Option { + pub fn name(&self, db: &dyn HirDatabase) -> Option { match self { - ModuleDef::Adt(it) => Some(it.name(db)), - ModuleDef::Trait(it) => Some(it.name(db)), - ModuleDef::Function(it) => Some(it.name(db)), - ModuleDef::EnumVariant(it) => Some(it.name(db)), - ModuleDef::TypeAlias(it) => Some(it.name(db)), - - ModuleDef::Module(it) => it.name(db), - ModuleDef::Const(it) => it.name(db), - ModuleDef::Static(it) => it.name(db), - - ModuleDef::BuiltinType(it) => Some(it.as_name()), + ModuleDef::Module(m) => m.name(db), + ModuleDef::Function(m) => Some(m.name(db)), + ModuleDef::Adt(m) => Some(m.name(db)), + ModuleDef::EnumVariant(m) => Some(m.name(db)), + ModuleDef::Const(m) => {m.name(db)}, + ModuleDef::Static(m) => {m.name(db)}, + ModuleDef::Trait(m) => {Some(m.name(db))}, + ModuleDef::TypeAlias(m) => {Some(m.name(db))}, + ModuleDef::BuiltinType(m) => {Some(m.as_name())} } } + + pub fn resolver(&self, db: &D) -> Option { + Some(match self { + ModuleDef::Module(m) => Into::::into(m.clone()).resolver(db), + ModuleDef::Function(f) => Into::::into(f.clone()).resolver(db), + ModuleDef::Adt(adt) => Into::::into(adt.clone()).resolver(db), + ModuleDef::EnumVariant(ev) => Into::::into(Into::::into(ev.clone())).resolver(db), + ModuleDef::Const(c) => Into::::into(Into::::into(c.clone())).resolver(db), + ModuleDef::Static(s) => Into::::into(s.clone()).resolver(db), + ModuleDef::Trait(t) => Into::::into(t.clone()).resolver(db), + ModuleDef::TypeAlias(t) => Into::::into(t.module(db)).resolver(db), + // TODO: This should be a resolver relative to `std` + ModuleDef::BuiltinType(_t) => None? + }) + } } pub use hir_def::{ diff --git a/crates/ra_ide/src/hover.rs b/crates/ra_ide/src/hover.rs index 0da10a08e..34fc36a1f 100644 --- a/crates/ra_ide/src/hover.rs +++ b/crates/ra_ide/src/hover.rs @@ -146,7 +146,8 @@ pub(crate) fn hover(db: &RootDatabase, position: FilePosition) -> Option Err(_) => { let link_str = String::from_utf8(link.url.clone()).unwrap(); let link_text = String::from_utf8(link.title.clone()).unwrap(); - let resolved = try_resolve_path(db, definition, &link_str) - .or_else(|| try_resolve_intra(db, definition, &link_text, &link_str)); + let resolved = try_resolve_intra(db, definition, &link_text, &link_str) + .or_else(|| try_resolve_path(db, definition, &link_str)); if let Some(resolved) = resolved { link.url = resolved.as_bytes().to_vec(); @@ -420,7 +421,7 @@ fn rewrite_links(db: &RootDatabase, markdown: &str, definition: &Definition) -> }); let mut out = Vec::new(); format_commonmark(doc, &ComrakOptions::default(), &mut out).ok()?; - Some(String::from_utf8(out).unwrap()) + Some(String::from_utf8(out).unwrap().trim().to_string()) } #[derive(PartialEq, Eq, Hash, Copy, Clone, Debug)] @@ -470,32 +471,7 @@ fn try_resolve_intra(db: &RootDatabase, definition: &Definition, link_text: &str let modpath = ModPath::from_src(path, &Hygiene::new_unhygienic()).unwrap(); // Resolve it relative to symbol's location (according to the RFC this should consider small scopes - let resolver = { - use ra_hir_def::*; - use hir::*; - - // TODO: This should be replaced by implementing HasResolver/TryHasResolver on ModuleDef and Definition. - match definition { - Definition::ModuleDef(def) => match def { - ModuleDef::Module(m) => Into::::into(m.clone()).resolver(db), - ModuleDef::Function(f) => Into::::into(f.clone()).resolver(db), - ModuleDef::Adt(adt) => Into::::into(adt.clone()).resolver(db), - ModuleDef::EnumVariant(ev) => Into::::into(Into::::into(ev.clone())).resolver(db), - ModuleDef::Const(c) => Into::::into(Into::::into(c.clone())).resolver(db), - ModuleDef::Static(s) => Into::::into(s.clone()).resolver(db), - ModuleDef::Trait(t) => Into::::into(t.clone()).resolver(db), - ModuleDef::TypeAlias(t) => Into::::into(t.module(db)).resolver(db), - // TODO: This should be a resolver relative to `std` - ModuleDef::BuiltinType(_t) => Into::::into(definition.module(db)?).resolver(db) - }, - Definition::Field(field) => Into::::into(Into::::into(field.parent_def(db))).resolver(db), - Definition::Macro(m) => Into::::into(m.module(db)?).resolver(db), - Definition::SelfType(imp) => Into::::into(imp.clone()).resolver(db), - // it's possible, read probable, that other arms of this are also unreachable - Definition::Local(_local) => unreachable!(), - Definition::TypeParam(tp) => Into::::into(tp.module(db)).resolver(db) - } - }; + let resolver = definition.resolver(db)?; // Namespace disambiguation let namespace = Namespace::from_intra_spec(link_target); @@ -527,7 +503,7 @@ fn try_resolve_intra(db: &RootDatabase, definition: &Definition, link_text: &str get_doc_url(db, &krate)? .join(&format!("{}/", krate.display_name(db)?)).ok()? .join(&path.segments.iter().map(|name| format!("{}", name)).join("/")).ok()? - .join(&get_symbol_filename(db, definition)?).ok()? + .join(&get_symbol_filename(db, &Definition::ModuleDef(def))?).ok()? .into_string() ) } @@ -637,11 +613,20 @@ mod tests { use crate::mock_analysis::analysis_and_position; - fn trim_markup(s: &str) -> &str { - s.trim_start_matches("```rust\n").trim_end_matches("\n```") + fn trim_markup(s: &str) -> String { + s + .replace("``` rust", "```rust") + .replace("-----", "___") + .replace("\n\n___\n\n", "\n___\n\n") + .replace("\\<-", "<-") + .trim_start_matches("test\n```\n\n") + .trim_start_matches("```rust\n") + .trim_start_matches("test\n```\n\n```rust\n") + .trim_end_matches("\n```") + .to_string() } - fn trim_markup_opt(s: Option<&str>) -> Option<&str> { + fn trim_markup_opt(s: Option<&str>) -> Option { s.map(trim_markup) } @@ -689,7 +674,7 @@ fn main() { ); let hover = analysis.hover(position).unwrap().unwrap(); assert_eq!(hover.range, TextRange::new(58.into(), 63.into())); - assert_eq!(trim_markup_opt(hover.info.first()), Some("u32")); + assert_eq!(trim_markup_opt(hover.info.first()).as_deref(), Some("u32")); } #[test] @@ -818,7 +803,7 @@ fn main() { }; } "#, - &["Foo\n```\n\n```rust\nfield_a: u32"], + &["test::Foo\n```\n\n```rust\nfield_a: u32"], ); // Hovering over the field in the definition @@ -835,7 +820,7 @@ fn main() { }; } "#, - &["Foo\n```\n\n```rust\nfield_a: u32"], + &["test::Foo\n```\n\n```rust\nfield_a: u32"], ); } @@ -888,7 +873,7 @@ fn main() { ", ); let hover = analysis.hover(position).unwrap().unwrap(); - assert_eq!(trim_markup_opt(hover.info.first()), Some("Option\n```\n\n```rust\nSome")); + assert_eq!(trim_markup_opt(hover.info.first()).as_deref(), Some("test::Option\n```\n\n```rust\nSome")); let (analysis, position) = analysis_and_position( " @@ -901,7 +886,7 @@ fn main() { ", ); let hover = analysis.hover(position).unwrap().unwrap(); - assert_eq!(trim_markup_opt(hover.info.first()), Some("Option")); + assert_eq!(trim_markup_opt(hover.info.first()).as_deref(), Some("Option")); } #[test] @@ -915,7 +900,7 @@ fn main() { } "#, &[" -Option +test::Option ``` ```rust @@ -940,7 +925,7 @@ The None variant } "#, &[" -Option +test::Option ``` ```rust @@ -958,14 +943,14 @@ The Some variant fn hover_for_local_variable() { let (analysis, position) = analysis_and_position("fn func(foo: i32) { fo<|>o; }"); let hover = analysis.hover(position).unwrap().unwrap(); - assert_eq!(trim_markup_opt(hover.info.first()), Some("i32")); + assert_eq!(trim_markup_opt(hover.info.first()).as_deref(), Some("i32")); } #[test] fn hover_for_local_variable_pat() { let (analysis, position) = analysis_and_position("fn func(fo<|>o: i32) {}"); let hover = analysis.hover(position).unwrap().unwrap(); - assert_eq!(trim_markup_opt(hover.info.first()), Some("i32")); + assert_eq!(trim_markup_opt(hover.info.first()).as_deref(), Some("i32")); } #[test] @@ -976,14 +961,14 @@ fn func(foo: i32) { if true { <|>foo; }; } ", ); let hover = analysis.hover(position).unwrap().unwrap(); - assert_eq!(trim_markup_opt(hover.info.first()), Some("i32")); + assert_eq!(trim_markup_opt(hover.info.first()).as_deref(), Some("i32")); } #[test] fn hover_for_param_edge() { let (analysis, position) = analysis_and_position("fn func(<|>foo: i32) {}"); let hover = analysis.hover(position).unwrap().unwrap(); - assert_eq!(trim_markup_opt(hover.info.first()), Some("i32")); + assert_eq!(trim_markup_opt(hover.info.first()).as_deref(), Some("i32")); } #[test] @@ -1004,7 +989,7 @@ fn func(foo: i32) { if true { <|>foo; }; } ", ); let hover = analysis.hover(position).unwrap().unwrap(); - assert_eq!(trim_markup_opt(hover.info.first()), Some("Thing")); + assert_eq!(trim_markup_opt(hover.info.first()).as_deref(), Some("Thing")); } #[test] @@ -1028,8 +1013,8 @@ fn func(foo: i32) { if true { <|>foo; }; } ); let hover = analysis.hover(position).unwrap().unwrap(); assert_eq!( - trim_markup_opt(hover.info.first()), - Some("wrapper::Thing\n```\n\n```rust\nfn new() -> Thing") + trim_markup_opt(hover.info.first()).as_deref(), + Some("test::wrapper::Thing\n```\n\n```rust\nfn new() -> Thing") ); } @@ -1052,7 +1037,7 @@ fn func(foo: i32) { if true { <|>foo; }; } ", ); let hover = analysis.hover(position).unwrap().unwrap(); - assert_eq!(trim_markup_opt(hover.info.first()), Some("const C: u32")); + assert_eq!(trim_markup_opt(hover.info.first()).as_deref(), Some("const C: u32")); } #[test] @@ -1068,7 +1053,7 @@ fn func(foo: i32) { if true { <|>foo; }; } ", ); let hover = analysis.hover(position).unwrap().unwrap(); - assert_eq!(trim_markup_opt(hover.info.first()), Some("Thing")); + assert_eq!(trim_markup_opt(hover.info.first()).as_deref(), Some("Thing")); /* FIXME: revive these tests let (analysis, position) = analysis_and_position( @@ -1125,7 +1110,7 @@ fn func(foo: i32) { if true { <|>foo; }; } ", ); let hover = analysis.hover(position).unwrap().unwrap(); - assert_eq!(trim_markup_opt(hover.info.first()), Some("i32")); + assert_eq!(trim_markup_opt(hover.info.first()).as_deref(), Some("i32")); } #[test] @@ -1142,7 +1127,7 @@ fn func(foo: i32) { if true { <|>foo; }; } ", ); let hover = analysis.hover(position).unwrap().unwrap(); - assert_eq!(trim_markup_opt(hover.info.first()), Some("macro_rules! foo")); + assert_eq!(trim_markup_opt(hover.info.first()).as_deref(), Some("macro_rules! foo")); } #[test] @@ -1153,7 +1138,7 @@ fn func(foo: i32) { if true { <|>foo; }; } ", ); let hover = analysis.hover(position).unwrap().unwrap(); - assert_eq!(trim_markup_opt(hover.info.first()), Some("i32")); + assert_eq!(trim_markup_opt(hover.info.first()).as_deref(), Some("i32")); } #[test] @@ -1414,6 +1399,59 @@ fn func(foo: i32) { if true { <|>foo; }; } ); } + #[test] + fn test_hover_path_link() { + check_hover_result( + r" + //- /lib.rs + pub struct Foo; + /// [Foo](struct.Foo.html) + pub struct B<|>ar + ", + &["pub struct Bar\n```\n___\n\n[Foo](https://docs.rs/test/*/test/struct.Foo.html)"] + ); + } + + #[test] + fn test_hover_intra_link() { + check_hover_result( + r" + //- /lib.rs + pub struct Foo; + /// [Foo](Foo) + pub struct B<|>ar + ", + &["pub struct Bar\n```\n___\n\n[Foo](https://docs.rs/test/*/test/struct.Foo.html)"] + ); + } + + #[test] + fn test_hover_intra_link_shortlink() { + check_hover_result( + r" + //- /lib.rs + pub struct Foo; + /// [Foo] + pub struct B<|>ar + ", + &["pub struct Bar\n```\n___\n\n[Foo](https://docs.rs/test/*/test/struct.Foo.html)"] + ); + } + + #[test] + fn test_hover_intra_link_namespaced() { + check_hover_result( + r" + //- /lib.rs + pub struct Foo; + fn Foo() {} + /// [Foo()] + pub struct B<|>ar + ", + &["pub struct Bar\n```\n___\n\n[Foo](https://docs.rs/test/*/test/struct.Foo.html)"] + ); + } + #[test] fn test_hover_macro_generated_struct_fn_doc_comment() { mark::check!(hover_macro_generated_struct_fn_doc_comment); @@ -1438,7 +1476,7 @@ fn func(foo: i32) { if true { <|>foo; }; } bar.fo<|>o(); } "#, - &["Bar\n```\n\n```rust\nfn foo(&self)\n```\n___\n\n Do the foo"], + &["test::Bar\n```\n\n```rust\nfn foo(&self)\n```\n___\n\nDo the foo"], ); } @@ -1466,7 +1504,7 @@ fn func(foo: i32) { if true { <|>foo; }; } bar.fo<|>o(); } "#, - &["Bar\n```\n\n```rust\nfn foo(&self)\n```\n___\n\nDo the foo"], + &["test::Bar\n```\n\n```rust\nfn foo(&self)\n```\n___\n\nDo the foo"], ); } diff --git a/crates/ra_ide/src/mock_analysis.rs b/crates/ra_ide/src/mock_analysis.rs index 889b84c59..490ee0dc3 100644 --- a/crates/ra_ide/src/mock_analysis.rs +++ b/crates/ra_ide/src/mock_analysis.rs @@ -99,7 +99,7 @@ impl MockAnalysis { root_crate = Some(crate_graph.add_crate_root( file_id, edition, - None, + Some(CrateName::new("test").unwrap()), cfg, env, Default::default(), diff --git a/crates/ra_ide_db/Cargo.toml b/crates/ra_ide_db/Cargo.toml index c3921bd40..b14206c9b 100644 --- a/crates/ra_ide_db/Cargo.toml +++ b/crates/ra_ide_db/Cargo.toml @@ -24,6 +24,7 @@ ra_text_edit = { path = "../ra_text_edit" } ra_db = { path = "../ra_db" } ra_prof = { path = "../ra_prof" } test_utils = { path = "../test_utils" } +ra_hir_def = { path = "../ra_hir_def" } # ra_ide should depend only on the top-level `hir` package. if you need # something from some `hir_xxx` subpackage, reexport the API via `hir`. diff --git a/crates/ra_ide_db/src/defs.rs b/crates/ra_ide_db/src/defs.rs index 3ef5e74b6..a158169e3 100644 --- a/crates/ra_ide_db/src/defs.rs +++ b/crates/ra_ide_db/src/defs.rs @@ -7,7 +7,7 @@ use hir::{ Field, HasVisibility, ImplDef, Local, MacroDef, Module, ModuleDef, Name, PathResolution, - Semantics, TypeParam, Visibility, + Semantics, TypeParam, Visibility, db::{DefDatabase, HirDatabase}, }; use ra_prof::profile; use ra_syntax::{ @@ -16,6 +16,7 @@ use ra_syntax::{ }; use crate::RootDatabase; +use ra_hir_def::resolver::{Resolver, HasResolver}; // FIXME: a more precise name would probably be `Symbol`? #[derive(Debug, PartialEq, Eq, Copy, Clone)] @@ -76,6 +77,20 @@ impl Definition { }; Some(name) } + + pub fn resolver(&self, db: &D) -> Option { + use hir::VariantDef; + use ra_hir_def::*; + Some(match self { + Definition::ModuleDef(def) => def.resolver(db)?, + Definition::Field(field) => Into::::into(Into::::into(field.parent_def(db))).resolver(db), + Definition::Macro(m) => Into::::into(m.module(db)?).resolver(db), + Definition::SelfType(imp) => Into::::into(imp.clone()).resolver(db), + // it's possible, read probable, that other arms of this are also unreachable + Definition::Local(_local) => unreachable!(), + Definition::TypeParam(tp) => Into::::into(tp.module(db)).resolver(db) + }) + } } #[derive(Debug)] diff --git a/crates/rust-analyzer/src/handlers.rs b/crates/rust-analyzer/src/handlers.rs index 951006771..6c0d3e19c 100644 --- a/crates/rust-analyzer/src/handlers.rs +++ b/crates/rust-analyzer/src/handlers.rs @@ -4,8 +4,7 @@ use std::{ io::Write as _, - process::{self, Stdio}, - sync::Arc + process::{self, Stdio} }; use lsp_server::ErrorCode; -- cgit v1.2.3