Link to files, not topics

.github/workflows/R-CMD-check.yaml

@@ -67,6 +67,7 @@ jobs:
  run: |
  remotes::install_deps(dependencies = TRUE)
  remotes::install_cran("rcmdcheck")
+ remotes::install_github("rstudio/rmarkdown")
  shell: Rscript {0}
 
  - name: Check

DESCRIPTION

@@ -59,5 +59,5 @@ VignetteBuilder:
  knitr
 Encoding: UTF-8
 Roxygen: list(markdown = TRUE, load = "installed")
-RoxygenNote: 7.1.0
+RoxygenNote: 7.1.0.9000
 Language: en-GB

NEWS.md

@@ -1,5 +1,9 @@
 # roxygen2 (development version)
 
+* When processing cross package markdown links (e.g. `[pkg::fun()]`),
+ roxygen2 now looks up the file it needs to link to, instead of linking to
+ the topic, to avoid "Non-file package-anchored links" `R CMD check` warnings.
+
 # roxygen2 7.1.0
 
 ## New features

R/markdown-link.R

@@ -4,20 +4,23 @@
 #' spaces between the closing and opening bracket in the `[text][ref]`
 #' form.
 #'
+#' Starting from R 4.0.2-ish, explicit cross-package links to topics are not
+#' allowed, so for each such linked topic, we look up the linked file.
+#'
 #' These are the link references we add:
 #' ```
 #' MARKDOWN LINK TEXT CODE RD
 #' -------- --------- ---- --
 #' [fun()] fun() T \\link[=fun]{fun()}
 #' [obj] obj F \\link{obj}
-#' [pkg::fun()] pkg::fun() T \\link[pkg:fun]{pkg::fun()}
-#' [pkg::obj] pkg::obj F \\link[pkg:obj]{pkg::obj}
+#' [pkg::fun()] pkg::fun() T \\link[pkg:file]{pkg::fun()}
+#' [pkg::obj] pkg::obj F \\link[pkg:file]{pkg::obj}
 #' [text][fun()] text F \\link[=fun]{text}
 #' [text][obj] text F \\link[=obj]{text}
-#' [text][pkg::fun()] text F \\link[pkg:fun]{text}
-#' [text][pkg::obj] text F \\link[pkg:obj]{text}
+#' [text][pkg::fun()] text F \\link[pkg:file]{text}
+#' [text][pkg::obj] text F \\link[pkg:file]{text}
 #' [s4-class] s4 F \\linkS4class{s4}
-#' [pkg::s4-class] pkg::s4 F \\link[pkg:s4-class]{pkg::s4}
+#' [pkg::s4-class] pkg::s4 F \\link[pkg:file]{pkg::s4}
 #' ```
 #'
 #' The reference links will always look like `R:ref` for `[ref]` and
@@ -111,6 +114,8 @@ parse_link <- function(destination, contents, state) {
  ## `obj` is fun or obj (fun is without parens)
  ## `s4` is TRUE if we link to an S4 class (i.e. have -class suffix)
  ## `noclass` is fun with -class removed
+ ## `file` is the file name of the linked topic, if known. Otherwise random
+ ## id to fill in later.
 
  is_code <- is_code || (grepl("[(][)]$", destination) && ! has_link_text)
  pkg <- str_match(destination, "^(.*)::")[1,2]
@@ -121,6 +126,7 @@ parse_link <- function(destination, contents, state) {
  obj <- sub("[(][)]$", "", fun)
  s4 <- str_detect(destination, "-class$")
  noclass <- str_match(fun, "^(.*)-class$")[1,2]
+ file <- find_topic_filename(pkg, obj, state$tag)
 
  ## To understand this, look at the RD column of the table above
  if (!has_link_text) {
@@ -130,7 +136,7 @@ parse_link <- function(destination, contents, state) {
  if (is_fun || ! is.na(pkg)) "[",
  if (is_fun && is.na(pkg)) "=",
  if (! is.na(pkg)) paste0(pkg, ":"),
- if (is_fun || ! is.na(pkg)) paste0(obj, "]"),
+ if (is_fun || ! is.na(pkg)) paste0(if (is.na(pkg)) obj else file, "]"),
  "{",
  if (!is.na(pkg)) paste0(pkg, "::"),
  if (s4) noclass else fun,
@@ -146,7 +152,7 @@ parse_link <- function(destination, contents, state) {
  if (is_code) "\\code{",
  "\\link[",
  if (is.na(pkg)) "=" else paste0(pkg, ":"),
- obj,
+ if (is.na(pkg)) obj else file,
  "]{"
  ),
  contents,

R/object-import.R

@@ -19,7 +19,12 @@ merge.rd_section_reexport <- function(x, y, ...) {
 format.rd_section_reexport <- function(x, ...) {
  pkgs <- split(x$value$fun, x$value$pkg)
  pkg_links <- map2(names(pkgs), pkgs, function(pkg, funs) {
- links <- paste0("\\code{\\link[", pkg, "]{", escape(sort(funs)), "}}",
+ funs <- sort(funs)
+ files <- vapply(funs, find_topic_in_package_reexp, character(1), pkg = pkg)
+ links <- paste0(
+ "\\code{\\link[", pkg,
+ ifelse(files == funs, "", paste0(":", files)),
+ "]{", escape(funs), "}}",
  collapse = ", ")
  paste0("\\item{", pkg, "}{", links, "}")
  })
@@ -34,3 +39,19 @@ format.rd_section_reexport <- function(x, ...) {
  "\n}}\n"
  )
 }
+
+find_topic_in_package_reexp <- function(pkg, topic) {
+ path <- tryCatch(
+ find_topic_in_package(pkg, topic),
+ error = function(err) {
+ roxy_warning("Unavailable package in re-export: ", pkg, "::", topic)
+ topic
+ }
+ )
+ if (is.na(path)) {
+ roxy_warning("Unavailable topic in re-export: ", pkg, "::", topic)
+ topic
+ } else {
+ path
+ }
+}

R/rd-find-link-files.R

@@ -0,0 +1,63 @@
+
+#' Find the Rd file of a topic
+#'
+#' @param pkg Package to search in, or `NA` if no package was specified.
+#' If the same as the dev package, then we treat it as `NA`.
+#' @param topic Topic to search for. This is the escaped, so it is `"\%\%"` and
+#' not `"%%"`.
+#' @param tag The roxy tag object that contains the link. We use this for
+#' better warnings, that include the file name and line number (of the tag).
+#' @return String. File name to link to.
+#'
+#' @details
+#' If `pkg` is `NA` or the package being documented, we'll just leave the
+#' topic alone.
+#'
+#' If `pkg` is not `NA` and not the package being documented (the _dev_
+#' package), then we need to be able to find the Rd file. If we can't, that's
+#' a warning and the link is left untouched. This typically happens when the
+#' linked package is not installed or cannot be loaded.
+#'
+#' @noRd
+
+find_topic_filename <- function(pkg, topic, tag) {
+ tag <- tag %||% list(file = NA, line = NA)
+ if (is.na(pkg) || identical(roxy_meta_get("current_package"), pkg)) {
+ topic
+ } else {
+ path <- tryCatch(
+ find_topic_in_package(pkg, topic),
+ error = function(err) {
+ roxy_tag_warning(tag, "Link to unavailable package: ", pkg, ". ", err$message)
+ topic
+ }
+ )
+ if (is.na(path)) {
+ roxy_tag_warning(tag, "Link to unknown topic: ", topic, " in package ", pkg)
+ topic
+ } else {
+ path
+ }
+ }
+}
+
+#' Find a help topic in a package
+#'
+#' This is used by both `find_topic_filename()` and
+#' `format.rd_section_reexport()` that creates the re-exports page. The error
+#' messages are different for the two, so errors are not handled here.
+#'
+#' @param pkg Package name. This cannot be `NA`.
+#' @inheritParams find_topic_filename
+#' @return File name if the topic was found, `NA` if the package could be
+#' searched, but the topic was not found. Errors if the package cannot be
+#' searched. (Because it is not installed or cannot be loaded, etc.)
+#'
+#' @noRd
+
+find_topic_in_package <- function(pkg, topic) {
+ # This is needed because we have the escaped text here, and parse_Rd will
+ # un-escape it properly.
+ raw_topic <- str_trim(tools::parse_Rd(textConnection(topic))[[1]][1])
+ basename(utils::help((raw_topic), (pkg))[1])
+}

R/rd-inherit.R

@@ -186,7 +186,13 @@ inherit_dot_params <- function(topic, topics, env) {
  # Build the Rd
  # (1) Link to function(s) that was inherited from
  src <- inheritors$source
- dest <- ifelse(has_colons(src), gsub("::", ":", src), paste0("=", src))
+ if (has_colons(src)) {
+ target <- str_split_fixed(src, "::", n = 2)
+ file <- find_topic_in_package(target[1], target[2])
+ dest <- paste0(target[1], ":", file)
+ } else {
+ dest <- paste0("=", src)
+ }
  from <- paste0("\\code{\\link[", dest, "]{", src, "}}", collapse = ", ")
 
  # (2) Show each inherited argument

R/utils-rd.R

@@ -88,8 +88,14 @@ tweak_links <- function(x, package) {
  topic <- substr(opt, 2, nchar(opt))
 
  if (has_topic(topic, package)) {
- attr(x, "Rd_option") <- structure(paste0(package, ":", topic), Rd_tag = "TEXT")
+ file <- find_topic_in_package(package, topic)
+ attr(x, "Rd_option") <- structure(paste0(package, ":", file), Rd_tag = "TEXT")
  }
+ } else if (grepl(":", opt)) {
+ # need to fix the link to point to a file
+ target <- str_split_fixed(opt, ":", n = 2)
+ file <- find_topic_in_inherited_link(target[1], target[2])
+ attr(x, "Rd_option") <- structure(paste0(target[1], ":", file), Rd_tag = "TEXT")
  }
  }
  } else if (length(x) > 0) {
@@ -100,6 +106,22 @@ tweak_links <- function(x, package) {
  x
 }
 
+find_topic_in_inherited_link <- function(pkg, topic) {
+ path <- tryCatch(
+ find_topic_in_package(pkg, topic),
+ error = function(err) {
+ roxy_warning("Unavailable package in inherited link: ", pkg, "::", topic)
+ topic
+ }
+ )
+ if (is.na(path)) {
+ roxy_warning("Unavailable topic in inherited link: ", pkg, "::", topic)
+ topic
+ } else {
+ path
+ }
+}
+
 # helpers -----------------------------------------------------------------
 
 parse_rd <- function(x) {

tests/testthat/test-markdown-link.R

@@ -51,7 +51,7 @@ test_that("% in links are escaped", {
  expect_equal(markdown("[x][%%]"), "\\link[=\\%\\%]{x}")
  expect_equal(markdown("[%][x]"), "\\link[=x]{\\%}")
  expect_equal(markdown("[%%]"), "\\link{\\%\\%}")
- expect_equal(markdown("[foo::%%]"), "\\link[foo:\\%\\%]{foo::\\%\\%}")
+ expect_equal(markdown("[base::%%]"), "\\link[base:Arithmetic]{base::\\%\\%}")
 })
 
 test_that("commonmark picks up the various link references", {
@@ -94,16 +94,19 @@ test_that("short and sweet links work", {
  foo <- function() {}")[[1]]
  expect_equivalent_rd(out1, out2)
 
- out1 <- roc_proc_text(rd_roclet(), "
+ expect_warning(
+ out1 <- roc_proc_text(rd_roclet(), "
  #' Title
  #'
- #' See [pkg::function()], [pkg::object].
+ #' See [11pkg::function()], [11pkg::object].
  #' @md
- foo <- function() {}")[[1]]
+ foo <- function() {}")[[1]],
+ "Link to unavailable package"
+ )
  out2 <- roc_proc_text(rd_roclet(), "
  #' Title
  #'
- #' See \\code{\\link[pkg:function]{pkg::function()}}, \\link[pkg:object]{pkg::object}.
+ #' See \\code{\\link[11pkg:function]{11pkg::function()}}, \\link[11pkg:object]{11pkg::object}.
  foo <- function() {}")[[1]]
  expect_equivalent_rd(out1, out2)
 
@@ -120,16 +123,19 @@ test_that("short and sweet links work", {
  foo <- function() {}")[[1]]
  expect_equivalent_rd(out1, out2)
 
- out1 <- roc_proc_text(rd_roclet(), "
+ expect_warning(
+ out1 <- roc_proc_text(rd_roclet(), "
  #' Title
  #'
- #' Description, see [name words][pkg::bar].
+ #' Description, see [name words][stringr::bar111].
  #' @md
- foo <- function() {}")[[1]]
+ foo <- function() {}")[[1]],
+ "Link to unknown topic: bar111 in package stringr"
+ )
  out2 <- roc_proc_text(rd_roclet(), "
  #' Title
  #'
- #' Description, see \\link[pkg:bar]{name words}.
+ #' Description, see \\link[stringr:bar111]{name words}.
  foo <- function() {}")[[1]]
  expect_equivalent_rd(out1, out2)
 
@@ -243,7 +249,7 @@ test_that("another markdown link bug is fixed", {
 
 test_that("markdown code as link text is rendered as code", {
 
- out1 <- roc_proc_text(rd_roclet(), "
+ suppressWarnings(out1 <- roc_proc_text(rd_roclet(), "
  #' Title
  #'
  #' Description, see [`name`][dest],
@@ -253,7 +259,7 @@ test_that("markdown code as link text is rendered as code", {
  #' [`terms`][terms.object],
  #' [`abc`][abc-class].
  #' @md
- foo <- function() {}")[[1]]
+ foo <- function() {}")[[1]])
  out2 <- roc_proc_text(rd_roclet(), "
  #' Title
  #'
@@ -366,12 +372,12 @@ test_that("links to S4 classes are OK", {
  foo <- function() {}")[[1]]
  expect_equivalent_rd(out1, out2)
 
- out1 <- roc_proc_text(rd_roclet(), "
+ suppressWarnings(out1 <- roc_proc_text(rd_roclet(), "
  #' Title
  #'
  #' Description, see [pkg::linktos4-class] as well.
  #' @md
- foo <- function() {}")[[1]]
+ foo <- function() {}")[[1]])
  out2 <- roc_proc_text(rd_roclet(), "
  #' Title
  #'

tests/testthat/test-object-import.R

@@ -51,3 +51,14 @@ test_that("can't set description and re-export", {
 
  expect_length(out, 0)
 })
+
+test_that("warnings for unknown packages and objects", {
+ expect_warning(
+ format(rd_section_reexport("11papaya", "fun")),
+ "Unavailable package in re-export"
+ )
+ expect_warning(
+ format(rd_section_reexport("stringr", "12345543221")),
+ "Unavailable topic in re-export"
+ )
+})

tests/testthat/testNamespace/DESCRIPTION

@@ -6,4 +6,4 @@ Author: Hadley <hadley@rstudio.com>
 Maintainer: Hadley <hadley@rstudio.com>
 Encoding: UTF-8
 Version: 0.1
-RoxygenNote: 7.0.2.9000
+RoxygenNote: 7.1.0.9000