feat(cli/rustup-mode): allow rustup doc with both a flag and a topic

src/cli/rustup_mode.rs

@@ -1,11 +1,12 @@
+use std::borrow::Cow;
 use std::env::consts::EXE_SUFFIX;
 use std::fmt;
 use std::io::Write;
 use std::path::{Path, PathBuf};
 use std::process::ExitStatus;
 use std::str::FromStr;
 
-use anyhow::{anyhow, Error, Result};
+use anyhow::{anyhow, Context, Error, Result};
 use clap::{builder::PossibleValue, Args, CommandFactory, Parser, Subcommand, ValueEnum};
 use clap_complete::Shell;
 use itertools::Itertools;
@@ -1425,11 +1426,7 @@ macro_rules! docs_data {
         }
 
         impl DocPage {
-            fn name(&self) -> Option<&'static str> {
-                Some(self.path()?.rsplit_once('/')?.0)
-            }
-
-            fn path(&self) -> Option<&'static str> {
+            fn path_str(&self) -> Option<&'static str> {
                 $( if self.$ident { return Some($path); } )+
                 None
             }
@@ -1466,6 +1463,42 @@ docs_data![
     (unstable_book, "The Unstable Book", "unstable-book/index.html"),
 ];
 
+impl DocPage {
+    fn path(&self) -> Option<&'static Path> {
+        self.path_str().map(Path::new)
+    }
+
+    fn name(&self) -> Option<&'static str> {
+        Some(self.path_str()?.rsplit_once('/')?.0)
+    }
+
+    fn resolve<'t>(&self, root: &Path, topic: &'t str) -> Option<(PathBuf, Option<&'t str>)> {
+        // Save the components in case the last one is used later with `parent_html`.
+        let components = topic.split("::").collect::<Vec<_>>();
+
+        // Use `.parent()` to chop off the default top-level `index.html`.
+        let mut base = root.join(Path::new(self.path()?).parent()?);
+        base.extend(&components);
+        let base_index_html = base.join("index.html");
+
+        if base_index_html.is_file() {
+            return Some((base_index_html, None));
+        }
+
+        let base_html = base.with_extension("html");
+        if base_html.is_file() {
+            return Some((base_html, None));
+        }
+
+        let parent_html = base.parent()?.with_extension("html");
+        if parent_html.is_file() {
+            return Some((parent_html, components.last().copied()));
+        }
+
+        None
+    }
+}
+
 async fn doc(
     cfg: &Cfg<'_>,
     path_only: bool,
@@ -1500,32 +1533,40 @@ async fn doc(
         }
     };
 
-    let topical_path: PathBuf;
-
-    let doc_url = if let Some(topic) = topic {
-        topical_path = topical_doc::local_path(&toolchain.doc_path("").unwrap(), topic)?;
-        topical_path.to_str().unwrap()
-    } else {
-        topic = doc_page.name();
-        doc_page.path().unwrap_or("index.html")
+    let (doc_path, fragment): (Cow<'_, Path>, _) = match (topic, doc_page.name()) {
+        (Some(topic), Some(name)) => {
+            let (doc_path, fragment) = doc_page
+                .resolve(&toolchain.doc_path("")?, topic)
+                .context(format!("no document for {name} on {topic}"))?;
+            (doc_path.into(), fragment)
+        }
+        (Some(topic), None) => {
+            let doc_path = topical_doc::local_path(&toolchain.doc_path("").unwrap(), topic)?;
+            (doc_path.into(), None)
+        }
+        (None, name) => {
+            topic = name;
+            let doc_path = doc_page.path().unwrap_or(Path::new("index.html"));
+            (doc_path.into(), None)
+        }
     };
 
     if path_only {
-        let doc_path = toolchain.doc_path(doc_url)?;
+        let doc_path = toolchain.doc_path(&doc_path)?;
         writeln!(cfg.process.stdout().lock(), "{}", doc_path.display())?;
-        Ok(utils::ExitCode(0))
+        return Ok(utils::ExitCode(0));
+    }
+
+    if let Some(name) = topic {
+        writeln!(
+            cfg.process.stderr().lock(),
+            "Opening docs named `{name}` in your browser"
+        )?;
     } else {
-        if let Some(name) = topic {
-            writeln!(
-                cfg.process.stderr().lock(),
-                "Opening docs named `{name}` in your browser"
-            )?;
-        } else {
-            writeln!(cfg.process.stderr().lock(), "Opening docs in your browser")?;
-        }
-        toolchain.open_docs(doc_url)?;
-        Ok(utils::ExitCode(0))
+        writeln!(cfg.process.stderr().lock(), "Opening docs in your browser")?;
     }
+    toolchain.open_docs(&doc_path, fragment)?;
+    Ok(utils::ExitCode(0))
 }
 
 #[cfg(not(windows))]

src/cli/topical_doc.rs

@@ -2,7 +2,8 @@ use std::ffi::OsString;
 use std::fs;
 use std::path::{Path, PathBuf};
 
-use anyhow::{anyhow, bail, Context, Result};
+use anyhow::{anyhow, Context, Result};
+use itertools::Itertools;
 
 struct DocData<'a> {
     topic: &'a str,
@@ -19,13 +20,10 @@ fn index_html(doc: &DocData<'_>, wpath: &Path) -> Option<PathBuf> {
 }
 
 fn dir_into_vec(dir: &Path) -> Result<Vec<OsString>> {
-    let entries = fs::read_dir(dir).with_context(|| format!("Failed to read_dir {dir:?}"))?;
-    let mut v = Vec::new();
-    for entry in entries {
-        let entry = entry?;
-        v.push(entry.file_name());
-    }
-    Ok(v)
+    fs::read_dir(dir)
+        .with_context(|| format!("Failed to read_dir {dir:?}"))?
+        .map(|f| anyhow::Ok(f?.file_name()))
+        .try_collect()
 }
 
 fn search_path(doc: &DocData<'_>, wpath: &Path, keywords: &[&str]) -> Result<PathBuf> {
@@ -118,17 +116,12 @@ pub(crate) fn local_path(root: &Path, topic: &str) -> Result<PathBuf> {
     // topic.split.count cannot be 0
     let subpath_os_path = match topic_vec.len() {
         1 => match topic {
-            "std" | "core" | "alloc" => match index_html(&doc, &work_path) {
-                Some(f) => f,
-                None => bail!(format!("No document for '{}'", doc.topic)),
-            },
+            "std" | "core" | "alloc" => {
+                index_html(&doc, &work_path).context(anyhow!("No document for '{}'", doc.topic))?
+            }
             _ => {
-                let std = PathBuf::from("std");
-                let search_keywords = match forced_keyword {
-                    Some(k) => k,
-                    None => keywords_top,
-                };
-                search_path(&doc, &std, &search_keywords)?
+                let search_keywords = forced_keyword.unwrap_or(keywords_top);
+                search_path(&doc, Path::new("std"), &search_keywords)?
             }
         },
         2 => match index_html(&doc, &work_path) {

src/test/mock/topical_doc_data.rs

@@ -3,23 +3,29 @@ use std::path::PathBuf;
 
 // Paths are written as a string in the UNIX format to make it easy
 // to maintain.
-static TEST_CASES: &[&[&str]] = &[
-    &["core", "core/index.html"],
-    &["core::arch", "core/arch/index.html"],
-    &["fn", "std/keyword.fn.html"],
-    &["keyword:fn", "std/keyword.fn.html"],
-    &["primitive:fn", "std/primitive.fn.html"],
-    &["macro:file!", "std/macro.file!.html"],
-    &["macro:file", "std/macro.file.html"],
-    &["std::fs", "std/fs/index.html"],
-    &["std::fs::read_dir", "std/fs/fn.read_dir.html"],
-    &["std::io::Bytes", "std/io/struct.Bytes.html"],
-    &["std::iter::Sum", "std/iter/trait.Sum.html"],
-    &["std::io::error::Result", "std/io/error/type.Result.html"],
-    &["usize", "std/primitive.usize.html"],
-    &["eprintln", "std/macro.eprintln.html"],
-    &["alloc::format", "alloc/macro.format.html"],
-    &["std::mem::MaybeUninit", "std/mem/union.MaybeUninit.html"],
+static TEST_CASES: &[(&[&str], &str)] = &[
+    (&["core"], "core/index.html"),
+    (&["core::arch"], "core/arch/index.html"),
+    (&["fn"], "std/keyword.fn.html"),
+    (&["keyword:fn"], "std/keyword.fn.html"),
+    (&["primitive:fn"], "std/primitive.fn.html"),
+    (&["macro:file!"], "std/macro.file!.html"),
+    (&["macro:file"], "std/macro.file.html"),
+    (&["std::fs"], "std/fs/index.html"),
+    (&["std::fs::read_dir"], "std/fs/fn.read_dir.html"),
+    (&["std::io::Bytes"], "std/io/struct.Bytes.html"),
+    (&["std::iter::Sum"], "std/iter/trait.Sum.html"),
+    (&["std::io::error::Result"], "std/io/error/type.Result.html"),
+    (&["usize"], "std/primitive.usize.html"),
+    (&["eprintln"], "std/macro.eprintln.html"),
+    (&["alloc::format"], "alloc/macro.format.html"),
+    (&["std::mem::MaybeUninit"], "std/mem/union.MaybeUninit.html"),
+    (&["--rustc", "lints"], "rustc/lints/index.html"),
+    (&["--rustdoc", "lints"], "rustdoc/lints.html"),
+    (
+        &["lints::broken_intra_doc_links", "--rustdoc"],
+        "rustdoc/lints.html",
+    ),
 ];
 
 fn repath(origin: &str) -> String {
@@ -30,15 +36,15 @@ fn repath(origin: &str) -> String {
     repathed.into_os_string().into_string().unwrap()
 }
 
-pub fn test_cases<'a>() -> impl Iterator<Item = (&'a str, String)> {
-    TEST_CASES.iter().map(|x| (x[0], repath(x[1])))
+pub fn test_cases<'a>() -> impl Iterator<Item = (&'a [&'a str], String)> {
+    TEST_CASES.iter().map(|(args, path)| (*args, repath(path)))
 }
 
 pub fn unique_paths() -> impl Iterator<Item = String> {
     // Hashset used to test uniqueness of values through insert method.
     let mut unique_paths = HashSet::new();
     TEST_CASES
         .iter()
-        .filter(move |e| unique_paths.insert(e[1]))
-        .map(|e| repath(e[1]))
+        .filter(move |(_, p)| unique_paths.insert(p))
+        .map(|(_, p)| repath(p))
 }

src/toolchain.rs

@@ -10,9 +10,10 @@ use std::{
     time::Duration,
 };
 
-use anyhow::{anyhow, bail};
+use anyhow::{anyhow, bail, Context};
 use fs_at::OpenOptions;
 use tracing::info;
+use url::Url;
 use wait_timeout::ChildExt;
 
 use crate::{
@@ -408,19 +409,35 @@ impl<'a> Toolchain<'a> {
         buf
     }
 
-    pub fn doc_path(&self, relative: &str) -> anyhow::Result<PathBuf> {
-        let parts = vec!["share", "doc", "rust", "html"];
-        let mut doc_dir = self.path.clone();
-        for part in parts {
-            doc_dir.push(part);
+    pub fn doc_path(&self, maybe_relative: impl AsRef<Path>) -> anyhow::Result<PathBuf> {
+        let relative = maybe_relative.as_ref();
+        if relative.is_absolute() {
+            return Ok(relative.to_owned());
         }
+
+        let mut doc_dir = self.path.clone();
+        doc_dir.extend(["share", "doc", "rust", "html"]);
         doc_dir.push(relative);
 
         Ok(doc_dir)
     }
 
-    pub fn open_docs(&self, relative: &str) -> anyhow::Result<()> {
-        utils::open_browser(&self.doc_path(relative)?)
+    pub fn open_docs(
+        &self,
+        maybe_relative: impl AsRef<Path>,
+        fragment: Option<&str>,
+    ) -> anyhow::Result<()> {
+        let maybe_relative = maybe_relative.as_ref();
+        let mut doc_url = Url::from_file_path(self.doc_path(maybe_relative)?)
+            .ok()
+            .with_context(|| {
+                anyhow!(
+                    "invalid doc file absolute path `{}`",
+                    maybe_relative.display()
+                )
+            })?;
+        doc_url.set_fragment(fragment);
+        utils::open_browser(doc_url.to_string())
     }
 
     /// Remove the toolchain from disk

src/utils/mod.rs

@@ -1,6 +1,7 @@
 //!  Utility functions for Rustup
 
 use std::env;
+use std::ffi::OsStr;
 use std::fs::{self, File};
 use std::io::{self, BufReader, Write};
 use std::ops::{BitAnd, BitAndAssign};
@@ -459,7 +460,7 @@ pub(crate) fn read_dir(name: &'static str, path: &Path) -> Result<fs::ReadDir> {
     })
 }
 
-pub(crate) fn open_browser(path: &Path) -> Result<()> {
+pub(crate) fn open_browser(path: impl AsRef<OsStr>) -> Result<()> {
     opener::open_browser(path).context("couldn't open browser")
 }
 

tests/suite/cli_rustup.rs

@@ -4,6 +4,7 @@ use std::fs;
 use std::path::{PathBuf, MAIN_SEPARATOR};
 use std::{env::consts::EXE_SUFFIX, path::Path};
 
+use itertools::chain;
 use rustup::for_host;
 use rustup::test::{
     mock::{
@@ -2640,16 +2641,20 @@ async fn docs_topical_with_path() {
         .expect_ok(&["rustup", "toolchain", "install", "nightly"])
         .await;
 
-    for (topic, path) in mock::topical_doc_data::test_cases() {
-        let mut cmd = clitools::cmd(&cx.config, "rustup", ["doc", "--path", topic]);
+    for (args, path) in mock::topical_doc_data::test_cases() {
+        let mut cmd = clitools::cmd(
+            &cx.config,
+            "rustup",
+            chain!(["doc", "--path"], args.iter().cloned()),
+        );
         clitools::env(&cx.config, &mut cmd);
 
         let out = cmd.output().unwrap();
         eprintln!("{:?}", String::from_utf8(out.stderr).unwrap());
         let out_str = String::from_utf8(out.stdout).unwrap();
         assert!(
             out_str.contains(&path),
-            "comparing path\ntopic: '{topic}'\nexpected path: '{path}'\noutput: {out_str}\n\n\n",
+            "comparing path\nargs: '{args:?}'\nexpected path: '{path}'\noutput: {out_str}\n\n\n",
         );
     }
 }