From fc888b583d3dd06ea89a3558350dda9440c82f61 Mon Sep 17 00:00:00 2001 From: Ddystopia Date: Tue, 25 Apr 2023 11:10:08 +0200 Subject: [PATCH 01/16] Add `target_directory` path to `CargoWorkspace` --- crates/project-model/src/cargo_workspace.rs | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/crates/project-model/src/cargo_workspace.rs b/crates/project-model/src/cargo_workspace.rs index fb98d61963ca..e821cae00ac9 100644 --- a/crates/project-model/src/cargo_workspace.rs +++ b/crates/project-model/src/cargo_workspace.rs @@ -32,6 +32,7 @@ pub struct CargoWorkspace { packages: Arena, targets: Arena, workspace_root: AbsPathBuf, + target_directory: AbsPathBuf, } impl ops::Index for CargoWorkspace { @@ -414,7 +415,10 @@ impl CargoWorkspace { let workspace_root = AbsPathBuf::assert(PathBuf::from(meta.workspace_root.into_os_string())); - CargoWorkspace { packages, targets, workspace_root } + let target_directory = + AbsPathBuf::assert(PathBuf::from(meta.target_directory.into_os_string())); + + CargoWorkspace { packages, targets, workspace_root, target_directory } } pub fn packages(&self) -> impl Iterator + ExactSizeIterator + '_ { @@ -432,6 +436,10 @@ impl CargoWorkspace { &self.workspace_root } + pub fn target_directory(&self) -> &AbsPath { + &self.target_directory + } + pub fn package_flag(&self, package: &PackageData) -> String { if self.is_unique(&package.name) { package.name.clone() From f2d933ecaf5f3d1749c5aa31c3b99c07573c7369 Mon Sep 17 00:00:00 2001 From: Ddystopia Date: Tue, 25 Apr 2023 15:14:35 +0200 Subject: [PATCH 02/16] Add support for local documentation links alongside web documentation links, pending for `target_dir` path and tests --- crates/ide/src/doc_links.rs | 114 ++++++++++++------- crates/ide/src/lib.rs | 2 +- crates/rust-analyzer/src/handlers/request.rs | 9 +- crates/rust-analyzer/src/lsp_ext.rs | 2 +- 4 files changed, 82 insertions(+), 45 deletions(-) diff --git a/crates/ide/src/doc_links.rs b/crates/ide/src/doc_links.rs index 8d86c615d44d..a291389363b0 100644 --- a/crates/ide/src/doc_links.rs +++ b/crates/ide/src/doc_links.rs @@ -29,8 +29,16 @@ use crate::{ FilePosition, Semantics, }; -/// Weblink to an item's documentation. -pub(crate) type DocumentationLink = String; +/// Web and local links to an item's documentation. +#[derive(Default, Debug, Clone, PartialEq, Eq)] +pub struct DocumentationLinks { + /// The URL to the documentation on docs.rs. + /// Could be invalid. + pub web_url: Option, + /// The URL to the documentation in the local file system. + /// Could be invalid. + pub local_url: Option, +} const MARKDOWN_OPTIONS: Options = Options::ENABLE_FOOTNOTES.union(Options::ENABLE_TABLES).union(Options::ENABLE_TASKLISTS); @@ -119,10 +127,7 @@ pub(crate) fn remove_links(markdown: &str) -> String { // // | VS Code | **rust-analyzer: Open Docs** // |=== -pub(crate) fn external_docs( - db: &RootDatabase, - position: &FilePosition, -) -> Option { +pub(crate) fn external_docs(db: &RootDatabase, position: &FilePosition) -> DocumentationLinks { let sema = &Semantics::new(db); let file = sema.parse(position.file_id).syntax().clone(); let token = pick_best_token(file.token_at_offset(position.offset), |kind| match kind { @@ -130,27 +135,30 @@ pub(crate) fn external_docs( T!['('] | T![')'] => 2, kind if kind.is_trivia() => 0, _ => 1, - })?; + }); + let Some(token) = token else { return Default::default() }; let token = sema.descend_into_macros_single(token); - let node = token.parent()?; + let Some(node) = token.parent() else { return Default::default() }; let definition = match_ast! { match node { - ast::NameRef(name_ref) => match NameRefClass::classify(sema, &name_ref)? { - NameRefClass::Definition(def) => def, - NameRefClass::FieldShorthand { local_ref: _, field_ref } => { + ast::NameRef(name_ref) => match NameRefClass::classify(sema, &name_ref) { + Some(NameRefClass::Definition(def)) => def, + Some(NameRefClass::FieldShorthand { local_ref: _, field_ref }) => { Definition::Field(field_ref) } + None => return Default::default(), }, - ast::Name(name) => match NameClass::classify(sema, &name)? { - NameClass::Definition(it) | NameClass::ConstReference(it) => it, - NameClass::PatFieldShorthand { local_def: _, field_ref } => Definition::Field(field_ref), + ast::Name(name) => match NameClass::classify(sema, &name) { + Some(NameClass::Definition(it) | NameClass::ConstReference(it)) => it, + Some(NameClass::PatFieldShorthand { local_def: _, field_ref }) => Definition::Field(field_ref), + None => return Default::default(), }, - _ => return None, + _ => return Default::default(), } }; - get_doc_link(db, definition) + return get_doc_links(db, definition); } /// Extracts all links from a given markdown text returning the definition text range, link-text @@ -308,19 +316,34 @@ fn broken_link_clone_cb(link: BrokenLink<'_>) -> Option<(CowStr<'_>, CowStr<'_>) // // 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, def: Definition) -> Option { - let (target, file, frag) = filename_and_frag_for_def(db, def)?; +fn get_doc_links(db: &RootDatabase, def: Definition) -> DocumentationLinks { + let Some((target, file, frag)) = filename_and_frag_for_def(db, def) else { return Default::default(); }; - let mut url = get_doc_base_url(db, target)?; + let (mut web_url, mut local_url) = get_doc_base_urls(db, target); if let Some(path) = mod_path_of_def(db, target) { - url = url.join(&path).ok()?; + web_url = join_url(web_url, &path); + local_url = join_url(local_url, &path); } - url = url.join(&file).ok()?; - url.set_fragment(frag.as_deref()); + web_url = join_url(web_url, &file); + local_url = join_url(local_url, &file); + + set_fragment_for_url(web_url.as_mut(), frag.as_deref()); + set_fragment_for_url(local_url.as_mut(), frag.as_deref()); - Some(url.into()) + return DocumentationLinks { + web_url: web_url.map(|it| it.into()), + local_url: local_url.map(|it| it.into()), + }; + + fn join_url(base_url: Option, path: &str) -> Option { + base_url.and_then(|url| url.join(path).ok()) + } + + fn set_fragment_for_url(url: Option<&mut Url>, frag: Option<&str>) { + url.map(|url| url.set_fragment(frag)); + } } fn rewrite_intra_doc_link( @@ -332,7 +355,7 @@ fn rewrite_intra_doc_link( let (link, ns) = parse_intra_doc_link(target); let resolved = resolve_doc_path_for_def(db, def, link, ns)?; - let mut url = get_doc_base_url(db, resolved)?; + let mut url = get_doc_base_urls(db, resolved).0?; let (_, file, frag) = filename_and_frag_for_def(db, resolved)?; if let Some(path) = mod_path_of_def(db, resolved) { @@ -351,7 +374,7 @@ fn rewrite_url_link(db: &RootDatabase, def: Definition, target: &str) -> Option< return None; } - let mut url = get_doc_base_url(db, def)?; + let mut url = get_doc_base_urls(db, def).0?; let (def, file, frag) = filename_and_frag_for_def(db, def)?; if let Some(path) = mod_path_of_def(db, def) { @@ -427,18 +450,26 @@ fn map_links<'e>( /// https://doc.rust-lang.org/std/iter/trait.Iterator.html#tymethod.next /// ^^^^^^^^^^^^^^^^^^^^^^^^^^ /// ``` -fn get_doc_base_url(db: &RootDatabase, def: Definition) -> Option { +fn get_doc_base_urls(db: &RootDatabase, def: Definition) -> (Option, Option) { + // TODO: get this is from `CargoWorkspace` + // TODO: get `CargoWorkspace` from `db` + let target_path = "file:///project/root/target"; + let target_path = Url::parse(target_path).ok(); + let local_doc_path = target_path.and_then(|url| url.join("doc").ok()); + debug_assert!(local_doc_path.is_some(), "failed to parse local doc path"); + // special case base url of `BuiltinType` to core // https://github.com/rust-lang/rust-analyzer/issues/12250 if let Definition::BuiltinType(..) = def { - return Url::parse("https://doc.rust-lang.org/nightly/core/").ok(); + let weblink = Url::parse("https://doc.rust-lang.org/nightly/core/").ok(); + return (weblink, local_doc_path); }; - let krate = def.krate(db)?; - let display_name = krate.display_name(db)?; + let Some(krate) = def.krate(db) else { return Default::default() }; + let Some(display_name) = krate.display_name(db) else { return Default::default() }; let crate_data = &db.crate_graph()[krate.into()]; let channel = crate_data.channel.map_or("nightly", ReleaseChannel::as_str); - let base = match &crate_data.origin { + let (web_base, local_base) = match &crate_data.origin { // std and co do not specify `html_root_url` any longer so we gotta handwrite this ourself. // FIXME: Use the toolchains channel instead of nightly CrateOrigin::Lang( @@ -447,16 +478,14 @@ fn get_doc_base_url(db: &RootDatabase, def: Definition) -> Option { | LangCrateOrigin::ProcMacro | LangCrateOrigin::Std | LangCrateOrigin::Test), - ) => { - format!("https://doc.rust-lang.org/{channel}/{origin}") - } - CrateOrigin::Lang(_) => return None, + ) => (Some(format!("https://doc.rust-lang.org/{channel}/{origin}")), None), + CrateOrigin::Lang(_) => return (None, None), CrateOrigin::Rustc { name: _ } => { - format!("https://doc.rust-lang.org/{channel}/nightly-rustc/") + (Some(format!("https://doc.rust-lang.org/{channel}/nightly-rustc/")), None) } CrateOrigin::Local { repo: _, name: _ } => { // FIXME: These should not attempt to link to docs.rs! - krate.get_html_root_url(db).or_else(|| { + let weblink = krate.get_html_root_url(db).or_else(|| { let version = krate.version(db); // Fallback to docs.rs. This uses `display_name` and can never be // correct, but that's what fallbacks are about. @@ -468,10 +497,11 @@ fn get_doc_base_url(db: &RootDatabase, def: Definition) -> Option { krate = display_name, version = version.as_deref().unwrap_or("*") )) - })? + }); + (weblink, local_doc_path) } CrateOrigin::Library { repo: _, name } => { - krate.get_html_root_url(db).or_else(|| { + let weblink = krate.get_html_root_url(db).or_else(|| { let version = krate.version(db); // Fallback to docs.rs. This uses `display_name` and can never be // correct, but that's what fallbacks are about. @@ -483,10 +513,14 @@ fn get_doc_base_url(db: &RootDatabase, def: Definition) -> Option { krate = name, version = version.as_deref().unwrap_or("*") )) - })? + }); + (weblink, local_doc_path) } }; - Url::parse(&base).ok()?.join(&format!("{display_name}/")).ok() + let web_base = web_base + .and_then(|it| Url::parse(&it).ok()) + .and_then(|it| it.join(&format!("{display_name}/")).ok()); + (web_base, local_base) } /// Get the filename and extension generated for a symbol by rustdoc. diff --git a/crates/ide/src/lib.rs b/crates/ide/src/lib.rs index 24e2aed65a5b..4a4410ad20f5 100644 --- a/crates/ide/src/lib.rs +++ b/crates/ide/src/lib.rs @@ -471,7 +471,7 @@ impl Analysis { pub fn external_docs( &self, position: FilePosition, - ) -> Cancellable> { + ) -> Cancellable { self.with_db(|db| doc_links::external_docs(db, &position)) } diff --git a/crates/rust-analyzer/src/handlers/request.rs b/crates/rust-analyzer/src/handlers/request.rs index f25dc74a142b..47dd571054b8 100644 --- a/crates/rust-analyzer/src/handlers/request.rs +++ b/crates/rust-analyzer/src/handlers/request.rs @@ -1535,13 +1535,16 @@ pub(crate) fn handle_semantic_tokens_range( pub(crate) fn handle_open_docs( snap: GlobalStateSnapshot, params: lsp_types::TextDocumentPositionParams, -) -> Result> { +) -> Result<(Option, Option)> { let _p = profile::span("handle_open_docs"); let position = from_proto::file_position(&snap, params)?; - let remote = snap.analysis.external_docs(position)?; + let Ok(remote_urls) = snap.analysis.external_docs(position) else { return Ok((None, None)); }; - Ok(remote.and_then(|remote| Url::parse(&remote).ok())) + let web_url = remote_urls.web_url.and_then(|it| Url::parse(&it).ok()); + let local_url = remote_urls.local_url.and_then(|it| Url::parse(&it).ok()); + + Ok((web_url, local_url)) } pub(crate) fn handle_open_cargo_toml( diff --git a/crates/rust-analyzer/src/lsp_ext.rs b/crates/rust-analyzer/src/lsp_ext.rs index 69e7d824680f..9c27f6e1c335 100644 --- a/crates/rust-analyzer/src/lsp_ext.rs +++ b/crates/rust-analyzer/src/lsp_ext.rs @@ -508,7 +508,7 @@ pub enum ExternalDocs {} impl Request for ExternalDocs { type Params = lsp_types::TextDocumentPositionParams; - type Result = Option; + type Result = (Option, Option); const METHOD: &'static str = "experimental/externalDocs"; } From c47a34fddc4663563e172125f17bae86bb2e8f0f Mon Sep 17 00:00:00 2001 From: Ddystopia Date: Tue, 25 Apr 2023 19:49:49 +0200 Subject: [PATCH 03/16] Add `target_dir` path argument for `external_docs` and other methods --- crates/ide/src/doc_links.rs | 42 ++++++++++++-------- crates/ide/src/lib.rs | 5 ++- crates/rust-analyzer/src/handlers/request.rs | 8 +++- 3 files changed, 36 insertions(+), 19 deletions(-) diff --git a/crates/ide/src/doc_links.rs b/crates/ide/src/doc_links.rs index a291389363b0..5b67761acd40 100644 --- a/crates/ide/src/doc_links.rs +++ b/crates/ide/src/doc_links.rs @@ -5,6 +5,8 @@ mod tests; mod intra_doc_links; +use std::ffi::OsStr; + use pulldown_cmark::{BrokenLink, CowStr, Event, InlineStr, LinkType, Options, Parser, Tag}; use pulldown_cmark_to_cmark::{cmark_resume_with_options, Options as CMarkOptions}; use stdx::format_to; @@ -127,7 +129,11 @@ pub(crate) fn remove_links(markdown: &str) -> String { // // | VS Code | **rust-analyzer: Open Docs** // |=== -pub(crate) fn external_docs(db: &RootDatabase, position: &FilePosition) -> DocumentationLinks { +pub(crate) fn external_docs( + db: &RootDatabase, + position: &FilePosition, + target_dir: Option<&OsStr>, +) -> DocumentationLinks { let sema = &Semantics::new(db); let file = sema.parse(position.file_id).syntax().clone(); let token = pick_best_token(file.token_at_offset(position.offset), |kind| match kind { @@ -158,7 +164,7 @@ pub(crate) fn external_docs(db: &RootDatabase, position: &FilePosition) -> Docum } }; - return get_doc_links(db, definition); + return get_doc_links(db, definition, target_dir); } /// Extracts all links from a given markdown text returning the definition text range, link-text @@ -316,10 +322,14 @@ fn broken_link_clone_cb(link: BrokenLink<'_>) -> Option<(CowStr<'_>, CowStr<'_>) // // 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_links(db: &RootDatabase, def: Definition) -> DocumentationLinks { +fn get_doc_links( + db: &RootDatabase, + def: Definition, + target_dir: Option<&OsStr>, +) -> DocumentationLinks { let Some((target, file, frag)) = filename_and_frag_for_def(db, def) else { return Default::default(); }; - let (mut web_url, mut local_url) = get_doc_base_urls(db, target); + let (mut web_url, mut local_url) = get_doc_base_urls(db, target, target_dir); if let Some(path) = mod_path_of_def(db, target) { web_url = join_url(web_url, &path); @@ -355,7 +365,7 @@ fn rewrite_intra_doc_link( let (link, ns) = parse_intra_doc_link(target); let resolved = resolve_doc_path_for_def(db, def, link, ns)?; - let mut url = get_doc_base_urls(db, resolved).0?; + let mut url = get_doc_base_urls(db, resolved, None).0?; let (_, file, frag) = filename_and_frag_for_def(db, resolved)?; if let Some(path) = mod_path_of_def(db, resolved) { @@ -374,7 +384,7 @@ fn rewrite_url_link(db: &RootDatabase, def: Definition, target: &str) -> Option< return None; } - let mut url = get_doc_base_urls(db, def).0?; + let mut url = get_doc_base_urls(db, def, None).0?; let (def, file, frag) = filename_and_frag_for_def(db, def)?; if let Some(path) = mod_path_of_def(db, def) { @@ -450,14 +460,14 @@ fn map_links<'e>( /// https://doc.rust-lang.org/std/iter/trait.Iterator.html#tymethod.next /// ^^^^^^^^^^^^^^^^^^^^^^^^^^ /// ``` -fn get_doc_base_urls(db: &RootDatabase, def: Definition) -> (Option, Option) { - // TODO: get this is from `CargoWorkspace` - // TODO: get `CargoWorkspace` from `db` - let target_path = "file:///project/root/target"; - let target_path = Url::parse(target_path).ok(); - let local_doc_path = target_path.and_then(|url| url.join("doc").ok()); - debug_assert!(local_doc_path.is_some(), "failed to parse local doc path"); - +fn get_doc_base_urls( + db: &RootDatabase, + def: Definition, + target_dir: Option<&OsStr>, +) -> (Option, Option) { + let local_doc_path = target_dir + .and_then(|it| Url::from_directory_path(it).ok()) + .and_then(|it| it.join("doc").ok()); // special case base url of `BuiltinType` to core // https://github.com/rust-lang/rust-analyzer/issues/12250 if let Definition::BuiltinType(..) = def { @@ -465,8 +475,8 @@ fn get_doc_base_urls(db: &RootDatabase, def: Definition) -> (Option, Option return (weblink, local_doc_path); }; - let Some(krate) = def.krate(db) else { return Default::default() }; - let Some(display_name) = krate.display_name(db) else { return Default::default() }; + let Some(krate) = def.krate(db) else { return (None, local_doc_path) }; + let Some(display_name) = krate.display_name(db) else { return (None, local_doc_path) }; let crate_data = &db.crate_graph()[krate.into()]; let channel = crate_data.channel.map_or("nightly", ReleaseChannel::as_str); let (web_base, local_base) = match &crate_data.origin { diff --git a/crates/ide/src/lib.rs b/crates/ide/src/lib.rs index 4a4410ad20f5..4e930cbc5906 100644 --- a/crates/ide/src/lib.rs +++ b/crates/ide/src/lib.rs @@ -61,7 +61,7 @@ mod view_item_tree; mod shuffle_crate_graph; mod fetch_crates; -use std::sync::Arc; +use std::{ffi::OsStr, sync::Arc}; use cfg::CfgOptions; use fetch_crates::CrateInfo; @@ -471,8 +471,9 @@ impl Analysis { pub fn external_docs( &self, position: FilePosition, + target_dir: Option<&OsStr>, ) -> Cancellable { - self.with_db(|db| doc_links::external_docs(db, &position)) + self.with_db(|db| doc_links::external_docs(db, &position, target_dir)) } /// Computes parameter information at the given position. diff --git a/crates/rust-analyzer/src/handlers/request.rs b/crates/rust-analyzer/src/handlers/request.rs index 47dd571054b8..1332dc43e984 100644 --- a/crates/rust-analyzer/src/handlers/request.rs +++ b/crates/rust-analyzer/src/handlers/request.rs @@ -1539,7 +1539,13 @@ pub(crate) fn handle_open_docs( let _p = profile::span("handle_open_docs"); let position = from_proto::file_position(&snap, params)?; - let Ok(remote_urls) = snap.analysis.external_docs(position) else { return Ok((None, None)); }; + let cargo = match snap.workspaces.get(0) { + Some(ProjectWorkspace::Cargo { cargo, .. }) => Some(cargo), + _ => None, + }; + let target_dir = + cargo.and_then(|cargo| Some(cargo.target_directory())).and_then(|p| Some(p.as_os_str())); + let Ok(remote_urls) = snap.analysis.external_docs(position, target_dir) else { return Ok((None, None)); }; let web_url = remote_urls.web_url.and_then(|it| Url::parse(&it).ok()); let local_url = remote_urls.local_url.and_then(|it| Url::parse(&it).ok()); From 3fbb48907e775278982fc4522a2f0925de389396 Mon Sep 17 00:00:00 2001 From: Ddystopia Date: Tue, 25 Apr 2023 20:16:28 +0200 Subject: [PATCH 04/16] Add doc strings --- crates/ide/src/doc_links.rs | 8 +++++--- crates/ide/src/lib.rs | 5 ++++- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/crates/ide/src/doc_links.rs b/crates/ide/src/doc_links.rs index 5b67761acd40..8f7cd534a27d 100644 --- a/crates/ide/src/doc_links.rs +++ b/crates/ide/src/doc_links.rs @@ -35,10 +35,10 @@ use crate::{ #[derive(Default, Debug, Clone, PartialEq, Eq)] pub struct DocumentationLinks { /// The URL to the documentation on docs.rs. - /// Could be invalid. + /// May not lead anywhere. pub web_url: Option, /// The URL to the documentation in the local file system. - /// Could be invalid. + /// May not lead anywhere. pub local_url: Option, } @@ -119,7 +119,7 @@ pub(crate) fn remove_links(markdown: &str) -> String { // Feature: Open Docs // -// Retrieve a link to documentation for the given symbol. +// Retrieve a links to documentation for the given symbol. // // The simplest way to use this feature is via the context menu. Right-click on // the selected item. The context menu opens. Select **Open Docs**. @@ -459,6 +459,8 @@ fn map_links<'e>( /// ```ignore /// https://doc.rust-lang.org/std/iter/trait.Iterator.html#tymethod.next /// ^^^^^^^^^^^^^^^^^^^^^^^^^^ +/// file:///project/root/target/doc/std/iter/trait.Iterator.html#tymethod.next +/// ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ /// ``` fn get_doc_base_urls( db: &RootDatabase, diff --git a/crates/ide/src/lib.rs b/crates/ide/src/lib.rs index 4e930cbc5906..80f9707f9af0 100644 --- a/crates/ide/src/lib.rs +++ b/crates/ide/src/lib.rs @@ -467,7 +467,10 @@ impl Analysis { self.with_db(|db| moniker::moniker(db, position)) } - /// Return URL(s) for the documentation of the symbol under the cursor. + /// Returns URL(s) for the documentation of the symbol under the cursor. + /// # Arguments + /// * `position` - Position in the file. + /// * `target_dir` - Directory where the build output is storeda. pub fn external_docs( &self, position: FilePosition, From 2503fbefde0780186013df4696625cf4f5e9ba27 Mon Sep 17 00:00:00 2001 From: Ddystopia Date: Wed, 26 Apr 2023 14:50:13 +0200 Subject: [PATCH 05/16] Small improvements and fixes --- crates/ide/src/doc_links.rs | 22 ++++++++++++++++------ 1 file changed, 16 insertions(+), 6 deletions(-) diff --git a/crates/ide/src/doc_links.rs b/crates/ide/src/doc_links.rs index 8f7cd534a27d..ac91354a34a8 100644 --- a/crates/ide/src/doc_links.rs +++ b/crates/ide/src/doc_links.rs @@ -467,9 +467,8 @@ fn get_doc_base_urls( def: Definition, target_dir: Option<&OsStr>, ) -> (Option, Option) { - let local_doc_path = target_dir - .and_then(|it| Url::from_directory_path(it).ok()) - .and_then(|it| it.join("doc").ok()); + let local_doc_path = + target_dir.and_then(create_url_from_os_str).and_then(|it| it.join("doc/").ok()); // special case base url of `BuiltinType` to core // https://github.com/rust-lang/rust-analyzer/issues/12250 if let Definition::BuiltinType(..) = def { @@ -477,8 +476,8 @@ fn get_doc_base_urls( return (weblink, local_doc_path); }; - let Some(krate) = def.krate(db) else { return (None, local_doc_path) }; - let Some(display_name) = krate.display_name(db) else { return (None, local_doc_path) }; + let Some(krate) = def.krate(db) else { return Default::default() }; + let Some(display_name) = krate.display_name(db) else { return Default::default() }; let crate_data = &db.crate_graph()[krate.into()]; let channel = crate_data.channel.map_or("nightly", ReleaseChannel::as_str); let (web_base, local_base) = match &crate_data.origin { @@ -532,7 +531,18 @@ fn get_doc_base_urls( let web_base = web_base .and_then(|it| Url::parse(&it).ok()) .and_then(|it| it.join(&format!("{display_name}/")).ok()); - (web_base, local_base) + let local_base = local_base.and_then(|it| it.join(&format!("{display_name}/")).ok()); + + return (web_base, local_base); + + // On Windows, cargo metadata returns paths without leading slashes, but + // Url::from_directory_path requires them. + // In unix adding another "/" will not make any difference. + fn create_url_from_os_str(path: &OsStr) -> Option { + let mut with_leading_slash = OsStr::new("/").to_os_string(); + with_leading_slash.push(path); + Url::from_directory_path(with_leading_slash.as_os_str()).ok() + } } /// Get the filename and extension generated for a symbol by rustdoc. From da0ffe79d0a0006d379ad1eec9e3e1b27ad98583 Mon Sep 17 00:00:00 2001 From: Ddystopia Date: Wed, 26 Apr 2023 14:51:19 +0200 Subject: [PATCH 06/16] Change signature of `externalDocs` in `lsp-extensions` --- docs/dev/lsp-extensions.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/dev/lsp-extensions.md b/docs/dev/lsp-extensions.md index a4ad3e5a553c..55a3708d2959 100644 --- a/docs/dev/lsp-extensions.md +++ b/docs/dev/lsp-extensions.md @@ -386,13 +386,13 @@ rust-analyzer supports only one `kind`, `"cargo"`. The `args` for `"cargo"` look ## Open External Documentation -This request is sent 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 web and local URL(s) to documentation for the symbol under the cursor, if available. **Method** `experimental/externalDocs` **Request:**: `TextDocumentPositionParams` -**Response** `string | null` +**Response** `[string | null, string | null]` ## Analyzer Status From 12292e445afeb0757848a409e1e872f28db9cc74 Mon Sep 17 00:00:00 2001 From: Ddystopia Date: Wed, 26 Apr 2023 14:51:45 +0200 Subject: [PATCH 07/16] Tests for `externalDocs` --- crates/ide/src/doc_links/tests.rs | 119 +++++++++++++++++++++++++----- 1 file changed, 100 insertions(+), 19 deletions(-) diff --git a/crates/ide/src/doc_links/tests.rs b/crates/ide/src/doc_links/tests.rs index b6b46c450888..d04d124ce02d 100644 --- a/crates/ide/src/doc_links/tests.rs +++ b/crates/ide/src/doc_links/tests.rs @@ -1,3 +1,5 @@ +use std::ffi::OsStr; + use expect_test::{expect, Expect}; use hir::{HasAttrs, Semantics}; use ide_db::{ @@ -13,11 +15,32 @@ use crate::{ fixture, TryToNav, }; -fn check_external_docs(ra_fixture: &str, expect: Expect) { +fn check_external_docs( + ra_fixture: &str, + target_dir: Option<&OsStr>, + expect_web_url: Option, + expect_local_url: Option, +) { let (analysis, position) = fixture::position(ra_fixture); - let url = analysis.external_docs(position).unwrap().expect("could not find url for symbol"); + let links = analysis.external_docs(position, target_dir).unwrap(); + + let web_url = links.web_url; + let local_url = links.local_url; + + println!("web_url: {:?}", web_url); + println!("local_url: {:?}", local_url); - expect.assert_eq(&url) + match (expect_web_url, web_url) { + (Some(expect), Some(url)) => expect.assert_eq(&url), + (None, None) => (), + _ => panic!("Unexpected web url"), + } + + match (expect_local_url, local_url) { + (Some(expect), Some(url)) => expect.assert_eq(&url), + (None, None) => (), + _ => panic!("Unexpected local url"), + } } fn check_rewrite(ra_fixture: &str, expect: Expect) { @@ -105,7 +128,9 @@ use foo$0::Foo; //- /lib.rs crate:foo pub struct Foo; "#, - expect![[r#"https://docs.rs/foo/*/foo/index.html"#]], + Some(&OsStr::new("/home/user/project/")), + Some(expect![[r#"https://docs.rs/foo/*/foo/index.html"#]]), + Some(expect![[r#"file:///home/user/project/doc/foo/index.html"#]]), ); } @@ -116,7 +141,9 @@ fn external_docs_doc_url_std_crate() { //- /main.rs crate:std use self$0; "#, - expect!["https://doc.rust-lang.org/stable/std/index.html"], + Some(&OsStr::new("/home/user/project/")), + Some(expect!["https://doc.rust-lang.org/stable/std/index.html"]), + None, ); } @@ -127,7 +154,35 @@ fn external_docs_doc_url_struct() { //- /main.rs crate:foo pub struct Fo$0o; "#, - expect![[r#"https://docs.rs/foo/*/foo/struct.Foo.html"#]], + Some(&OsStr::new("/home/user/project/")), + Some(expect![[r#"https://docs.rs/foo/*/foo/struct.Foo.html"#]]), + Some(expect![[r#"file:///home/user/project/doc/foo/struct.Foo.html"#]]), + ); +} + +#[test] +fn external_docs_doc_url_windows_backslash_path() { + check_external_docs( + r#" +//- /main.rs crate:foo +pub struct Fo$0o; +"#, + Some(&OsStr::new(r"C:\Users\user\project")), + Some(expect![[r#"https://docs.rs/foo/*/foo/struct.Foo.html"#]]), + Some(expect![[r#"file:///C:\Users\user\project/doc/foo/struct.Foo.html"#]]), + ); +} + +#[test] +fn external_docs_doc_url_windows_slash_path() { + check_external_docs( + r#" +//- /main.rs crate:foo +pub struct Fo$0o; +"#, + Some(&OsStr::new(r"C:/Users/user/project")), + Some(expect![[r#"https://docs.rs/foo/*/foo/struct.Foo.html"#]]), + Some(expect![[r#"file:///C:/Users/user/project/doc/foo/struct.Foo.html"#]]), ); } @@ -140,7 +195,9 @@ pub struct Foo { field$0: () } "#, - expect![[r##"https://docs.rs/foo/*/foo/struct.Foo.html#structfield.field"##]], + None, + Some(expect![[r##"https://docs.rs/foo/*/foo/struct.Foo.html#structfield.field"##]]), + None, ); } @@ -151,7 +208,9 @@ fn external_docs_doc_url_fn() { //- /main.rs crate:foo pub fn fo$0o() {} "#, - expect![[r#"https://docs.rs/foo/*/foo/fn.foo.html"#]], + None, + Some(expect![[r#"https://docs.rs/foo/*/foo/fn.foo.html"#]]), + None, ); } @@ -165,7 +224,9 @@ impl Foo { pub fn method$0() {} } "#, - expect![[r##"https://docs.rs/foo/*/foo/struct.Foo.html#method.method"##]], + None, + Some(expect![[r##"https://docs.rs/foo/*/foo/struct.Foo.html#method.method"##]]), + None, ); check_external_docs( r#" @@ -175,7 +236,9 @@ impl Foo { const CONST$0: () = (); } "#, - expect![[r##"https://docs.rs/foo/*/foo/struct.Foo.html#associatedconstant.CONST"##]], + None, + Some(expect![[r##"https://docs.rs/foo/*/foo/struct.Foo.html#associatedconstant.CONST"##]]), + None, ); } @@ -192,7 +255,9 @@ impl Trait for Foo { pub fn method$0() {} } "#, - expect![[r##"https://docs.rs/foo/*/foo/struct.Foo.html#method.method"##]], + None, + Some(expect![[r##"https://docs.rs/foo/*/foo/struct.Foo.html#method.method"##]]), + None, ); check_external_docs( r#" @@ -205,7 +270,9 @@ impl Trait for Foo { const CONST$0: () = (); } "#, - expect![[r##"https://docs.rs/foo/*/foo/struct.Foo.html#associatedconstant.CONST"##]], + None, + Some(expect![[r##"https://docs.rs/foo/*/foo/struct.Foo.html#associatedconstant.CONST"##]]), + None, ); check_external_docs( r#" @@ -218,7 +285,9 @@ impl Trait for Foo { type Type$0 = (); } "#, - expect![[r##"https://docs.rs/foo/*/foo/struct.Foo.html#associatedtype.Type"##]], + None, + Some(expect![[r##"https://docs.rs/foo/*/foo/struct.Foo.html#associatedtype.Type"##]]), + None, ); } @@ -231,7 +300,9 @@ pub trait Foo { fn method$0(); } "#, - expect![[r##"https://docs.rs/foo/*/foo/trait.Foo.html#tymethod.method"##]], + None, + Some(expect![[r##"https://docs.rs/foo/*/foo/trait.Foo.html#tymethod.method"##]]), + None, ); check_external_docs( r#" @@ -240,7 +311,9 @@ pub trait Foo { const CONST$0: (); } "#, - expect![[r##"https://docs.rs/foo/*/foo/trait.Foo.html#associatedconstant.CONST"##]], + None, + Some(expect![[r##"https://docs.rs/foo/*/foo/trait.Foo.html#associatedconstant.CONST"##]]), + None, ); check_external_docs( r#" @@ -249,7 +322,9 @@ pub trait Foo { type Type$0; } "#, - expect![[r##"https://docs.rs/foo/*/foo/trait.Foo.html#associatedtype.Type"##]], + None, + Some(expect![[r##"https://docs.rs/foo/*/foo/trait.Foo.html#associatedtype.Type"##]]), + None, ); } @@ -260,7 +335,9 @@ fn external_docs_trait() { //- /main.rs crate:foo trait Trait$0 {} "#, - expect![[r#"https://docs.rs/foo/*/foo/trait.Trait.html"#]], + None, + Some(expect![[r#"https://docs.rs/foo/*/foo/trait.Trait.html"#]]), + None, ) } @@ -273,7 +350,9 @@ pub mod foo { pub mod ba$0r {} } "#, - expect![[r#"https://docs.rs/foo/*/foo/foo/bar/index.html"#]], + None, + Some(expect![[r#"https://docs.rs/foo/*/foo/foo/bar/index.html"#]]), + None, ) } @@ -294,7 +373,9 @@ fn foo() { let bar: wrapper::It$0em; } "#, - expect![[r#"https://docs.rs/foo/*/foo/wrapper/module/struct.Item.html"#]], + None, + Some(expect![[r#"https://docs.rs/foo/*/foo/wrapper/module/struct.Item.html"#]]), + None, ) } From b64c31c40aa36b45f2d2caea43966a4469c3e0a6 Mon Sep 17 00:00:00 2001 From: Ddystopia Date: Wed, 26 Apr 2023 16:10:45 +0200 Subject: [PATCH 08/16] Solve platform-specific issues --- crates/ide/src/doc_links.rs | 10 ++++------ crates/ide/src/doc_links/tests.rs | 8 ++++---- 2 files changed, 8 insertions(+), 10 deletions(-) diff --git a/crates/ide/src/doc_links.rs b/crates/ide/src/doc_links.rs index ac91354a34a8..017ca48e3b07 100644 --- a/crates/ide/src/doc_links.rs +++ b/crates/ide/src/doc_links.rs @@ -535,13 +535,11 @@ fn get_doc_base_urls( return (web_base, local_base); - // On Windows, cargo metadata returns paths without leading slashes, but - // Url::from_directory_path requires them. - // In unix adding another "/" will not make any difference. fn create_url_from_os_str(path: &OsStr) -> Option { - let mut with_leading_slash = OsStr::new("/").to_os_string(); - with_leading_slash.push(path); - Url::from_directory_path(with_leading_slash.as_os_str()).ok() + let mut with_prefix = OsStr::new("file:///").to_os_string(); + with_prefix.push(path); + with_prefix.push("/"); + with_prefix.to_str().and_then(|s| Url::parse(s).ok()) } } diff --git a/crates/ide/src/doc_links/tests.rs b/crates/ide/src/doc_links/tests.rs index d04d124ce02d..21bd9fb3217d 100644 --- a/crates/ide/src/doc_links/tests.rs +++ b/crates/ide/src/doc_links/tests.rs @@ -128,7 +128,7 @@ use foo$0::Foo; //- /lib.rs crate:foo pub struct Foo; "#, - Some(&OsStr::new("/home/user/project/")), + Some(&OsStr::new("/home/user/project")), Some(expect![[r#"https://docs.rs/foo/*/foo/index.html"#]]), Some(expect![[r#"file:///home/user/project/doc/foo/index.html"#]]), ); @@ -141,7 +141,7 @@ fn external_docs_doc_url_std_crate() { //- /main.rs crate:std use self$0; "#, - Some(&OsStr::new("/home/user/project/")), + Some(&OsStr::new("/home/user/project")), Some(expect!["https://doc.rust-lang.org/stable/std/index.html"]), None, ); @@ -154,7 +154,7 @@ fn external_docs_doc_url_struct() { //- /main.rs crate:foo pub struct Fo$0o; "#, - Some(&OsStr::new("/home/user/project/")), + Some(&OsStr::new("/home/user/project")), Some(expect![[r#"https://docs.rs/foo/*/foo/struct.Foo.html"#]]), Some(expect![[r#"file:///home/user/project/doc/foo/struct.Foo.html"#]]), ); @@ -169,7 +169,7 @@ pub struct Fo$0o; "#, Some(&OsStr::new(r"C:\Users\user\project")), Some(expect![[r#"https://docs.rs/foo/*/foo/struct.Foo.html"#]]), - Some(expect![[r#"file:///C:\Users\user\project/doc/foo/struct.Foo.html"#]]), + Some(expect![[r#"file:///C:/Users/user/project/doc/foo/struct.Foo.html"#]]), ); } From b74b9797bcc766fe0ddbb1a72675460c29ad1fe5 Mon Sep 17 00:00:00 2001 From: Ddystopia Date: Wed, 26 Apr 2023 16:47:34 +0200 Subject: [PATCH 09/16] Update hash in `lsp-extensions.md` --- docs/dev/lsp-extensions.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/dev/lsp-extensions.md b/docs/dev/lsp-extensions.md index 55a3708d2959..13407f80b228 100644 --- a/docs/dev/lsp-extensions.md +++ b/docs/dev/lsp-extensions.md @@ -1,5 +1,5 @@