From bfda0d25834250a3adbcd0d26953a1cdc6662e7f Mon Sep 17 00:00:00 2001 From: Zac Pullar-Strecker Date: Sun, 30 Aug 2020 20:02:29 +1200 Subject: [PATCH 01/17] WIP: Command to open docs under cursor --- crates/ide/src/lib.rs | 8 +++ crates/ide/src/link_rewrite.rs | 82 ++++++++++++++++++++++++++- crates/rust-analyzer/src/handlers.rs | 15 ++++- crates/rust-analyzer/src/lsp_ext.rs | 28 +++++++++ crates/rust-analyzer/src/main_loop.rs | 1 + editors/code/package.json | 9 +++ editors/code/src/commands.ts | 25 +++++++- editors/code/src/lsp_ext.ts | 11 ++++ editors/code/src/main.ts | 1 + 9 files changed, 176 insertions(+), 4 deletions(-) diff --git a/crates/ide/src/lib.rs b/crates/ide/src/lib.rs index 57f3581b6bd0..645369597936 100644 --- a/crates/ide/src/lib.rs +++ b/crates/ide/src/lib.rs @@ -382,6 +382,14 @@ impl Analysis { self.with_db(|db| hover::hover(db, position, links_in_hover, markdown)) } + /// Return URL(s) for the documentation of the symbol under the cursor. + pub fn get_doc_url( + &self, + position: FilePosition, + ) -> Cancelable> { + self.with_db(|db| link_rewrite::get_doc_url(db, &position)) + } + /// Computes parameter information for the given call expression. pub fn call_info(&self, position: FilePosition) -> Cancelable> { self.with_db(|db| call_info::call_info(db, position)) diff --git a/crates/ide/src/link_rewrite.rs b/crates/ide/src/link_rewrite.rs index c317a2379b4f..80005c06edf1 100644 --- a/crates/ide/src/link_rewrite.rs +++ b/crates/ide/src/link_rewrite.rs @@ -8,6 +8,16 @@ use pulldown_cmark::{CowStr, Event, LinkType, Options, Parser, Tag}; use pulldown_cmark_to_cmark::{cmark_with_options, Options as CmarkOptions}; use url::Url; +use crate::{FilePosition, Semantics}; +use hir::{get_doc_link, resolve_doc_link}; +use ide_db::{ + defs::{classify_name, classify_name_ref, Definition}, + RootDatabase, +}; +use syntax::{ast, match_ast, AstNode, SyntaxKind::*, SyntaxToken, TokenAtOffset, T}; + +pub type DocumentationLink = String; + /// Rewrite documentation links in markdown to point to an online host (e.g. docs.rs) pub fn rewrite_links(db: &RootDatabase, markdown: &str, definition: &Definition) -> String { let doc = Parser::new_with_broken_link_callback( @@ -80,6 +90,37 @@ pub fn remove_links(markdown: &str) -> String { out } +pub fn get_doc_link(db: &dyn HirDatabase, definition: &T) -> Option { + eprintln!("hir::doc_links::get_doc_link"); + let module_def = definition.clone().try_into_module_def()?; + + get_doc_link_impl(db, &module_def) +} + +// TODO: +// BUG: For Option +// Returns https://doc.rust-lang.org/nightly/core/prelude/v1/enum.Option.html#variant.Some +// Instead of https://doc.rust-lang.org/nightly/core/option/enum.Option.html +// +// BUG: For methods +// import_map.path_of(ns) fails, is not designed to resolve methods +fn get_doc_link_impl(db: &dyn HirDatabase, moddef: &ModuleDef) -> Option { + eprintln!("get_doc_link_impl: {:#?}", moddef); + let ns = ItemInNs::Types(moddef.clone().into()); + + let module = moddef.module(db)?; + let krate = module.krate(); + let import_map = db.import_map(krate.into()); + let base = once(krate.display_name(db).unwrap()) + .chain(import_map.path_of(ns).unwrap().segments.iter().map(|name| format!("{}", name))) + .join("/"); + + get_doc_url(db, &krate) + .and_then(|url| url.join(&base).ok()) + .and_then(|url| get_symbol_filename(db, &moddef).as_deref().and_then(|f| url.join(f).ok())) + .map(|url| url.into_string()) +} + fn rewrite_intra_doc_link( db: &RootDatabase, def: Definition, @@ -138,7 +179,34 @@ fn rewrite_url_link(db: &RootDatabase, def: ModuleDef, target: &str) -> Option Option { + let sema = Semantics::new(db); + let file = sema.parse(position.file_id).syntax().clone(); + let token = pick_best(file.token_at_offset(position.offset))?; + let token = sema.descend_into_macros(token); + + let node = token.parent(); + let definition = match_ast! { + match node { + ast::NameRef(name_ref) => classify_name_ref(&sema, &name_ref).map(|d| d.definition(sema.db)), + ast::Name(name) => classify_name(&sema, &name).map(|d| d.definition(sema.db)), + _ => None, + } + }; + + match definition? { + Definition::Macro(t) => get_doc_link(db, &t), + Definition::Field(t) => get_doc_link(db, &t), + Definition::ModuleDef(t) => get_doc_link(db, &t), + Definition::SelfType(t) => get_doc_link(db, &t), + Definition::Local(t) => get_doc_link(db, &t), + Definition::TypeParam(t) => get_doc_link(db, &t), + } +} + +/// Rewrites a markdown document, applying 'callback' to each link. fn map_links<'e>( events: impl Iterator>, callback: impl Fn(&str, &str) -> (String, String), @@ -275,3 +343,15 @@ fn get_symbol_filename(db: &RootDatabase, definition: &ModuleDef) -> Option format!("static.{}.html", s.name(db)?), }) } + +fn pick_best(tokens: TokenAtOffset) -> Option { + return tokens.max_by_key(priority); + fn priority(n: &SyntaxToken) -> usize { + match n.kind() { + IDENT | INT_NUMBER => 3, + T!['('] | T![')'] => 2, + kind if kind.is_trivia() => 0, + _ => 1, + } + } +} diff --git a/crates/rust-analyzer/src/handlers.rs b/crates/rust-analyzer/src/handlers.rs index 468655f9c39e..ec8c8fecdd65 100644 --- a/crates/rust-analyzer/src/handlers.rs +++ b/crates/rust-analyzer/src/handlers.rs @@ -34,7 +34,7 @@ use crate::{ config::RustfmtConfig, from_json, from_proto, global_state::{GlobalState, GlobalStateSnapshot}, - lsp_ext::{self, InlayHint, InlayHintsParams}, + lsp_ext::{self, DocumentationLink, InlayHint, InlayHintsParams, OpenDocsParams}, to_proto, LspError, Result, }; @@ -1310,6 +1310,19 @@ pub(crate) fn handle_semantic_tokens_range( Ok(Some(semantic_tokens.into())) } +pub(crate) fn handle_open_docs( + snap: GlobalStateSnapshot, + params: OpenDocsParams, +) -> Result { + let _p = profile::span("handle_open_docs"); + let position = from_proto::file_position(&snap, params.position)?; + + // FIXME: Propogate or ignore this error instead of panicking. + let remote = snap.analysis.get_doc_url(position)?.unwrap(); + + Ok(DocumentationLink { remote }) +} + fn implementation_title(count: usize) -> String { if count == 1 { "1 implementation".into() diff --git a/crates/rust-analyzer/src/lsp_ext.rs b/crates/rust-analyzer/src/lsp_ext.rs index fee0bb69c70b..83a20802f655 100644 --- a/crates/rust-analyzer/src/lsp_ext.rs +++ b/crates/rust-analyzer/src/lsp_ext.rs @@ -347,3 +347,31 @@ pub struct CommandLink { #[serde(skip_serializing_if = "Option::is_none")] pub tooltip: Option, } + +pub enum OpenDocs {} + +impl Request for OpenDocs { + type Params = OpenDocsParams; + type Result = DocumentationLink; + const METHOD: &'static str = "rust-analyzer/openDocs"; +} + +#[derive(Debug, PartialEq, Clone, Deserialize, Serialize)] +#[serde(rename_all = "camelCase")] +pub struct OpenDocsParams { + // TODO: I don't know the difference between these two methods of passing position. + #[serde(flatten)] + pub position: lsp_types::TextDocumentPositionParams, + // pub textDocument: lsp_types::TextDocumentIdentifier, + // pub position: lsp_types::Position, +} + +#[derive(Debug, PartialEq, Clone, Deserialize, Serialize)] +#[serde(rename_all = "camelCase")] +pub struct DocumentationLink { + pub remote: String, // TODO: Better API? + // #[serde(skip_serializing_if = "Option::is_none")] + // pub remote: Option, + // #[serde(skip_serializing_if = "Option::is_none")] + // pub local: Option +} diff --git a/crates/rust-analyzer/src/main_loop.rs b/crates/rust-analyzer/src/main_loop.rs index 4b7ac8224ef1..75f85011d9e8 100644 --- a/crates/rust-analyzer/src/main_loop.rs +++ b/crates/rust-analyzer/src/main_loop.rs @@ -384,6 +384,7 @@ impl GlobalState { .on::(handlers::handle_code_action)? .on::(handlers::handle_resolve_code_action)? .on::(handlers::handle_hover)? + .on::(handlers::handle_open_docs)? .on::(handlers::handle_on_type_formatting)? .on::(handlers::handle_document_symbol)? .on::(handlers::handle_workspace_symbol)? diff --git a/editors/code/package.json b/editors/code/package.json index 6a712a8a82f0..4bd3117fc806 100644 --- a/editors/code/package.json +++ b/editors/code/package.json @@ -182,6 +182,11 @@ "command": "rust-analyzer.toggleInlayHints", "title": "Toggle inlay hints", "category": "Rust Analyzer" + }, + { + "command": "rust-analyzer.openDocs", + "title": "Open docs under cursor", + "category": "Rust Analyzer" } ], "keybindings": [ @@ -1044,6 +1049,10 @@ { "command": "rust-analyzer.toggleInlayHints", "when": "inRustProject" + }, + { + "command": "rust-analyzer.openDocs", + "when": "inRustProject" } ] } diff --git a/editors/code/src/commands.ts b/editors/code/src/commands.ts index 1a90f1b7d9a0..b22cd450b072 100644 --- a/editors/code/src/commands.ts +++ b/editors/code/src/commands.ts @@ -419,10 +419,31 @@ export function gotoLocation(ctx: Ctx): Cmd { }; } +export function openDocs(ctx: Ctx): Cmd { + return async () => { + console.log("running openDocs"); + + const client = ctx.client; + const editor = vscode.window.activeTextEditor; + if (!editor || !client) { + console.log("not yet ready"); + return + }; + + const position = editor.selection.active; + const textDocument = { uri: editor.document.uri.toString() }; + + const doclink = await client.sendRequest(ra.openDocs, { position, textDocument }); + + vscode.commands.executeCommand("vscode.open", vscode.Uri.parse(doclink.remote)); + }; + +} + export function resolveCodeAction(ctx: Ctx): Cmd { const client = ctx.client; - return async (params: ra.ResolveCodeActionParams) => { - const item: lc.WorkspaceEdit = await client.sendRequest(ra.resolveCodeAction, params); + return async () => { + const item: lc.WorkspaceEdit = await client.sendRequest(ra.resolveCodeAction, null); if (!item) { return; } diff --git a/editors/code/src/lsp_ext.ts b/editors/code/src/lsp_ext.ts index f286b68a6858..569e747bd47c 100644 --- a/editors/code/src/lsp_ext.ts +++ b/editors/code/src/lsp_ext.ts @@ -118,3 +118,14 @@ export interface CommandLinkGroup { title?: string; commands: CommandLink[]; } + +export interface DocumentationLink { + remote: string; +} + +export interface OpenDocsParams { + textDocument: lc.TextDocumentIdentifier; + position: lc.Position; +} + +export const openDocs = new lc.RequestType('rust-analyzer/openDocs'); diff --git a/editors/code/src/main.ts b/editors/code/src/main.ts index 2896d90ac94b..09543e348a82 100644 --- a/editors/code/src/main.ts +++ b/editors/code/src/main.ts @@ -110,6 +110,7 @@ async function tryActivate(context: vscode.ExtensionContext) { ctx.registerCommand('run', commands.run); ctx.registerCommand('debug', commands.debug); ctx.registerCommand('newDebugConfig', commands.newDebugConfig); + ctx.registerCommand('openDocs', commands.openDocs); defaultOnEnter.dispose(); ctx.registerCommand('onEnter', commands.onEnter); From a06d736b77770e4c1e738086c81b4fd60fcfcb23 Mon Sep 17 00:00:00 2001 From: Zac Pullar-Strecker Date: Mon, 31 Aug 2020 20:26:55 +1200 Subject: [PATCH 02/17] Add support for struct & trait methods --- crates/ide/src/link_rewrite.rs | 90 +++++++++++++++++++++++++++++++--- 1 file changed, 83 insertions(+), 7 deletions(-) diff --git a/crates/ide/src/link_rewrite.rs b/crates/ide/src/link_rewrite.rs index 80005c06edf1..1e102997f29f 100644 --- a/crates/ide/src/link_rewrite.rs +++ b/crates/ide/src/link_rewrite.rs @@ -91,7 +91,6 @@ pub fn remove_links(markdown: &str) -> String { } pub fn get_doc_link(db: &dyn HirDatabase, definition: &T) -> Option { - eprintln!("hir::doc_links::get_doc_link"); let module_def = definition.clone().try_into_module_def()?; get_doc_link_impl(db, &module_def) @@ -105,8 +104,31 @@ pub fn get_doc_link(db: &dyn HirDatabase, definition: &T) // BUG: For methods // import_map.path_of(ns) fails, is not designed to resolve methods fn get_doc_link_impl(db: &dyn HirDatabase, moddef: &ModuleDef) -> Option { - eprintln!("get_doc_link_impl: {:#?}", moddef); - let ns = ItemInNs::Types(moddef.clone().into()); + // Get the outermost definition for the moduledef. This is used to resolve the public path to the type, + // then we can join the method, field, etc onto it if required. + let target_def: ModuleDef = match moddef { + ModuleDef::Function(f) => match f.as_assoc_item(db).map(|assoc| assoc.container(db)) { + Some(AssocItemContainer::Trait(t)) => t.into(), + Some(AssocItemContainer::ImplDef(imp)) => { + let resolver = ModuleId::from(imp.module(db)).resolver(db.upcast()); + let ctx = TyLoweringContext::new(db, &resolver); + Adt::from( + Ty::from_hir( + &ctx, + &imp.target_trait(db).unwrap_or_else(|| imp.target_type(db)), + ) + .as_adt() + .map(|t| t.0) + .unwrap(), + ) + .into() + } + None => ModuleDef::Function(*f), + }, + moddef => *moddef, + }; + + let ns = ItemInNs::Types(target_def.clone().into()); let module = moddef.module(db)?; let krate = module.krate(); @@ -117,7 +139,28 @@ fn get_doc_link_impl(db: &dyn HirDatabase, moddef: &ModuleDef) -> Option get_doc_url(db, &krate) .and_then(|url| url.join(&base).ok()) - .and_then(|url| get_symbol_filename(db, &moddef).as_deref().and_then(|f| url.join(f).ok())) + .and_then(|url| { + get_symbol_filename(db, &target_def).as_deref().and_then(|f| url.join(f).ok()) + }) + .and_then(|url| match moddef { + ModuleDef::Function(f) => { + get_symbol_fragment(db, &FieldOrAssocItem::AssocItem(AssocItem::Function(*f))) + .as_deref() + .and_then(|f| url.join(f).ok()) + } + ModuleDef::Const(c) => { + get_symbol_fragment(db, &FieldOrAssocItem::AssocItem(AssocItem::Const(*c))) + .as_deref() + .and_then(|f| url.join(f).ok()) + } + ModuleDef::TypeAlias(ty) => { + get_symbol_fragment(db, &FieldOrAssocItem::AssocItem(AssocItem::TypeAlias(*ty))) + .as_deref() + .and_then(|f| url.join(f).ok()) + } + // TODO: Field <- this requires passing in a definition or something + _ => Some(url), + }) .map(|url| url.into_string()) } @@ -307,6 +350,12 @@ fn ns_from_intra_spec(s: &str) -> Option { .next() } +/// Get the root URL for the documentation of a crate. +/// +/// ``` +/// https://doc.rust-lang.org/std/iter/trait.Iterator.html#tymethod.next +/// ^^^^^^^^^^^^^^^^^^^^^^^^^^ +/// ``` fn get_doc_url(db: &RootDatabase, krate: &Crate) -> Option { krate .get_html_root_url(db) @@ -323,8 +372,11 @@ fn get_doc_url(db: &RootDatabase, krate: &Crate) -> Option { /// Get the filename and extension generated for a symbol by rustdoc. /// -/// Example: `struct.Shard.html` -fn get_symbol_filename(db: &RootDatabase, definition: &ModuleDef) -> Option { +/// ``` +/// https://doc.rust-lang.org/std/iter/trait.Iterator.html#tymethod.next +/// ^^^^^^^^^^^^^^^^^^^ +/// ``` +fn get_symbol_filename(db: &dyn HirDatabase, definition: &ModuleDef) -> Option { Some(match definition { ModuleDef::Adt(adt) => match adt { Adt::Struct(s) => format!("struct.{}.html", s.name(db)), @@ -334,7 +386,7 @@ fn get_symbol_filename(db: &RootDatabase, definition: &ModuleDef) -> Option "index.html".to_string(), ModuleDef::Trait(t) => format!("trait.{}.html", t.name(db)), ModuleDef::TypeAlias(t) => format!("type.{}.html", t.name(db)), - ModuleDef::BuiltinType(t) => format!("primitive.{}.html", t), + ModuleDef::BuiltinType(t) => format!("primitive.{}.html", t.as_name()), ModuleDef::Function(f) => format!("fn.{}.html", f.name(db)), ModuleDef::EnumVariant(ev) => { format!("enum.{}.html#variant.{}", ev.parent_enum(db).name(db), ev.name(db)) @@ -344,6 +396,30 @@ fn get_symbol_filename(db: &RootDatabase, definition: &ModuleDef) -> Option Option { + Some(match field_or_assoc { + FieldOrAssocItem::Field(field) => format!("#structfield.{}", field.name(db)), + FieldOrAssocItem::AssocItem(assoc) => match assoc { + // TODO: Rustdoc sometimes uses tymethod instead of method. This case needs to be investigated. + AssocItem::Function(function) => format!("#method.{}", function.name(db)), + // TODO: This might be the old method for documenting associated constants, i32::MAX uses a separate page... + AssocItem::Const(constant) => format!("#associatedconstant.{}", constant.name(db)?), + AssocItem::TypeAlias(ty) => format!("#associatedtype.{}", ty.name(db)), + }, + }) +} + fn pick_best(tokens: TokenAtOffset) -> Option { return tokens.max_by_key(priority); fn priority(n: &SyntaxToken) -> usize { From 8c32bdea3662f4c65810e2d92569b0cb4e3872d9 Mon Sep 17 00:00:00 2001 From: Zac Pullar-Strecker Date: Mon, 31 Aug 2020 20:38:10 +1200 Subject: [PATCH 03/17] Rename ide::link_rewrite -> ide::doc_links & tidy imports --- crates/ide/src/{link_rewrite.rs => doc_links.rs} | 0 crates/ide/src/hover.rs | 2 +- crates/ide/src/lib.rs | 6 +++--- 3 files changed, 4 insertions(+), 4 deletions(-) rename crates/ide/src/{link_rewrite.rs => doc_links.rs} (100%) diff --git a/crates/ide/src/link_rewrite.rs b/crates/ide/src/doc_links.rs similarity index 100% rename from crates/ide/src/link_rewrite.rs rename to crates/ide/src/doc_links.rs diff --git a/crates/ide/src/hover.rs b/crates/ide/src/hover.rs index 53265488e60f..a53359d03635 100644 --- a/crates/ide/src/hover.rs +++ b/crates/ide/src/hover.rs @@ -14,8 +14,8 @@ use test_utils::mark; use crate::{ display::{macro_label, ShortLabel, ToNav, TryToNav}, - link_rewrite::{remove_links, rewrite_links}, markdown_remove::remove_markdown, + doc_links::{remove_links, rewrite_links}, markup::Markup, runnables::runnable, FileId, FilePosition, NavigationTarget, RangeInfo, Runnable, diff --git a/crates/ide/src/lib.rs b/crates/ide/src/lib.rs index 645369597936..b92e6d9ea876 100644 --- a/crates/ide/src/lib.rs +++ b/crates/ide/src/lib.rs @@ -45,8 +45,8 @@ mod status; mod syntax_highlighting; mod syntax_tree; mod typing; -mod link_rewrite; mod markdown_remove; +mod doc_links; use std::sync::Arc; @@ -386,8 +386,8 @@ impl Analysis { pub fn get_doc_url( &self, position: FilePosition, - ) -> Cancelable> { - self.with_db(|db| link_rewrite::get_doc_url(db, &position)) + ) -> Cancelable> { + self.with_db(|db| doc_links::get_doc_url(db, &position)) } /// Computes parameter information for the given call expression. From a14194b428efdb09cc45f9862ec34bef0038cd35 Mon Sep 17 00:00:00 2001 From: Zac Pullar-Strecker Date: Tue, 1 Sep 2020 11:38:32 +1200 Subject: [PATCH 04/17] Changes from review --- crates/ide/src/doc_links.rs | 3 --- crates/ide/src/lib.rs | 2 +- crates/rust-analyzer/src/handlers.rs | 13 ++++++------ crates/rust-analyzer/src/lsp_ext.rs | 30 +++++---------------------- crates/rust-analyzer/src/main_loop.rs | 2 +- editors/code/src/lsp_ext.ts | 11 +--------- 6 files changed, 14 insertions(+), 47 deletions(-) diff --git a/crates/ide/src/doc_links.rs b/crates/ide/src/doc_links.rs index 1e102997f29f..7fe88577db79 100644 --- a/crates/ide/src/doc_links.rs +++ b/crates/ide/src/doc_links.rs @@ -100,9 +100,6 @@ pub fn get_doc_link(db: &dyn HirDatabase, definition: &T) // BUG: For Option // Returns https://doc.rust-lang.org/nightly/core/prelude/v1/enum.Option.html#variant.Some // Instead of https://doc.rust-lang.org/nightly/core/option/enum.Option.html -// -// BUG: For methods -// import_map.path_of(ns) fails, is not designed to resolve methods fn get_doc_link_impl(db: &dyn HirDatabase, moddef: &ModuleDef) -> Option { // Get the outermost definition for the moduledef. This is used to resolve the public path to the type, // then we can join the method, field, etc onto it if required. diff --git a/crates/ide/src/lib.rs b/crates/ide/src/lib.rs index b92e6d9ea876..0580d2979e70 100644 --- a/crates/ide/src/lib.rs +++ b/crates/ide/src/lib.rs @@ -383,7 +383,7 @@ impl Analysis { } /// Return URL(s) for the documentation of the symbol under the cursor. - pub fn get_doc_url( + pub fn external_docs( &self, position: FilePosition, ) -> Cancelable> { diff --git a/crates/rust-analyzer/src/handlers.rs b/crates/rust-analyzer/src/handlers.rs index ec8c8fecdd65..ba73abcacd2b 100644 --- a/crates/rust-analyzer/src/handlers.rs +++ b/crates/rust-analyzer/src/handlers.rs @@ -34,7 +34,7 @@ use crate::{ config::RustfmtConfig, from_json, from_proto, global_state::{GlobalState, GlobalStateSnapshot}, - lsp_ext::{self, DocumentationLink, InlayHint, InlayHintsParams, OpenDocsParams}, + lsp_ext::{self, InlayHint, InlayHintsParams}, to_proto, LspError, Result, }; @@ -1312,15 +1312,14 @@ pub(crate) fn handle_semantic_tokens_range( pub(crate) fn handle_open_docs( snap: GlobalStateSnapshot, - params: OpenDocsParams, -) -> Result { + params: lsp_types::TextDocumentPositionParams, +) -> Result> { let _p = profile::span("handle_open_docs"); - let position = from_proto::file_position(&snap, params.position)?; + let position = from_proto::file_position(&snap, params)?; - // FIXME: Propogate or ignore this error instead of panicking. - let remote = snap.analysis.get_doc_url(position)?.unwrap(); + let remote = snap.analysis.external_docs(position)?; - Ok(DocumentationLink { remote }) + Ok(remote.and_then(|remote| Url::parse(&remote).ok())) } fn implementation_title(count: usize) -> String { diff --git a/crates/rust-analyzer/src/lsp_ext.rs b/crates/rust-analyzer/src/lsp_ext.rs index 83a20802f655..f31f8d9001b7 100644 --- a/crates/rust-analyzer/src/lsp_ext.rs +++ b/crates/rust-analyzer/src/lsp_ext.rs @@ -348,30 +348,10 @@ pub struct CommandLink { pub tooltip: Option, } -pub enum OpenDocs {} +pub enum ExternalDocs {} -impl Request for OpenDocs { - type Params = OpenDocsParams; - type Result = DocumentationLink; - const METHOD: &'static str = "rust-analyzer/openDocs"; -} - -#[derive(Debug, PartialEq, Clone, Deserialize, Serialize)] -#[serde(rename_all = "camelCase")] -pub struct OpenDocsParams { - // TODO: I don't know the difference between these two methods of passing position. - #[serde(flatten)] - pub position: lsp_types::TextDocumentPositionParams, - // pub textDocument: lsp_types::TextDocumentIdentifier, - // pub position: lsp_types::Position, -} - -#[derive(Debug, PartialEq, Clone, Deserialize, Serialize)] -#[serde(rename_all = "camelCase")] -pub struct DocumentationLink { - pub remote: String, // TODO: Better API? - // #[serde(skip_serializing_if = "Option::is_none")] - // pub remote: Option, - // #[serde(skip_serializing_if = "Option::is_none")] - // pub local: Option +impl Request for ExternalDocs { + type Params = lsp_types::TextDocumentPositionParams; + type Result = Option; + const METHOD: &'static str = "experimental/externalDocs"; } diff --git a/crates/rust-analyzer/src/main_loop.rs b/crates/rust-analyzer/src/main_loop.rs index 75f85011d9e8..06b38d99c8ee 100644 --- a/crates/rust-analyzer/src/main_loop.rs +++ b/crates/rust-analyzer/src/main_loop.rs @@ -384,7 +384,7 @@ impl GlobalState { .on::(handlers::handle_code_action)? .on::(handlers::handle_resolve_code_action)? .on::(handlers::handle_hover)? - .on::(handlers::handle_open_docs)? + .on::(handlers::handle_open_docs)? .on::(handlers::handle_on_type_formatting)? .on::(handlers::handle_document_symbol)? .on::(handlers::handle_workspace_symbol)? diff --git a/editors/code/src/lsp_ext.ts b/editors/code/src/lsp_ext.ts index 569e747bd47c..56280471509d 100644 --- a/editors/code/src/lsp_ext.ts +++ b/editors/code/src/lsp_ext.ts @@ -119,13 +119,4 @@ export interface CommandLinkGroup { commands: CommandLink[]; } -export interface DocumentationLink { - remote: string; -} - -export interface OpenDocsParams { - textDocument: lc.TextDocumentIdentifier; - position: lc.Position; -} - -export const openDocs = new lc.RequestType('rust-analyzer/openDocs'); +export const openDocs = new lc.RequestType('experimental/externalDocs'); From 974518fde7975b839ed4ccd4c5ce1d48cd6db3c7 Mon Sep 17 00:00:00 2001 From: Zac Pullar-Strecker Date: Tue, 1 Sep 2020 20:26:10 +1200 Subject: [PATCH 05/17] Code reorganisation and field support --- crates/hir/src/code_model.rs | 48 ++++++++++++++++- crates/ide/src/doc_links.rs | 99 +++++++++++++++++------------------- crates/ide/src/lib.rs | 2 +- crates/stdx/src/macros.rs | 8 ++- editors/code/src/commands.ts | 6 +-- editors/code/src/lsp_ext.ts | 2 +- 6 files changed, 105 insertions(+), 60 deletions(-) diff --git a/crates/hir/src/code_model.rs b/crates/hir/src/code_model.rs index 031c91ccf612..1dd6d73f3807 100644 --- a/crates/hir/src/code_model.rs +++ b/crates/hir/src/code_model.rs @@ -35,7 +35,7 @@ use hir_ty::{ traits::SolutionVariables, ApplicationTy, BoundVar, CallableDefId, Canonical, DebruijnIndex, FnSig, GenericPredicate, InEnvironment, Obligation, ProjectionPredicate, ProjectionTy, Substs, TraitEnvironment, Ty, - TyDefId, TyKind, TypeCtor, + TyDefId, TyKind, TypeCtor, TyLoweringContext, TypeCtor, }; use rustc_hash::FxHashSet; use stdx::impl_from; @@ -186,6 +186,25 @@ impl_from!( for ModuleDef ); +impl From for ModuleDef { + fn from(mowner: MethodOwner) -> Self { + match mowner { + MethodOwner::Trait(t) => t.into(), + MethodOwner::Adt(t) => t.into(), + } + } +} + +impl From for ModuleDef { + fn from(var: VariantDef) -> Self { + match var { + VariantDef::Struct(t) => Adt::from(t).into(), + VariantDef::Union(t) => Adt::from(t).into(), + VariantDef::EnumVariant(t) => t.into(), + } + } +} + impl ModuleDef { pub fn module(self, db: &dyn HirDatabase) -> Option { match self { @@ -752,8 +771,35 @@ impl Function { pub fn diagnostics(self, db: &dyn HirDatabase, sink: &mut DiagnosticSink) { hir_ty::diagnostics::validate_body(db, self.id.into(), sink) } + + pub fn parent_def(self, db: &dyn HirDatabase) -> Option { + match self.as_assoc_item(db).map(|assoc| assoc.container(db)) { + Some(AssocItemContainer::Trait(t)) => Some(t.into()), + Some(AssocItemContainer::ImplDef(imp)) => { + let resolver = ModuleId::from(imp.module(db)).resolver(db.upcast()); + let ctx = TyLoweringContext::new(db, &resolver); + let adt = Ty::from_hir( + &ctx, + &imp.target_trait(db).unwrap_or_else(|| imp.target_type(db)), + ) + .as_adt() + .map(|t| t.0) + .unwrap(); + Some(Adt::from(adt).into()) + } + None => None, + } + } } +#[derive(Debug)] +pub enum MethodOwner { + Trait(Trait), + Adt(Adt), +} + +impl_from!(Trait, Adt for MethodOwner); + // Note: logically, this belongs to `hir_ty`, but we are not using it there yet. pub enum Access { Shared, diff --git a/crates/ide/src/doc_links.rs b/crates/ide/src/doc_links.rs index 7fe88577db79..512c42c4d169 100644 --- a/crates/ide/src/doc_links.rs +++ b/crates/ide/src/doc_links.rs @@ -2,20 +2,27 @@ //! //! Most of the implementation can be found in [`hir::doc_links`]. -use hir::{Adt, Crate, HasAttrs, ModuleDef}; -use ide_db::{defs::Definition, RootDatabase}; -use pulldown_cmark::{CowStr, Event, LinkType, Options, Parser, Tag}; +use std::iter::once; + +use itertools::Itertools; use pulldown_cmark_to_cmark::{cmark_with_options, Options as CmarkOptions}; +use pulldown_cmark::{CowStr, Event, LinkType, Options, Parser, Tag}; use url::Url; -use crate::{FilePosition, Semantics}; -use hir::{get_doc_link, resolve_doc_link}; +use ide_db::{defs::Definition, RootDatabase}; + +use hir::{ + db::{DefDatabase, HirDatabase}, + Adt, AsName, AssocItem, Crate, Field, HasAttrs, ItemInNs, ModuleDef, +}; use ide_db::{ defs::{classify_name, classify_name_ref, Definition}, RootDatabase, }; use syntax::{ast, match_ast, AstNode, SyntaxKind::*, SyntaxToken, TokenAtOffset, T}; +use crate::{FilePosition, Semantics}; + pub type DocumentationLink = String; /// Rewrite documentation links in markdown to point to an online host (e.g. docs.rs) @@ -100,64 +107,58 @@ pub fn get_doc_link(db: &dyn HirDatabase, definition: &T) // BUG: For Option // Returns https://doc.rust-lang.org/nightly/core/prelude/v1/enum.Option.html#variant.Some // Instead of https://doc.rust-lang.org/nightly/core/option/enum.Option.html -fn get_doc_link_impl(db: &dyn HirDatabase, moddef: &ModuleDef) -> Option { +// This could be worked around by turning the `EnumVariant` into `Enum` before attempting resolution, +// but it's really just working around the problem. Ideally we need to implement a slightly different +// version of import map which follows the same process as rustdoc. Otherwise there'll always be some +// edge cases where we select the wrong import path. +fn get_doc_link(db: &RootDatabase, definition: Definition) -> Option { // Get the outermost definition for the moduledef. This is used to resolve the public path to the type, // then we can join the method, field, etc onto it if required. - let target_def: ModuleDef = match moddef { - ModuleDef::Function(f) => match f.as_assoc_item(db).map(|assoc| assoc.container(db)) { - Some(AssocItemContainer::Trait(t)) => t.into(), - Some(AssocItemContainer::ImplDef(imp)) => { - let resolver = ModuleId::from(imp.module(db)).resolver(db.upcast()); - let ctx = TyLoweringContext::new(db, &resolver); - Adt::from( - Ty::from_hir( - &ctx, - &imp.target_trait(db).unwrap_or_else(|| imp.target_type(db)), - ) - .as_adt() - .map(|t| t.0) - .unwrap(), - ) - .into() + let target_def: ModuleDef = match definition { + Definition::ModuleDef(moddef) => match moddef { + ModuleDef::Function(f) => { + f.parent_def(db).map(|mowner| mowner.into()).unwrap_or_else(|| f.clone().into()) } - None => ModuleDef::Function(*f), + moddef => moddef, }, - moddef => *moddef, + Definition::Field(f) => f.parent_def(db).into(), + // FIXME: Handle macros + _ => return None, }; let ns = ItemInNs::Types(target_def.clone().into()); - let module = moddef.module(db)?; + let module = definition.module(db)?; let krate = module.krate(); let import_map = db.import_map(krate.into()); let base = once(krate.display_name(db).unwrap()) .chain(import_map.path_of(ns).unwrap().segments.iter().map(|name| format!("{}", name))) .join("/"); - get_doc_url(db, &krate) - .and_then(|url| url.join(&base).ok()) - .and_then(|url| { - get_symbol_filename(db, &target_def).as_deref().and_then(|f| url.join(f).ok()) - }) - .and_then(|url| match moddef { + let filename = get_symbol_filename(db, &target_def); + let fragment = match definition { + Definition::ModuleDef(moddef) => match moddef { ModuleDef::Function(f) => { - get_symbol_fragment(db, &FieldOrAssocItem::AssocItem(AssocItem::Function(*f))) - .as_deref() - .and_then(|f| url.join(f).ok()) + get_symbol_fragment(db, &FieldOrAssocItem::AssocItem(AssocItem::Function(f))) } ModuleDef::Const(c) => { - get_symbol_fragment(db, &FieldOrAssocItem::AssocItem(AssocItem::Const(*c))) - .as_deref() - .and_then(|f| url.join(f).ok()) + get_symbol_fragment(db, &FieldOrAssocItem::AssocItem(AssocItem::Const(c))) } ModuleDef::TypeAlias(ty) => { - get_symbol_fragment(db, &FieldOrAssocItem::AssocItem(AssocItem::TypeAlias(*ty))) - .as_deref() - .and_then(|f| url.join(f).ok()) + get_symbol_fragment(db, &FieldOrAssocItem::AssocItem(AssocItem::TypeAlias(ty))) } - // TODO: Field <- this requires passing in a definition or something - _ => Some(url), - }) + _ => None, + }, + Definition::Field(field) => get_symbol_fragment(db, &FieldOrAssocItem::Field(field)), + _ => None, + }; + + get_doc_url(db, &krate) + .and_then(|url| url.join(&base).ok()) + .and_then(|url| filename.as_deref().and_then(|f| url.join(f).ok())) + .and_then( + |url| if let Some(fragment) = fragment { url.join(&fragment).ok() } else { Some(url) }, + ) .map(|url| url.into_string()) } @@ -219,9 +220,8 @@ fn rewrite_url_link(db: &RootDatabase, def: ModuleDef, target: &str) -> Option Option { +pub fn external_docs(db: &RootDatabase, position: &FilePosition) -> Option { let sema = Semantics::new(db); let file = sema.parse(position.file_id).syntax().clone(); let token = pick_best(file.token_at_offset(position.offset))?; @@ -236,14 +236,7 @@ pub fn get_doc_url(db: &RootDatabase, position: &FilePosition) -> Option get_doc_link(db, &t), - Definition::Field(t) => get_doc_link(db, &t), - Definition::ModuleDef(t) => get_doc_link(db, &t), - Definition::SelfType(t) => get_doc_link(db, &t), - Definition::Local(t) => get_doc_link(db, &t), - Definition::TypeParam(t) => get_doc_link(db, &t), - } + get_doc_link(db, definition?) } /// Rewrites a markdown document, applying 'callback' to each link. diff --git a/crates/ide/src/lib.rs b/crates/ide/src/lib.rs index 0580d2979e70..5db6e1311fd4 100644 --- a/crates/ide/src/lib.rs +++ b/crates/ide/src/lib.rs @@ -387,7 +387,7 @@ impl Analysis { &self, position: FilePosition, ) -> Cancelable> { - self.with_db(|db| doc_links::get_doc_url(db, &position)) + self.with_db(|db| doc_links::external_docs(db, &position)) } /// Computes parameter information for the given call expression. diff --git a/crates/stdx/src/macros.rs b/crates/stdx/src/macros.rs index bf298460f3ae..f5ee3484b0ce 100644 --- a/crates/stdx/src/macros.rs +++ b/crates/stdx/src/macros.rs @@ -18,7 +18,13 @@ macro_rules! format_to { }; } -// Generates `From` impls for `Enum E { Foo(Foo), Bar(Bar) }` enums +/// Generates `From` impls for `Enum E { Foo(Foo), Bar(Bar) }` enums +/// +/// # Example +/// +/// ```rust +/// impl_from!(Struct, Union, Enum for Adt); +/// ``` #[macro_export] macro_rules! impl_from { ($($variant:ident $(($($sub_variant:ident),*))?),* for $enum:ident) => { diff --git a/editors/code/src/commands.ts b/editors/code/src/commands.ts index b22cd450b072..24c2e196dbf2 100644 --- a/editors/code/src/commands.ts +++ b/editors/code/src/commands.ts @@ -421,12 +421,10 @@ export function gotoLocation(ctx: Ctx): Cmd { export function openDocs(ctx: Ctx): Cmd { return async () => { - console.log("running openDocs"); const client = ctx.client; const editor = vscode.window.activeTextEditor; if (!editor || !client) { - console.log("not yet ready"); return }; @@ -435,7 +433,9 @@ export function openDocs(ctx: Ctx): Cmd { const doclink = await client.sendRequest(ra.openDocs, { position, textDocument }); - vscode.commands.executeCommand("vscode.open", vscode.Uri.parse(doclink.remote)); + if (doclink != null) { + vscode.commands.executeCommand("vscode.open", vscode.Uri.parse(doclink)); + } }; } diff --git a/editors/code/src/lsp_ext.ts b/editors/code/src/lsp_ext.ts index 56280471509d..fc8e120b3fc6 100644 --- a/editors/code/src/lsp_ext.ts +++ b/editors/code/src/lsp_ext.ts @@ -119,4 +119,4 @@ export interface CommandLinkGroup { commands: CommandLink[]; } -export const openDocs = new lc.RequestType('experimental/externalDocs'); +export const openDocs = new lc.RequestType('experimental/externalDocs'); From 62b76e7004bc215a375e41bd204b2eab5acdf9c2 Mon Sep 17 00:00:00 2001 From: Zac Pullar-Strecker Date: Tue, 1 Sep 2020 20:36:48 +1200 Subject: [PATCH 06/17] Document the protocol extension --- docs/dev/lsp-extensions.md | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/docs/dev/lsp-extensions.md b/docs/dev/lsp-extensions.md index f1160bb1cc94..173c04a142ab 100644 --- a/docs/dev/lsp-extensions.md +++ b/docs/dev/lsp-extensions.md @@ -303,7 +303,7 @@ SSR with query `foo($a, $b) ==>> ($a).foo($b)` will transform, eg `foo(y + 5, z) **Server Capability:** `{ "matchingBrace": boolean }` -This request is send from client to server to handle "Matching Brace" editor action. +This request is sent from client to server to handle "Matching Brace" editor action. **Method:** `experimental/matchingBrace` @@ -386,6 +386,17 @@ rust-analyzer supports only one `kind`, `"cargo"`. The `args` for `"cargo"` look } ``` +## Open External Documentation + +This request is send from client to server to get a URL to documentation for the symbol under the cursor, if available. + +**Method** `experimental/externalDocs` + +**Request:**: `TextDocumentPositionParams` + +**Response** `string | null` + + ## Analyzer Status **Method:** `rust-analyzer/analyzerStatus` From c648884397bfdb779c447fa31964dc1fce94bd95 Mon Sep 17 00:00:00 2001 From: Zac Pullar-Strecker Date: Thu, 3 Sep 2020 19:55:24 +1200 Subject: [PATCH 07/17] Differentiate method/tymethod by determining 'defaultness' Currently a method only has defaultness if it is a provided trait method, but this will change when specialisation is available and may need to become a concept known to hir. I opted to go for a 'fewest changes' approach given specialisation is still under development. --- crates/hir/src/code_model.rs | 9 ++++++++- crates/hir/src/lib.rs | 4 ++-- crates/hir_def/src/data.rs | 2 ++ crates/hir_def/src/item_tree.rs | 1 + crates/hir_def/src/item_tree/lower.rs | 3 +++ crates/ide/src/doc_links.rs | 19 ++++++++++++++----- editors/code/src/commands.ts | 2 +- 7 files changed, 31 insertions(+), 9 deletions(-) diff --git a/crates/hir/src/code_model.rs b/crates/hir/src/code_model.rs index 1dd6d73f3807..0b24f247c7af 100644 --- a/crates/hir/src/code_model.rs +++ b/crates/hir/src/code_model.rs @@ -772,7 +772,14 @@ impl Function { hir_ty::diagnostics::validate_body(db, self.id.into(), sink) } - pub fn parent_def(self, db: &dyn HirDatabase) -> Option { + /// Whether this function declaration has a definition. + /// + /// This is false in the case of required (not provided) trait methods. + pub fn has_body(self, db: &dyn HirDatabase) -> bool { + db.function_data(self.id).has_body + } + + pub fn method_owner(self, db: &dyn HirDatabase) -> Option { match self.as_assoc_item(db).map(|assoc| assoc.container(db)) { Some(AssocItemContainer::Trait(t)) => Some(t.into()), Some(AssocItemContainer::ImplDef(imp)) => { diff --git a/crates/hir/src/lib.rs b/crates/hir/src/lib.rs index 4094a76cbdba..687abe6ca182 100644 --- a/crates/hir/src/lib.rs +++ b/crates/hir/src/lib.rs @@ -35,8 +35,8 @@ pub use crate::{ code_model::{ Access, Adt, AsAssocItem, AssocItem, AssocItemContainer, Callable, CallableKind, Const, Crate, CrateDependency, DefWithBody, Enum, EnumVariant, Field, FieldSource, Function, - GenericDef, HasVisibility, ImplDef, Local, MacroDef, Module, ModuleDef, ScopeDef, Static, - Struct, Trait, Type, TypeAlias, TypeParam, Union, VariantDef, Visibility, + GenericDef, HasVisibility, ImplDef, Local, MacroDef, MethodOwner, Module, ModuleDef, + ScopeDef, Static, Struct, Trait, Type, TypeAlias, TypeParam, Union, VariantDef, Visibility, }, has_source::HasSource, semantics::{original_range, PathResolution, Semantics, SemanticsScope}, diff --git a/crates/hir_def/src/data.rs b/crates/hir_def/src/data.rs index 6190906da985..ff1ef0df64e5 100644 --- a/crates/hir_def/src/data.rs +++ b/crates/hir_def/src/data.rs @@ -25,6 +25,7 @@ pub struct FunctionData { /// True if the first param is `self`. This is relevant to decide whether this /// can be called as a method. pub has_self_param: bool, + pub has_body: bool, pub is_unsafe: bool, pub is_varargs: bool, pub visibility: RawVisibility, @@ -42,6 +43,7 @@ impl FunctionData { ret_type: func.ret_type.clone(), attrs: item_tree.attrs(ModItem::from(loc.id.value).into()).clone(), has_self_param: func.has_self_param, + has_body: func.has_body, is_unsafe: func.is_unsafe, is_varargs: func.is_varargs, visibility: item_tree[func.visibility].clone(), diff --git a/crates/hir_def/src/item_tree.rs b/crates/hir_def/src/item_tree.rs index 0fd91b9d0170..8a1121bbdfbc 100644 --- a/crates/hir_def/src/item_tree.rs +++ b/crates/hir_def/src/item_tree.rs @@ -505,6 +505,7 @@ pub struct Function { pub visibility: RawVisibilityId, pub generic_params: GenericParamsId, pub has_self_param: bool, + pub has_body: bool, pub is_unsafe: bool, pub params: Box<[TypeRef]>, pub is_varargs: bool, diff --git a/crates/hir_def/src/item_tree/lower.rs b/crates/hir_def/src/item_tree/lower.rs index 54814f141419..3328639cfe3f 100644 --- a/crates/hir_def/src/item_tree/lower.rs +++ b/crates/hir_def/src/item_tree/lower.rs @@ -330,12 +330,15 @@ impl Ctx { ret_type }; + let has_body = func.body().is_some(); + let ast_id = self.source_ast_id_map.ast_id(func); let mut res = Function { name, visibility, generic_params: GenericParamsId::EMPTY, has_self_param, + has_body, is_unsafe: func.unsafe_token().is_some(), params: params.into_boxed_slice(), is_varargs, diff --git a/crates/ide/src/doc_links.rs b/crates/ide/src/doc_links.rs index 512c42c4d169..2f6c59c4065f 100644 --- a/crates/ide/src/doc_links.rs +++ b/crates/ide/src/doc_links.rs @@ -13,7 +13,7 @@ use ide_db::{defs::Definition, RootDatabase}; use hir::{ db::{DefDatabase, HirDatabase}, - Adt, AsName, AssocItem, Crate, Field, HasAttrs, ItemInNs, ModuleDef, + Adt, AsName, AssocItem, Crate, Field, HasAttrs, ItemInNs, MethodOwner, ModuleDef, }; use ide_db::{ defs::{classify_name, classify_name_ref, Definition}, @@ -117,7 +117,7 @@ fn get_doc_link(db: &RootDatabase, definition: Definition) -> Option { let target_def: ModuleDef = match definition { Definition::ModuleDef(moddef) => match moddef { ModuleDef::Function(f) => { - f.parent_def(db).map(|mowner| mowner.into()).unwrap_or_else(|| f.clone().into()) + f.method_owner(db).map(|mowner| mowner.into()).unwrap_or_else(|| f.clone().into()) } moddef => moddef, }, @@ -401,9 +401,18 @@ fn get_symbol_fragment(db: &dyn HirDatabase, field_or_assoc: &FieldOrAssocItem) Some(match field_or_assoc { FieldOrAssocItem::Field(field) => format!("#structfield.{}", field.name(db)), FieldOrAssocItem::AssocItem(assoc) => match assoc { - // TODO: Rustdoc sometimes uses tymethod instead of method. This case needs to be investigated. - AssocItem::Function(function) => format!("#method.{}", function.name(db)), - // TODO: This might be the old method for documenting associated constants, i32::MAX uses a separate page... + AssocItem::Function(function) => { + let is_trait_method = + matches!(function.method_owner(db), Some(MethodOwner::Trait(..))); + // This distinction may get more complicated when specialisation is available. + // In particular this decision is made based on whether a method 'has defaultness'. + // Currently this is only the case for provided trait methods. + if is_trait_method && !function.has_body(db) { + format!("#tymethod.{}", function.name(db)) + } else { + format!("#method.{}", function.name(db)) + } + } AssocItem::Const(constant) => format!("#associatedconstant.{}", constant.name(db)?), AssocItem::TypeAlias(ty) => format!("#associatedtype.{}", ty.name(db)), }, diff --git a/editors/code/src/commands.ts b/editors/code/src/commands.ts index 24c2e196dbf2..1445e41d3cb0 100644 --- a/editors/code/src/commands.ts +++ b/editors/code/src/commands.ts @@ -425,7 +425,7 @@ export function openDocs(ctx: Ctx): Cmd { const client = ctx.client; const editor = vscode.window.activeTextEditor; if (!editor || !client) { - return + return; }; const position = editor.selection.active; From 26086faab2dab6baeaa050f73a7f64b83ead6807 Mon Sep 17 00:00:00 2001 From: Zac Pullar-Strecker Date: Thu, 3 Sep 2020 20:09:36 +1200 Subject: [PATCH 08/17] Change Option::Some bug to a fixme note IMO this is too much work to be worth fixing at the moment. --- crates/hir/src/code_model.rs | 1 + crates/ide/src/doc_links.rs | 6 +++--- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/crates/hir/src/code_model.rs b/crates/hir/src/code_model.rs index 0b24f247c7af..231ec000403d 100644 --- a/crates/hir/src/code_model.rs +++ b/crates/hir/src/code_model.rs @@ -779,6 +779,7 @@ impl Function { db.function_data(self.id).has_body } + /// If this function is a method, the trait or type where it is declared. pub fn method_owner(self, db: &dyn HirDatabase) -> Option { match self.as_assoc_item(db).map(|assoc| assoc.container(db)) { Some(AssocItemContainer::Trait(t)) => Some(t.into()), diff --git a/crates/ide/src/doc_links.rs b/crates/ide/src/doc_links.rs index 2f6c59c4065f..0608c2ac694a 100644 --- a/crates/ide/src/doc_links.rs +++ b/crates/ide/src/doc_links.rs @@ -103,8 +103,8 @@ pub fn get_doc_link(db: &dyn HirDatabase, definition: &T) get_doc_link_impl(db, &module_def) } -// TODO: -// BUG: For Option +// FIXME: +// BUG: For Option::Some // Returns https://doc.rust-lang.org/nightly/core/prelude/v1/enum.Option.html#variant.Some // Instead of https://doc.rust-lang.org/nightly/core/option/enum.Option.html // This could be worked around by turning the `EnumVariant` into `Enum` before attempting resolution, @@ -405,7 +405,7 @@ fn get_symbol_fragment(db: &dyn HirDatabase, field_or_assoc: &FieldOrAssocItem) let is_trait_method = matches!(function.method_owner(db), Some(MethodOwner::Trait(..))); // This distinction may get more complicated when specialisation is available. - // In particular this decision is made based on whether a method 'has defaultness'. + // Rustdoc makes this decision based on whether a method 'has defaultness'. // Currently this is only the case for provided trait methods. if is_trait_method && !function.has_body(db) { format!("#tymethod.{}", function.name(db)) From f1decfc1106fb25f5d36dfbd789370e5d59f251c Mon Sep 17 00:00:00 2001 From: Zac Pullar-Strecker Date: Thu, 3 Sep 2020 20:13:02 +1200 Subject: [PATCH 09/17] Fix send->sent typo --- docs/dev/lsp-extensions.md | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/docs/dev/lsp-extensions.md b/docs/dev/lsp-extensions.md index 173c04a142ab..3f861f3e00d3 100644 --- a/docs/dev/lsp-extensions.md +++ b/docs/dev/lsp-extensions.md @@ -129,7 +129,7 @@ As a result of the command call the client will get the respective workspace edi **Server Capability:** `{ "parentModule": boolean }` -This request is send from client to server to handle "Goto Parent Module" editor action. +This request is sent from client to server to handle "Goto Parent Module" editor action. **Method:** `experimental/parentModule` @@ -163,7 +163,7 @@ mod foo; **Server Capability:** `{ "joinLines": boolean }` -This request is send from client to server to handle "Join Lines" editor action. +This request is sent from client to server to handle "Join Lines" editor action. **Method:** `experimental/joinLines` @@ -210,7 +210,7 @@ fn main() { **Server Capability:** `{ "onEnter": boolean }` -This request is send from client to server to handle Enter keypress. +This request is sent from client to server to handle Enter keypress. **Method:** `experimental/onEnter` @@ -261,7 +261,7 @@ As proper cursor positioning is raison-d'etat for `onEnter`, it uses `SnippetTex **Server Capability:** `{ "ssr": boolean }` -This request is send from client to server to handle structural search replace -- automated syntax tree based transformation of the source. +This request is sent from client to server to handle structural search replace -- automated syntax tree based transformation of the source. **Method:** `experimental/ssr` @@ -348,7 +348,7 @@ Moreover, it would be cool if editors didn't need to implement even basic langua **Server Capability:** `{ "runnables": { "kinds": string[] } }` -This request is send from client to server to get the list of things that can be run (tests, binaries, `cargo check -p`). +This request is sent from client to server to get the list of things that can be run (tests, binaries, `cargo check -p`). **Method:** `experimental/runnables` @@ -388,7 +388,7 @@ rust-analyzer supports only one `kind`, `"cargo"`. The `args` for `"cargo"` look ## Open External Documentation -This request is send from client to server to get a URL to documentation for the symbol under the cursor, if available. +This request is sent from client to server to get a URL to documentation for the symbol under the cursor, if available. **Method** `experimental/externalDocs` @@ -488,7 +488,7 @@ Expands macro call at a given position. **Method:** `rust-analyzer/inlayHints` -This request is send from client to server to render "inlay hints" -- virtual text inserted into editor to show things like inferred types. +This request is sent from client to server to render "inlay hints" -- virtual text inserted into editor to show things like inferred types. Generally, the client should re-query inlay hints after every modification. Note that we plan to move this request to `experimental/inlayHints`, as it is not really Rust-specific, but the current API is not necessary the right one. Upstream issue: https://github.com/microsoft/language-server-protocol/issues/956 From ec75d8bd75b2b54138d6a46d0a800c92ac07cfe5 Mon Sep 17 00:00:00 2001 From: Zac Pullar-Strecker Date: Thu, 3 Sep 2020 20:27:59 +1200 Subject: [PATCH 10/17] Update tests for new function field --- crates/hir_def/src/item_tree/tests.rs | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/crates/hir_def/src/item_tree/tests.rs b/crates/hir_def/src/item_tree/tests.rs index 1a806cda52c3..4b354c4c145e 100644 --- a/crates/hir_def/src/item_tree/tests.rs +++ b/crates/hir_def/src/item_tree/tests.rs @@ -240,9 +240,9 @@ fn smoke() { > #[Attrs { entries: Some([Attr { path: ModPath { kind: Plain, segments: [Name(Text("assoc_const"))] }, input: None }]) }] > Const { name: Some(Name(Text("CONST"))), visibility: RawVisibilityId("pub(self)"), type_ref: Path(Path { type_anchor: None, mod_path: ModPath { kind: Plain, segments: [Name(Text("u8"))] }, generic_args: [None] }), ast_id: FileAstId::(9) } > #[Attrs { entries: Some([Attr { path: ModPath { kind: Plain, segments: [Name(Text("assoc_method"))] }, input: None }]) }] - > Function { name: Name(Text("method")), visibility: RawVisibilityId("pub(self)"), generic_params: GenericParamsId(4294967295), has_self_param: true, is_unsafe: false, params: [Reference(Path(Path { type_anchor: None, mod_path: ModPath { kind: Plain, segments: [Name(Text("Self"))] }, generic_args: [None] }), Shared)], is_varargs: false, ret_type: Tuple([]), ast_id: FileAstId::(10) } + > Function { name: Name(Text("method")), visibility: RawVisibilityId("pub(self)"), generic_params: GenericParamsId(4294967295), has_self_param: true, has_body: false, is_unsafe: false, params: [Reference(Path(Path { type_anchor: None, mod_path: ModPath { kind: Plain, segments: [Name(Text("Self"))] }, generic_args: [None] }), Shared)], is_varargs: false, ret_type: Tuple([]), ast_id: FileAstId::(10) } > #[Attrs { entries: Some([Attr { path: ModPath { kind: Plain, segments: [Name(Text("assoc_dfl_method"))] }, input: None }]) }] - > Function { name: Name(Text("dfl_method")), visibility: RawVisibilityId("pub(self)"), generic_params: GenericParamsId(4294967295), has_self_param: true, is_unsafe: false, params: [Reference(Path(Path { type_anchor: None, mod_path: ModPath { kind: Plain, segments: [Name(Text("Self"))] }, generic_args: [None] }), Mut)], is_varargs: false, ret_type: Tuple([]), ast_id: FileAstId::(11) } + > Function { name: Name(Text("dfl_method")), visibility: RawVisibilityId("pub(self)"), generic_params: GenericParamsId(4294967295), has_self_param: true, has_body: true, is_unsafe: false, params: [Reference(Path(Path { type_anchor: None, mod_path: ModPath { kind: Plain, segments: [Name(Text("Self"))] }, generic_args: [None] }), Mut)], is_varargs: false, ret_type: Tuple([]), ast_id: FileAstId::(11) } #[Attrs { entries: Some([Attr { path: ModPath { kind: Plain, segments: [Name(Text("struct0"))] }, input: None }]) }] Struct { name: Name(Text("Struct0")), visibility: RawVisibilityId("pub(self)"), generic_params: GenericParamsId(1), fields: Unit, ast_id: FileAstId::(3), kind: Unit } #[Attrs { entries: Some([Attr { path: ModPath { kind: Plain, segments: [Name(Text("struct1"))] }, input: None }]) }] @@ -275,12 +275,12 @@ fn simple_inner_items() { top-level items: Impl { generic_params: GenericParamsId(0), target_trait: Some(Path(Path { type_anchor: None, mod_path: ModPath { kind: Plain, segments: [Name(Text("D"))] }, generic_args: [None] })), target_type: Path(Path { type_anchor: None, mod_path: ModPath { kind: Plain, segments: [Name(Text("Response"))] }, generic_args: [Some(GenericArgs { args: [Type(Path(Path { type_anchor: None, mod_path: ModPath { kind: Plain, segments: [Name(Text("T"))] }, generic_args: [None] }))], has_self_type: false, bindings: [] })] }), is_negative: false, items: [Function(Idx::(1))], ast_id: FileAstId::(0) } - > Function { name: Name(Text("foo")), visibility: RawVisibilityId("pub(self)"), generic_params: GenericParamsId(4294967295), has_self_param: false, is_unsafe: false, params: [], is_varargs: false, ret_type: Tuple([]), ast_id: FileAstId::(1) } + > Function { name: Name(Text("foo")), visibility: RawVisibilityId("pub(self)"), generic_params: GenericParamsId(4294967295), has_self_param: false, has_body: true, is_unsafe: false, params: [], is_varargs: false, ret_type: Tuple([]), ast_id: FileAstId::(1) } inner items: for AST FileAstId::(2): - Function { name: Name(Text("end")), visibility: RawVisibilityId("pub(self)"), generic_params: GenericParamsId(1), has_self_param: false, is_unsafe: false, params: [], is_varargs: false, ret_type: Tuple([]), ast_id: FileAstId::(2) } + Function { name: Name(Text("end")), visibility: RawVisibilityId("pub(self)"), generic_params: GenericParamsId(1), has_self_param: false, has_body: true, is_unsafe: false, params: [], is_varargs: false, ret_type: Tuple([]), ast_id: FileAstId::(2) } "#]], ); @@ -303,9 +303,9 @@ fn extern_attrs() { top-level items: #[Attrs { entries: Some([Attr { path: ModPath { kind: Plain, segments: [Name(Text("attr_a"))] }, input: None }, Attr { path: ModPath { kind: Plain, segments: [Name(Text("block_attr"))] }, input: None }]) }] - Function { name: Name(Text("a")), visibility: RawVisibilityId("pub(self)"), generic_params: GenericParamsId(4294967295), has_self_param: false, is_unsafe: true, params: [], is_varargs: false, ret_type: Tuple([]), ast_id: FileAstId::(1) } + Function { name: Name(Text("a")), visibility: RawVisibilityId("pub(self)"), generic_params: GenericParamsId(4294967295), has_self_param: false, has_body: true, is_unsafe: true, params: [], is_varargs: false, ret_type: Tuple([]), ast_id: FileAstId::(1) } #[Attrs { entries: Some([Attr { path: ModPath { kind: Plain, segments: [Name(Text("attr_b"))] }, input: None }, Attr { path: ModPath { kind: Plain, segments: [Name(Text("block_attr"))] }, input: None }]) }] - Function { name: Name(Text("b")), visibility: RawVisibilityId("pub(self)"), generic_params: GenericParamsId(4294967295), has_self_param: false, is_unsafe: true, params: [], is_varargs: false, ret_type: Tuple([]), ast_id: FileAstId::(2) } + Function { name: Name(Text("b")), visibility: RawVisibilityId("pub(self)"), generic_params: GenericParamsId(4294967295), has_self_param: false, has_body: true, is_unsafe: true, params: [], is_varargs: false, ret_type: Tuple([]), ast_id: FileAstId::(2) } "##]], ); } @@ -329,9 +329,9 @@ fn trait_attrs() { #[Attrs { entries: Some([Attr { path: ModPath { kind: Plain, segments: [Name(Text("trait_attr"))] }, input: None }]) }] Trait { name: Name(Text("Tr")), visibility: RawVisibilityId("pub(self)"), generic_params: GenericParamsId(0), auto: false, items: [Function(Idx::(0)), Function(Idx::(1))], ast_id: FileAstId::(0) } > #[Attrs { entries: Some([Attr { path: ModPath { kind: Plain, segments: [Name(Text("attr_a"))] }, input: None }]) }] - > Function { name: Name(Text("a")), visibility: RawVisibilityId("pub(self)"), generic_params: GenericParamsId(4294967295), has_self_param: false, is_unsafe: false, params: [], is_varargs: false, ret_type: Tuple([]), ast_id: FileAstId::(1) } + > Function { name: Name(Text("a")), visibility: RawVisibilityId("pub(self)"), generic_params: GenericParamsId(4294967295), has_self_param: false, has_body: true, is_unsafe: false, params: [], is_varargs: false, ret_type: Tuple([]), ast_id: FileAstId::(1) } > #[Attrs { entries: Some([Attr { path: ModPath { kind: Plain, segments: [Name(Text("attr_b"))] }, input: None }]) }] - > Function { name: Name(Text("b")), visibility: RawVisibilityId("pub(self)"), generic_params: GenericParamsId(4294967295), has_self_param: false, is_unsafe: false, params: [], is_varargs: false, ret_type: Tuple([]), ast_id: FileAstId::(2) } + > Function { name: Name(Text("b")), visibility: RawVisibilityId("pub(self)"), generic_params: GenericParamsId(4294967295), has_self_param: false, has_body: true, is_unsafe: false, params: [], is_varargs: false, ret_type: Tuple([]), ast_id: FileAstId::(2) } "##]], ); } @@ -355,9 +355,9 @@ fn impl_attrs() { #[Attrs { entries: Some([Attr { path: ModPath { kind: Plain, segments: [Name(Text("impl_attr"))] }, input: None }]) }] Impl { generic_params: GenericParamsId(4294967295), target_trait: None, target_type: Path(Path { type_anchor: None, mod_path: ModPath { kind: Plain, segments: [Name(Text("Ty"))] }, generic_args: [None] }), is_negative: false, items: [Function(Idx::(0)), Function(Idx::(1))], ast_id: FileAstId::(0) } > #[Attrs { entries: Some([Attr { path: ModPath { kind: Plain, segments: [Name(Text("attr_a"))] }, input: None }]) }] - > Function { name: Name(Text("a")), visibility: RawVisibilityId("pub(self)"), generic_params: GenericParamsId(4294967295), has_self_param: false, is_unsafe: false, params: [], is_varargs: false, ret_type: Tuple([]), ast_id: FileAstId::(1) } + > Function { name: Name(Text("a")), visibility: RawVisibilityId("pub(self)"), generic_params: GenericParamsId(4294967295), has_self_param: false, has_body: true, is_unsafe: false, params: [], is_varargs: false, ret_type: Tuple([]), ast_id: FileAstId::(1) } > #[Attrs { entries: Some([Attr { path: ModPath { kind: Plain, segments: [Name(Text("attr_b"))] }, input: None }]) }] - > Function { name: Name(Text("b")), visibility: RawVisibilityId("pub(self)"), generic_params: GenericParamsId(4294967295), has_self_param: false, is_unsafe: false, params: [], is_varargs: false, ret_type: Tuple([]), ast_id: FileAstId::(2) } + > Function { name: Name(Text("b")), visibility: RawVisibilityId("pub(self)"), generic_params: GenericParamsId(4294967295), has_self_param: false, has_body: true, is_unsafe: false, params: [], is_varargs: false, ret_type: Tuple([]), ast_id: FileAstId::(2) } "##]], ); } @@ -408,13 +408,13 @@ fn inner_item_attrs() { inner attrs: Attrs { entries: None } top-level items: - Function { name: Name(Text("foo")), visibility: RawVisibilityId("pub(self)"), generic_params: GenericParamsId(4294967295), has_self_param: false, is_unsafe: false, params: [], is_varargs: false, ret_type: Tuple([]), ast_id: FileAstId::(0) } + Function { name: Name(Text("foo")), visibility: RawVisibilityId("pub(self)"), generic_params: GenericParamsId(4294967295), has_self_param: false, has_body: true, is_unsafe: false, params: [], is_varargs: false, ret_type: Tuple([]), ast_id: FileAstId::(0) } inner items: for AST FileAstId::(1): #[Attrs { entries: Some([Attr { path: ModPath { kind: Plain, segments: [Name(Text("on_inner"))] }, input: None }]) }] - Function { name: Name(Text("inner")), visibility: RawVisibilityId("pub(self)"), generic_params: GenericParamsId(4294967295), has_self_param: false, is_unsafe: false, params: [], is_varargs: false, ret_type: Tuple([]), ast_id: FileAstId::(1) } + Function { name: Name(Text("inner")), visibility: RawVisibilityId("pub(self)"), generic_params: GenericParamsId(4294967295), has_self_param: false, has_body: true, is_unsafe: false, params: [], is_varargs: false, ret_type: Tuple([]), ast_id: FileAstId::(1) } "##]], ); From 37a4d060a7709da42a67fe1631d5786f7e106884 Mon Sep 17 00:00:00 2001 From: Zac Pullar-Strecker Date: Thu, 3 Sep 2020 21:27:46 +1200 Subject: [PATCH 11/17] Add tests --- crates/ide/src/doc_links.rs | 96 ++++++++++++++++++++++++++++++++++++- 1 file changed, 94 insertions(+), 2 deletions(-) diff --git a/crates/ide/src/doc_links.rs b/crates/ide/src/doc_links.rs index 0608c2ac694a..b9d53bcd49ac 100644 --- a/crates/ide/src/doc_links.rs +++ b/crates/ide/src/doc_links.rs @@ -112,6 +112,7 @@ pub fn get_doc_link(db: &dyn HirDatabase, definition: &T) // version of import map which follows the same process as rustdoc. Otherwise there'll always be some // edge cases where we select the wrong import path. fn get_doc_link(db: &RootDatabase, definition: Definition) -> Option { + eprintln!("enter"); // Get the outermost definition for the moduledef. This is used to resolve the public path to the type, // then we can join the method, field, etc onto it if required. let target_def: ModuleDef = match definition { @@ -131,8 +132,8 @@ fn get_doc_link(db: &RootDatabase, definition: Definition) -> Option { let module = definition.module(db)?; let krate = module.krate(); let import_map = db.import_map(krate.into()); - let base = once(krate.display_name(db).unwrap()) - .chain(import_map.path_of(ns).unwrap().segments.iter().map(|name| format!("{}", name))) + let base = once(krate.display_name(db)?) + .chain(import_map.path_of(ns)?.segments.iter().map(|name| format!("{}", name))) .join("/"); let filename = get_symbol_filename(db, &target_def); @@ -152,6 +153,7 @@ fn get_doc_link(db: &RootDatabase, definition: Definition) -> Option { Definition::Field(field) => get_symbol_fragment(db, &FieldOrAssocItem::Field(field)), _ => None, }; + eprintln!("end-ish"); get_doc_url(db, &krate) .and_then(|url| url.join(&base).ok()) @@ -430,3 +432,93 @@ fn pick_best(tokens: TokenAtOffset) -> Option { } } } + +#[cfg(test)] +mod tests { + use expect_test::{expect, Expect}; + + use crate::mock_analysis::analysis_and_position; + + fn check(ra_fixture: &str, expect: Expect) { + let (analysis, position) = analysis_and_position(ra_fixture); + let url = analysis.external_docs(position).unwrap().unwrap(); + + expect.assert_eq(&url) + } + + #[test] + fn test_doc_url_struct() { + check( + r#" +pub struct Fo<|>o; +"#, + expect![[r#"https://docs.rs/test/*/test/struct.Foo.html"#]], + ); + } + + // TODO: Fix this test. Fails on `import_map.path_of(ns)` + #[test] + fn test_doc_url_fn() { + check( + r#" +pub fn fo<|>o() {} +"#, + expect![[r#""#]], + ); + } + + #[test] + fn test_doc_url_inherent_method() { + check( + r#" +pub struct Foo; + +impl Foo { + pub fn met<|>hod() {} +} + +"#, + expect![[r##"https://docs.rs/test/*/test/struct.Foo.html#method.method"##]], + ); + } + + #[test] + fn test_doc_url_trait_provided_method() { + check( + r#" +pub trait Bar { + fn met<|>hod() {} +} + +"#, + expect![[r##"https://docs.rs/test/*/test/trait.Bar.html#method.method"##]], + ); + } + + #[test] + fn test_doc_url_trait_required_method() { + check( + r#" +pub trait Foo { + fn met<|>hod(); +} + +"#, + expect![[r##"https://docs.rs/test/*/test/trait.Foo.html#tymethod.method"##]], + ); + } + + + #[test] + fn test_doc_url_field() { + check( + r#" +pub struct Foo { + pub fie<|>ld: () +} + +"#, + expect![[r##"https://docs.rs/test/*/test/struct.Foo.html#structfield.field"##]], + ); + } +} From 6cae6b8f3cf284f9f386ea8ce7e347043fe9a040 Mon Sep 17 00:00:00 2001 From: Zac Pullar-Strecker Date: Sat, 5 Sep 2020 16:58:04 +1200 Subject: [PATCH 12/17] Fix namespace detection & function test --- crates/ide/src/doc_links.rs | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/crates/ide/src/doc_links.rs b/crates/ide/src/doc_links.rs index b9d53bcd49ac..1014660146e7 100644 --- a/crates/ide/src/doc_links.rs +++ b/crates/ide/src/doc_links.rs @@ -112,7 +112,6 @@ pub fn get_doc_link(db: &dyn HirDatabase, definition: &T) // version of import map which follows the same process as rustdoc. Otherwise there'll always be some // edge cases where we select the wrong import path. fn get_doc_link(db: &RootDatabase, definition: Definition) -> Option { - eprintln!("enter"); // Get the outermost definition for the moduledef. This is used to resolve the public path to the type, // then we can join the method, field, etc onto it if required. let target_def: ModuleDef = match definition { @@ -127,7 +126,7 @@ fn get_doc_link(db: &RootDatabase, definition: Definition) -> Option { _ => return None, }; - let ns = ItemInNs::Types(target_def.clone().into()); + let ns = ItemInNs::from(target_def.clone()); let module = definition.module(db)?; let krate = module.krate(); @@ -153,7 +152,6 @@ fn get_doc_link(db: &RootDatabase, definition: Definition) -> Option { Definition::Field(field) => get_symbol_fragment(db, &FieldOrAssocItem::Field(field)), _ => None, }; - eprintln!("end-ish"); get_doc_url(db, &krate) .and_then(|url| url.join(&base).ok()) @@ -456,14 +454,13 @@ pub struct Fo<|>o; ); } - // TODO: Fix this test. Fails on `import_map.path_of(ns)` #[test] fn test_doc_url_fn() { check( r#" pub fn fo<|>o() {} "#, - expect![[r#""#]], + expect![[r##"https://docs.rs/test/*/test/fn.foo.html#method.foo"##]], ); } @@ -508,7 +505,6 @@ pub trait Foo { ); } - #[test] fn test_doc_url_field() { check( From f6759ba3bbc2c17771735327238c4ed936d5cdc9 Mon Sep 17 00:00:00 2001 From: Zac Pullar-Strecker Date: Sat, 5 Sep 2020 17:21:52 +1200 Subject: [PATCH 13/17] Add ignored test to demonstrate ImportMap bug --- crates/ide/src/doc_links.rs | 27 ++++++++++++++++++++++++++- 1 file changed, 26 insertions(+), 1 deletion(-) diff --git a/crates/ide/src/doc_links.rs b/crates/ide/src/doc_links.rs index 1014660146e7..164783e14958 100644 --- a/crates/ide/src/doc_links.rs +++ b/crates/ide/src/doc_links.rs @@ -439,7 +439,7 @@ mod tests { fn check(ra_fixture: &str, expect: Expect) { let (analysis, position) = analysis_and_position(ra_fixture); - let url = analysis.external_docs(position).unwrap().unwrap(); + let url = analysis.external_docs(position).unwrap().expect("could not find url for symbol"); expect.assert_eq(&url) } @@ -517,4 +517,29 @@ pub struct Foo { expect![[r##"https://docs.rs/test/*/test/struct.Foo.html#structfield.field"##]], ); } + + // FIXME: ImportMap will return re-export paths instead of public module + // paths. The correct path to documentation will never be a re-export. + // This problem stops us from resolving stdlib items included in the prelude + // such as `Option::Some` correctly. + #[ignore = "ImportMap may return re-exports"] + #[test] + fn test_reexport_order() { + check( + r#" +pub mod wrapper { + pub use module::Item; + + pub mod module { + pub struct Item; + } +} + +fn foo() { + let bar: wrapper::It<|>em; +} + "#, + expect![[r#"https://docs.rs/test/*/test/wrapper/module/struct.Item.html"#]], + ) + } } From e4a787fcbc865cd66921f8052a9cccad33a13b92 Mon Sep 17 00:00:00 2001 From: Zac Pullar-Strecker Date: Sat, 5 Sep 2020 17:37:27 +1200 Subject: [PATCH 14/17] Remove outdated part of doc_links module docs --- crates/ide/src/doc_links.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/crates/ide/src/doc_links.rs b/crates/ide/src/doc_links.rs index 164783e14958..34dc122a8012 100644 --- a/crates/ide/src/doc_links.rs +++ b/crates/ide/src/doc_links.rs @@ -1,6 +1,4 @@ //! Resolves and rewrites links in markdown documentation. -//! -//! Most of the implementation can be found in [`hir::doc_links`]. use std::iter::once; From d2c68809ea3243ca68811e9b2e0f1592e2dc33fe Mon Sep 17 00:00:00 2001 From: Zac Pullar-Strecker Date: Thu, 8 Oct 2020 14:54:44 +1300 Subject: [PATCH 15/17] Changes from review --- crates/ide/src/doc_links.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/crates/ide/src/doc_links.rs b/crates/ide/src/doc_links.rs index 34dc122a8012..6b2ecf725119 100644 --- a/crates/ide/src/doc_links.rs +++ b/crates/ide/src/doc_links.rs @@ -11,7 +11,7 @@ use ide_db::{defs::Definition, RootDatabase}; use hir::{ db::{DefDatabase, HirDatabase}, - Adt, AsName, AssocItem, Crate, Field, HasAttrs, ItemInNs, MethodOwner, ModuleDef, + Adt, AsName, AssocItem, Crate, Field, HasAttrs, ItemInNs, MethodOwner, ModuleDef, AssocItemContainer, AsAssocItem }; use ide_db::{ defs::{classify_name, classify_name_ref, Definition}, @@ -219,7 +219,7 @@ fn rewrite_url_link(db: &RootDatabase, def: ModuleDef, target: &str) -> Option Option { +pub(crate) fn external_docs(db: &RootDatabase, position: &FilePosition) -> Option { let sema = Semantics::new(db); let file = sema.parse(position.file_id).syntax().clone(); let token = pick_best(file.token_at_offset(position.offset))?; @@ -401,7 +401,7 @@ fn get_symbol_fragment(db: &dyn HirDatabase, field_or_assoc: &FieldOrAssocItem) FieldOrAssocItem::AssocItem(assoc) => match assoc { AssocItem::Function(function) => { let is_trait_method = - matches!(function.method_owner(db), Some(MethodOwner::Trait(..))); + matches!(function.as_assoc_item(db).map(|assoc| assoc.container(db)), Some(AssocItemContainer::Trait(..))); // This distinction may get more complicated when specialisation is available. // Rustdoc makes this decision based on whether a method 'has defaultness'. // Currently this is only the case for provided trait methods. From 8af1dd733760c51abadda8f2bd20139e11ebba04 Mon Sep 17 00:00:00 2001 From: Zac Pullar-Strecker Date: Thu, 8 Oct 2020 15:22:57 +1300 Subject: [PATCH 16/17] Rebase fixes --- crates/hir/src/code_model.rs | 2 +- crates/ide/src/doc_links.rs | 18 +++++------------- 2 files changed, 6 insertions(+), 14 deletions(-) diff --git a/crates/hir/src/code_model.rs b/crates/hir/src/code_model.rs index 231ec000403d..9989b9b56f68 100644 --- a/crates/hir/src/code_model.rs +++ b/crates/hir/src/code_model.rs @@ -35,7 +35,7 @@ use hir_ty::{ traits::SolutionVariables, ApplicationTy, BoundVar, CallableDefId, Canonical, DebruijnIndex, FnSig, GenericPredicate, InEnvironment, Obligation, ProjectionPredicate, ProjectionTy, Substs, TraitEnvironment, Ty, - TyDefId, TyKind, TypeCtor, TyLoweringContext, TypeCtor, + TyDefId, TyKind, TypeCtor, TyLoweringContext, }; use rustc_hash::FxHashSet; use stdx::impl_from; diff --git a/crates/ide/src/doc_links.rs b/crates/ide/src/doc_links.rs index 6b2ecf725119..5ef708647954 100644 --- a/crates/ide/src/doc_links.rs +++ b/crates/ide/src/doc_links.rs @@ -7,11 +7,9 @@ use pulldown_cmark_to_cmark::{cmark_with_options, Options as CmarkOptions}; use pulldown_cmark::{CowStr, Event, LinkType, Options, Parser, Tag}; use url::Url; -use ide_db::{defs::Definition, RootDatabase}; - use hir::{ db::{DefDatabase, HirDatabase}, - Adt, AsName, AssocItem, Crate, Field, HasAttrs, ItemInNs, MethodOwner, ModuleDef, AssocItemContainer, AsAssocItem + Adt, AsName, AssocItem, Crate, Field, HasAttrs, ItemInNs, ModuleDef, AssocItemContainer, AsAssocItem }; use ide_db::{ defs::{classify_name, classify_name_ref, Definition}, @@ -95,12 +93,6 @@ pub fn remove_links(markdown: &str) -> String { out } -pub fn get_doc_link(db: &dyn HirDatabase, definition: &T) -> Option { - let module_def = definition.clone().try_into_module_def()?; - - get_doc_link_impl(db, &module_def) -} - // FIXME: // BUG: For Option::Some // Returns https://doc.rust-lang.org/nightly/core/prelude/v1/enum.Option.html#variant.Some @@ -129,8 +121,8 @@ fn get_doc_link(db: &RootDatabase, definition: Definition) -> Option { let module = definition.module(db)?; let krate = module.krate(); let import_map = db.import_map(krate.into()); - let base = once(krate.display_name(db)?) - .chain(import_map.path_of(ns)?.segments.iter().map(|name| format!("{}", name))) + let base = once(krate.declaration_name(db)?.to_string()) + .chain(import_map.path_of(ns)?.segments.iter().map(|name| name.to_string())) .join("/"); let filename = get_symbol_filename(db, &target_def); @@ -433,10 +425,10 @@ fn pick_best(tokens: TokenAtOffset) -> Option { mod tests { use expect_test::{expect, Expect}; - use crate::mock_analysis::analysis_and_position; + use crate::fixture; fn check(ra_fixture: &str, expect: Expect) { - let (analysis, position) = analysis_and_position(ra_fixture); + let (analysis, position) = fixture::position(ra_fixture); let url = analysis.external_docs(position).unwrap().expect("could not find url for symbol"); expect.assert_eq(&url) From 3bd4fe96dce17eb2bff380389b24ea325bf54803 Mon Sep 17 00:00:00 2001 From: Zac Pullar-Strecker Date: Thu, 8 Oct 2020 15:44:52 +1300 Subject: [PATCH 17/17] Remove methodowner & fix formatting --- crates/hir/src/code_model.rs | 39 +----------------------------------- crates/hir/src/lib.rs | 4 ++-- crates/ide/src/doc_links.rs | 35 +++++++++++++++++++++----------- crates/ide/src/hover.rs | 2 +- 4 files changed, 27 insertions(+), 53 deletions(-) diff --git a/crates/hir/src/code_model.rs b/crates/hir/src/code_model.rs index 9989b9b56f68..650b4fa40cec 100644 --- a/crates/hir/src/code_model.rs +++ b/crates/hir/src/code_model.rs @@ -35,7 +35,7 @@ use hir_ty::{ traits::SolutionVariables, ApplicationTy, BoundVar, CallableDefId, Canonical, DebruijnIndex, FnSig, GenericPredicate, InEnvironment, Obligation, ProjectionPredicate, ProjectionTy, Substs, TraitEnvironment, Ty, - TyDefId, TyKind, TypeCtor, TyLoweringContext, + TyDefId, TyKind, TypeCtor, }; use rustc_hash::FxHashSet; use stdx::impl_from; @@ -186,15 +186,6 @@ impl_from!( for ModuleDef ); -impl From for ModuleDef { - fn from(mowner: MethodOwner) -> Self { - match mowner { - MethodOwner::Trait(t) => t.into(), - MethodOwner::Adt(t) => t.into(), - } - } -} - impl From for ModuleDef { fn from(var: VariantDef) -> Self { match var { @@ -778,36 +769,8 @@ impl Function { pub fn has_body(self, db: &dyn HirDatabase) -> bool { db.function_data(self.id).has_body } - - /// If this function is a method, the trait or type where it is declared. - pub fn method_owner(self, db: &dyn HirDatabase) -> Option { - match self.as_assoc_item(db).map(|assoc| assoc.container(db)) { - Some(AssocItemContainer::Trait(t)) => Some(t.into()), - Some(AssocItemContainer::ImplDef(imp)) => { - let resolver = ModuleId::from(imp.module(db)).resolver(db.upcast()); - let ctx = TyLoweringContext::new(db, &resolver); - let adt = Ty::from_hir( - &ctx, - &imp.target_trait(db).unwrap_or_else(|| imp.target_type(db)), - ) - .as_adt() - .map(|t| t.0) - .unwrap(); - Some(Adt::from(adt).into()) - } - None => None, - } - } } -#[derive(Debug)] -pub enum MethodOwner { - Trait(Trait), - Adt(Adt), -} - -impl_from!(Trait, Adt for MethodOwner); - // Note: logically, this belongs to `hir_ty`, but we are not using it there yet. pub enum Access { Shared, diff --git a/crates/hir/src/lib.rs b/crates/hir/src/lib.rs index 687abe6ca182..4094a76cbdba 100644 --- a/crates/hir/src/lib.rs +++ b/crates/hir/src/lib.rs @@ -35,8 +35,8 @@ pub use crate::{ code_model::{ Access, Adt, AsAssocItem, AssocItem, AssocItemContainer, Callable, CallableKind, Const, Crate, CrateDependency, DefWithBody, Enum, EnumVariant, Field, FieldSource, Function, - GenericDef, HasVisibility, ImplDef, Local, MacroDef, MethodOwner, Module, ModuleDef, - ScopeDef, Static, Struct, Trait, Type, TypeAlias, TypeParam, Union, VariantDef, Visibility, + GenericDef, HasVisibility, ImplDef, Local, MacroDef, Module, ModuleDef, ScopeDef, Static, + Struct, Trait, Type, TypeAlias, TypeParam, Union, VariantDef, Visibility, }, has_source::HasSource, semantics::{original_range, PathResolution, Semantics, SemanticsScope}, diff --git a/crates/ide/src/doc_links.rs b/crates/ide/src/doc_links.rs index 5ef708647954..06af36b73e1c 100644 --- a/crates/ide/src/doc_links.rs +++ b/crates/ide/src/doc_links.rs @@ -3,13 +3,14 @@ use std::iter::once; use itertools::Itertools; -use pulldown_cmark_to_cmark::{cmark_with_options, Options as CmarkOptions}; use pulldown_cmark::{CowStr, Event, LinkType, Options, Parser, Tag}; +use pulldown_cmark_to_cmark::{cmark_with_options, Options as CmarkOptions}; use url::Url; use hir::{ db::{DefDatabase, HirDatabase}, - Adt, AsName, AssocItem, Crate, Field, HasAttrs, ItemInNs, ModuleDef, AssocItemContainer, AsAssocItem + Adt, AsAssocItem, AsName, AssocItem, AssocItemContainer, Crate, Field, HasAttrs, ItemInNs, + ModuleDef, }; use ide_db::{ defs::{classify_name, classify_name_ref, Definition}, @@ -97,18 +98,23 @@ pub fn remove_links(markdown: &str) -> String { // BUG: For Option::Some // Returns https://doc.rust-lang.org/nightly/core/prelude/v1/enum.Option.html#variant.Some // Instead of https://doc.rust-lang.org/nightly/core/option/enum.Option.html -// This could be worked around by turning the `EnumVariant` into `Enum` before attempting resolution, -// but it's really just working around the problem. Ideally we need to implement a slightly different -// version of import map which follows the same process as rustdoc. Otherwise there'll always be some -// edge cases where we select the wrong import path. +// +// This should cease to be a problem if RFC2988 (Stable Rustdoc URLs) is implemented +// https://github.com/rust-lang/rfcs/pull/2988 fn get_doc_link(db: &RootDatabase, definition: Definition) -> Option { // Get the outermost definition for the moduledef. This is used to resolve the public path to the type, // then we can join the method, field, etc onto it if required. let target_def: ModuleDef = match definition { Definition::ModuleDef(moddef) => match moddef { - ModuleDef::Function(f) => { - f.method_owner(db).map(|mowner| mowner.into()).unwrap_or_else(|| f.clone().into()) - } + ModuleDef::Function(f) => f + .as_assoc_item(db) + .and_then(|assoc| match assoc.container(db) { + AssocItemContainer::Trait(t) => Some(t.into()), + AssocItemContainer::ImplDef(impld) => { + impld.target_ty(db).as_adt().map(|adt| adt.into()) + } + }) + .unwrap_or_else(|| f.clone().into()), moddef => moddef, }, Definition::Field(f) => f.parent_def(db).into(), @@ -211,7 +217,10 @@ fn rewrite_url_link(db: &RootDatabase, def: ModuleDef, target: &str) -> Option Option { +pub(crate) fn external_docs( + db: &RootDatabase, + position: &FilePosition, +) -> Option { let sema = Semantics::new(db); let file = sema.parse(position.file_id).syntax().clone(); let token = pick_best(file.token_at_offset(position.offset))?; @@ -392,8 +401,10 @@ fn get_symbol_fragment(db: &dyn HirDatabase, field_or_assoc: &FieldOrAssocItem) FieldOrAssocItem::Field(field) => format!("#structfield.{}", field.name(db)), FieldOrAssocItem::AssocItem(assoc) => match assoc { AssocItem::Function(function) => { - let is_trait_method = - matches!(function.as_assoc_item(db).map(|assoc| assoc.container(db)), Some(AssocItemContainer::Trait(..))); + let is_trait_method = matches!( + function.as_assoc_item(db).map(|assoc| assoc.container(db)), + Some(AssocItemContainer::Trait(..)) + ); // This distinction may get more complicated when specialisation is available. // Rustdoc makes this decision based on whether a method 'has defaultness'. // Currently this is only the case for provided trait methods. diff --git a/crates/ide/src/hover.rs b/crates/ide/src/hover.rs index a53359d03635..6290b35bd894 100644 --- a/crates/ide/src/hover.rs +++ b/crates/ide/src/hover.rs @@ -14,8 +14,8 @@ use test_utils::mark; use crate::{ display::{macro_label, ShortLabel, ToNav, TryToNav}, - markdown_remove::remove_markdown, doc_links::{remove_links, rewrite_links}, + markdown_remove::remove_markdown, markup::Markup, runnables::runnable, FileId, FilePosition, NavigationTarget, RangeInfo, Runnable,