Changes from review

Author: zacps

crates/hir/src/code_model.rs

@@ -126,13 +126,16 @@ impl Crate {
     }
 
     /// Try to get the root URL of the documentation of a crate.
-    pub fn get_doc_url(self: &Crate, db: &dyn HirDatabase) -> Option<String> {
+    pub fn get_html_root_url(self: &Crate, db: &dyn HirDatabase) -> Option<String> {
         // Look for #![doc(html_root_url = "...")]
         let attrs = db.attrs(AttrDef::from(self.root_module(db)).into());
         let doc_attr_q = attrs.by_key("doc");
 
-        let doc_url = if doc_attr_q.exists() {
-            doc_attr_q.tt_values().map(|tt| {
+        if !doc_attr_q.exists() {
+            return None;
+        }
+
+        let doc_url = doc_attr_q.tt_values().map(|tt| {
             let name = tt.token_trees.iter()
                 .skip_while(|tt| !matches!(tt, TokenTree::Leaf(Leaf::Ident(Ident{text: ref ident, ..})) if ident == "html_root_url"))
                 .skip(2)
@@ -142,14 +145,9 @@ impl Crate {
                 Some(TokenTree::Leaf(Leaf::Literal(Literal{ref text, ..}))) => Some(text),
                 _ => None
             }
-        }).flat_map(|t| t).next().map(|s| s.to_string())
-        } else {
-            None
-        };
+        }).flat_map(|t| t).next();
 
-        doc_url
-            .map(|s| s.trim_matches('"').trim_end_matches("/").to_owned() + "/")
-            .map(|s| s.to_string())
+        doc_url.map(|s| s.trim_matches('"').trim_end_matches("/").to_owned() + "/")
     }
 }
 

crates/hir/src/doc_links.rs

@@ -38,21 +38,17 @@ fn try_resolve_intra<T: Resolvable, D: DefDatabase + HirDatabase>(
     let link_target =
         if link_target.is_empty() { link_text.trim_matches('`') } else { link_target };
 
-    // Namespace disambiguation
-    let namespace = Namespace::from_intra_spec(link_target);
-
-    // Strip prefixes/suffixes
-    let link_target = strip_prefixes_suffixes(link_target);
+    let doclink = IntraDocLink::from(link_target);
 
     // Parse link as a module path
-    let path = Path::parse(link_target).ok()?;
+    let path = Path::parse(doclink.path).ok()?;
     let modpath = ModPath::from_src(path, &Hygiene::new_unhygienic()).unwrap();
 
     // Resolve it relative to symbol's location (according to the RFC this should consider small scopes)
     let resolver = definition.resolver(db)?;
 
     let resolved = resolver.resolve_module_path_in_items(db, &modpath);
-    let (defid, namespace) = match namespace {
+    let (defid, namespace) = match doclink.namespace {
         // FIXME: .or(resolved.macros)
         None => resolved
             .types
@@ -133,7 +129,7 @@ fn strip_prefixes_suffixes(mut s: &str) -> &str {
 
 fn get_doc_url(db: &dyn HirDatabase, krate: &Crate) -> Option<Url> {
     krate
-        .get_doc_url(db)
+        .get_html_root_url(db)
         .or_else(||
         // Fallback to docs.rs
         // FIXME: Specify an exact version here. This may be difficult, as multiple versions of the same crate could exist.
@@ -164,6 +160,17 @@ fn get_symbol_filename(db: &dyn HirDatabase, definition: &ModuleDef) -> Option<S
     })
 }
 
+struct IntraDocLink<'s> {
+    path: &'s str,
+    namespace: Option<Namespace>,
+}
+
+impl<'s> From<&'s str> for IntraDocLink<'s> {
+    fn from(s: &'s str) -> Self {
+        Self { path: strip_prefixes_suffixes(s), namespace: Namespace::from_intra_spec(s) }
+    }
+}
+
 #[derive(PartialEq, Eq, Hash, Copy, Clone, Debug)]
 enum Namespace {
     Types,